Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Update typography system and values, implement compact theme #803

Merged
merged 30 commits into from
Nov 28, 2023

Conversation

VincentSmedinga
Copy link
Contributor

@VincentSmedinga VincentSmedinga commented Nov 22, 2023

  • Removes the typography breakpoint – text size grows in one straight line now
  • Updates the minimum and maximum font sizes for each text level using typographic scales
  • Updates all components to use the new tokens

Text size 4 and 5 are the same now. If the designers agree, we’ll consolidate them in code in a separate PR.

The .amsterdam-theme--density-high class name is a very temporary approach to allow visually evaluating the updated font sizes. We will soon replace it with a theming layer.

For the icon component, all ‘high density’ classes had to move to the bottom because of a specificity rule in Stylelint. For the rest, I minimised the changes to avoid merge conflicts. I did replace custom token names like container-multiplier with line-height.

@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 22, 2023 15:27 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 22, 2023 15:33 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 22, 2023 15:35 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 23, 2023 09:23 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 24, 2023 21:58 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 24, 2023 22:01 Destroyed
@VincentSmedinga VincentSmedinga changed the title Update typography system and values, implement compact theme feat!: Update typography system and values, implement compact theme Nov 24, 2023
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 24, 2023 22:05 Destroyed
@VincentSmedinga VincentSmedinga marked this pull request as ready for review November 24, 2023 22:27
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 26, 2023 17:40 Destroyed
# Conflicts:
#	packages/css/src/footer/footer.scss
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 27, 2023 08:58 Destroyed
@alimpens
Copy link
Contributor

I do not agree with scattering a wrapping theme class like this

Certainly not ideal, but the alternative is pulling a complete theming approach into scope of this PR. That would be even worse.

Let’s see this PR as an iteration in the right direction and start work on theming asap so that finding these ugly classes won’t be difficult.

or calling the default font size narrow density-low to avoid a boolean property for density.

The density theme is not about font size alone but spacing lengths (grid and future row and column margins) as well. So if we were to take the Boolean route we would use ‘spacious’ and ‘compact’.

I may be too strict in trying to keep our options open here. @alimpens which approach would you prefer?

I think it totally depends on whether we realistically expect there to be more density variants in the future, which is hard to guess. I'm not a huge fan of adding even more complexity to our already complex spacing system, so maybe we should go for a boolean to make it harder to make it more complex in the future ;) Unless we have an indication that more density variants are required?

@VincentSmedinga
Copy link
Contributor Author

I think it totally depends on whether we realistically expect there to be more density variants in the future, which is hard to guess.

To be honest, a third density variation appears far-fetched. Extra compact? Semi-spacious?

to make it harder to make it more complex in the future

Yes, how can no-one ever prevent unavoiding not disagreeing with that!

I’ve replaced everything with spacious and compact.

@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 27, 2023 22:02 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 27, 2023 22:19 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 27, 2023 22:31 Destroyed
@dlnr
Copy link
Contributor

dlnr commented Nov 28, 2023

I think it totally depends on whether we realistically expect there to be more density variants in the future, which is hard to guess.

To be honest, a third density variation appears far-fetched. Extra compact? Semi-spacious?

to make it harder to make it more complex in the future

Yes, how can no-one ever prevent unavoiding not disagreeing with that!

I’ve replaced everything with spacious and compact.

Here are links to theming in Storybook:

@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 28, 2023 08:57 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 28, 2023 09:06 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 28, 2023 09:08 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 28, 2023 09:11 Destroyed
@VincentSmedinga
Copy link
Contributor Author

VincentSmedinga commented Nov 28, 2023

Here are links to theming in Storybook:

Thanks. The second and third link are about theming canvases, e.g. dark mode.

The first is on theming Storybook itself. I can’t find anything on font sizes there yet. And I think that’s an important change we need.

@github-actions github-actions bot temporarily deployed to demo-DES-457-update-typography November 28, 2023 13:45 Destroyed
@VincentSmedinga VincentSmedinga merged commit 9b087ec into develop Nov 28, 2023
4 checks passed
@VincentSmedinga VincentSmedinga deleted the feature/DES-457-update-typography branch November 28, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants