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

task: adds functionality for the responsive tokens #44

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

skibinska
Copy link
Member

@skibinska skibinska commented Dec 6, 2024

This PR

  • generates a responsive.css file that contains all the responsive tokens from the tokens/semantic.json file.
  • adds formatResponsiveCss, which is responsible for the output of the responsive.css file by extracting tokens from the dictionary and organising them based on mobile, tablet, and desktop breakpoints.

@skibinska skibinska changed the title adds functionality for the responsive tokens task: adds functionality for the responsive tokens Dec 6, 2024
@skibinska
Copy link
Member Author

@AnnaSchmidt05 @dana-saur

I wanted to suggest updating the naming of the responsive tokens to better align with our current breakpoints. For reference, here are the breakpoints we’re using:

--wds-grid-breakpoint-xs: 320px; /* Web breakpoint - mobile */
--wds-grid-breakpoint-sm: 768px; /* Web breakpoint - tablet */
--wds-grid-breakpoint-md: 1024px; /* Web breakpoint - tablet */
--wds-grid-breakpoint-lg: 1440px; /* Web breakpoint - desktop */
--wds-grid-breakpoint-xl: 1920px;

It seems that the current naming for the responsive headers doesn’t fully reflect this structure. For example:

--wds-text-heading-breakpoint-sm-xl: 1.5rem; /* 24px */
--wds-text-heading-breakpoint-md-xl: 1.75rem; /* 28px */
--wds-text-heading-breakpoint-lg-xl: 2rem; /* 32px */

Based on this, here’s what I understand:

Since sm represents 768px (tablet), there isn’t a defined value for mobile. This means the token starts at the tablet breakpoint. As a result:

  • --wds-text-heading-xl would be 24px at sm (768px),
  • 28px at md (1024px),
  • and 32px at lg (1440px).

Please let me know if I’ve missed anything or if you have a different perspective. For consistency, I think it would be helpful to align with the naming conventions defined in the tokens.

@AnnaSchmidt05
Copy link
Collaborator

Thanks Ewelina for reviewing this and for spotting the inconsistencies with breakpoint names here.

These tokens may not be needed if we go ahead with the fluid typescale as the font size won't depend on set breakpoints. Therefore we can probably delete these tokens.

The reason I say probably though is because I'm not sure how we then define different typography styles in the tokens (e.g. Hading Brand, Heading, Body etc.). We've added font families, weights, line heights and now fluid font size as core tokens but it's unclear to me how we'd use the fluid tokens to create typography style tokens. My only thinking is if we don't include font size in typography styles and this is just calculated separately using the fluid scale.

@dana-saur do you have any more idea about this?

Once we have worked this out, we will either update the responsive header token naming OR delete the token completely

@AnnaSchmidt05
Copy link
Collaborator

@skibinska I have now removed the typography styling tokens (awaiting approval by Dana in this PR) and checked other responsive token set up and I believe all tokens now match the breakpoint naming convention above.

Are you able to double check and confirm all is ok?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants