Consolidate line-height() and font-size() function values in tokens

alex-page commented 2 years ago

This issue is blocked by https://github.com/Shopify/polaris-react/issues/4588 but in the meantime we can land on the approach we want to take.

aveline commented 2 years ago

Line height

token	base	large screen
`--line-height-small` `--line-height-button`	16px
`--line-height-body`	20px
`--line-height-large` `--line-height-input` `--line-height-display-small`	24px
`--line-height-display-medium` `--line-height-display-large`	28px	32px
`--line-height-display-xlarge`	36px	44px

Perhaps --line-height-button isn't necessary. Thoughts?

Otherwise the line heights map to the font sizes below, which should hopefully make it easy to reason about which to use together.

aveline commented 2 years ago

token	base	large screen
`--fs-body`	15px	14px
`--fs-input`	16px	14px
`--fs-small`	13px	12px
`--fs-large`	17px	16px
`--fs-display-small`	16px	20px
`--fs-display-medium`	21px	26px
`--fs-display-large`	24px	28px
`--fs-display-xlarge`	27px	42px

I am not convinced that we should retain the base and large screen definitions. I am worried that increases confusion between developer + designer handoff as that is extra context that has to be shared. We also don't have a great way to put responsive font-sizes into tools like Figma. I think we could simplify this with a numerical scale:

fs-1: 12px
fs-2: 14px
fs-3: 16px
fs-4: 20px

I also wonder if the 13px and 17px values are useful? Can our users notice the difference? Do they align to a 4x4 grid? We might be able to simplify the scale by removing values.

aveline commented 2 years ago

@alex-page my concern with removing the base/mobile scale is how large of a change it is. But if we are ok with the scope of the changes, then let's try removing them.

The one I do feel strongly about keeping responsive is the input one. We definitely want the input size to be 16px on mobile, any smaller will trigger input zoom on Safari mobile browsers. And perhaps that's why the body was bumped up to 15px, because otherwise the inputs at 16px look a bit large with body at 14px? I can play around with this to compare.

I'm inclined to have a bit more descriptive token names than a numerical scale. Looking at fs-2: 14px and fs-3: 16px it doesn't seem immediately obvious which one is the standard body font size.

alex-page commented 2 years ago

Oof. That is a clear concern. Let me try and jot down what I was thinking.

I see two potential approaches:

Remove all semantic connections in tokens. The tokens become a scale of font-size and line-height values we recommend for typography. We help users implement great typography through components and documentation. Display, TextField Label etc. would reference values from this scale. Users creating new or custom components could use existing components or create their own acessing values from the scale. In our documentation we could add important information like 16px is required for iOS devices to stop inputs zooming when focused.

This approach would keep the tokens simple with one object for font-size and line-height. We could iterated and adjust the tokens with little risk as there is no semantic layer.

fontsize: {
  "xxs": "12px",
  "xs": "14px",
  "sm": "16px",
  ...
  "xxl": "48px",
}

.TextInput {
  font-size: var(--p-fontsize-sm);
  font-size: var(--p-lineheight-sm);

  @include media(min-width: $mobile) {
    font-size: var(--p-fontsize-xs);
    font-size: var(--p-lineheight-xs);
  }
}

Keep semantic connections in tokens. We have values for specific implementations in our tokens. Users can easily access these tokens understanding when to use them. There might be overlapping values but that is okay because we have clear guidance for use. Tokens can be overriden at different breakpoints. Building custom components you have less options for font-size and line-height as it's dependent on implementation.

fontsize: {
  "caption": "12px",
  "body": "14px",
  "input": "16px",
  ...
  "heading-sm": "16px",
  ...
  "heading-lg": "48px",
}

.TextInput {
  font-size: var(--p-fontsize-input);
  font-size: var(--p-lineheight-input);

  @include media(min-width: $mobile) {
    --p-fontsize-input: 14px;
    --p-lineheight-input: 20px;
  }
}

My gut feeling is to ship 1 (for this project) and build/update our components to create semantic relationships (future work).

aveline commented 2 years ago

I like the approach of focusing on the scale first, with the intent to layer in semantic tokens down the line. With that in mind, here is what that could look like. The body font size should be the mid value of the scale so it's obvious that is the body font size instead of having to guess or remember that body font size is xs or sm. That does leave us with many larger sizes though, and the xxl pattern doesn't seem very elegant.

Font scale tokens

token	base	large screen
`--fs-sm`	13px	12px
`--fs`	15px	14px
`--fs-lg`	17px	16px
`--fs-xl`	16px	20px
`--fs-xxl`	21px	26px
`--fs-xxxl`	24px	28px
`--fs-xxxxl`	27px	42px

Line height tokens

token	base	large screen
`--line-height-small`	16px
`--line-height`	20px
`--line-height-lg` `--line-height-xl`	24px
`--line-height-xxl` `--line-height-xxxl`	28px	32px
`--line-height-xxxxl`	36px	44px

And I would make the case for including one semantic token for input. It's a bit of an edge case since it's important to maintain the current experience on mobile and prevent input zoom.

Semantic font size tokens

token	base	large screen
`--fs-input`	16px	14px

aaronccasanova commented 2 years ago

Dropping some comments that came out of @aveline and my pairing session:

Since our font-size and line-height tokens are not organized in token groups but rather token scales it might be more idiomatic to refer to each value by their index.

Example of token groups: --typography-heading1-font-size, --typography-heading2-font-size, etc...
Example of token scales: fontSize = [1, 2, 3] Note: Token scales can still have aliases. See the Scale Aliases section of the System UI spec.

One issue with establishing aliases for each token scale is the resulting custom property name can be excessively large and difficult to parse (visually) :

const fontSize = [1, 2, 3, 4, 5, 6]
--p-font-size-base: fontSize[0]
--p-font-size-lg: fontSize[1]
--p-font-size-xl: fontSize[2]
--p-font-size-xxl: fontSize[3]
--p-font-size-xxxl: fontSize[4]
--p-font-size-xxxxl: fontSize[5]

Alternatively, with array index values.

const fontSize = [1, 2, 3, 4, 5, 6]
--p-font-size-0: fontSize[0]
--p-font-size-1: fontSize[1]
--p-font-size-2: fontSize[2]
--p-font-size-3: fontSize[3]
--p-font-size-4: fontSize[4]
--p-font-size-5: fontSize[5]

// Keep in mind we can still establish aliases
--p-font-size-base: fontSize[1]
--p-font-size-display: fontSize[4]

I believe this pattern translates to all token scales when being transformed to CSS custom properties. Here is an example of how GitHub primary references token scales as an additional reference: https://primer.style/primitives/colors#scale-variables

sarahill commented 2 years ago

I kind of like the approach we came up with during the previous foundation work. Mapping more to h1 - h7 and body and caption sizes. Mapping it more to semantic naming feels a little more intuitive and scalable.

@aaronccasanova is this similar to what you're proposing above?

alex-page commented 2 years ago

I like that example @sarahill as it flattens the values out. I find the base and large-screen layer unecessary right now.

aveline commented 2 years ago

I find the base and large-screen layer unecessary right now.

@alex-page would this mean getting rid of the responsive font sizes?

aveline commented 2 years ago

Here is where I see us being at.

✅ Have a font scale that can be referenced by index

🟠 Create semantic aliases that map to the font scale to make it more intuitive

Should we include this as part of of the first version, or add the aliasases in the future? The alisases may add a bit of overhead to future changes and adjustments we may want to make.

❓ Font scale values are responsive

Handle the responsive values where custom properties are defined so there's a single source of truth

alex-page commented 2 years ago

@alex-page would this mean getting rid of the responsive font sizes?

No. It would mean that the responsive sizes are only paired together in components. Not at the token layer. When building new components I can choose to add responsive sizes or keep it as one size.

Adding a semantic layer for screen sizes at the token layer encourages them to be used in a specific way when building new or custom components. This could increase forks or hard coded values. I think the components should add the semantic layer and responsive styles, not the tokens.

aaronccasanova commented 2 years ago

@aaronccasanova is this similar to what you're proposing above?

No, but what you've proposed is actually my preferred approach. I think how you created scoped alias tokens that correspond to a composite token groups enables the most flexibility in regards to theming. Since we don't currently have that flexibility in place and our main focus is to get 100% token coverage in polaris-react I didn't push my bias on this topic.

If we are proceeding with token scales (e.g. a list of values with no alias associated) for font-size and line-height, I'm in favor of using the index value for each custom property (e.g. --font-size-0, --font-size-1, etc).

Note: Although not my preferred approach, typography token scales are perfectly sufficient for the admin where the values are unchanged between different color schemes. This does not hold true and will eventually breakdown if the tokens are consumed in other applications where type styles change between themes.

Otherwise, if we decide on alias/composite token groups, I'm in favor of restructuring the tokens to match your screenshot. This is the most future facing approach IMO but may be off target from our initiative.

sarahill commented 2 years ago

☝️ That makes sense. I'll let you all make the call in terms of what's possible or makes sense within the scope of this work. We're looking at type now so we can always make some of these updates as part of those updates if that makes more sense

lgriffee commented 2 years ago

I feel some of the topics we've touched on in this thread relate to a great article Typography in Design Systems by Dan Mall.

I really like the token scale approach, @aaronccasanova and @aveline, that you suggested as a result of your pairing session:

const fontSize = [1, 2, 3, 4, 5, 6] --p-font-size-0: fontSize[0] --p-font-size-1: fontSize[1] --p-font-size-2: fontSize[2] --p-font-size-3: fontSize[3] --p-font-size-4: fontSize[4] --p-font-size-5: fontSize[5]

To pull out some of the things Dan mentions in his article, I think this approach for both token values and alias avoids:

T-shirt sizing
- Doesn't scale well
- ex. --p-font-size-xxxxl
Context-specific dichotomies
- Can be easily conflated with HTML tags and might prevent the usage of the right style in places
- ex. <h2 class="heading-4">
- ex. if the title caption prevents a someone from using that style in a nav even if it's the right choice
Some sort of typographic scale
- Hard to calculate in your head
- ex. a base size of 16px multiplied by the Golden Ratio (1.618) leads to a secondary value of 25.888px

While using numerical based alias values is perhaps not as intuitive as context-specific dichotomies, I think they still could be guessable—we could keep all of our font size tokens in size order for guessability (if you select a higher number the font-size will go up). I'm not sure how that would play out future proofing wise though...ex. we want to add a new value in between --p-font-size-0 and --p-font-size-1 so we do the following:

const fontSize = [1, 2 (new value), 3, 4, 5, 6, 7] --p-font-size-0: fontSize[0] --p-font-size-1: fontSize[1] --p-font-size-2: fontSize[2] --p-font-size-3: fontSize[3] --p-font-size-4: fontSize[4] --p-font-size-5: fontSize[5] --p-font-size-6: fontSize[6]

Which would require us to go and replace all those values across the codebase (ex. --p-font-size-1 > --p-font-size-2, --p-font-size-2 > --p-font-size-3, etc.) and would create breaking changes for consumers...but perhaps we would still face those same challenges with context-specific dichotomy alias naming? Or perhaps the benefits of context-specific dichotomy alias naming (despite it's downsides) does outweigh this approach? Just trying to think through all the possibilities here with how alias names could affect consumer usage of the tokens and updates in the future.

One more thought on using a numerical based alias values is that it could also work for defining scoped alias tokens that correspond to composite token groups (to enable the most flexibility in regards to theming).

Ex. Instead of a composite token being heading-1 we could do font-1 or text-1

I'm not 100% sure that numerical based alias values are the way to go (or even matter right now for the scope of our current work vs. future work) but just wanted to try and name some pros and cons here as part of the discussion.

aaronccasanova commented 2 years ago

@lgriffee Thanks for sharing the Typography in Design Systems article! It was a very good read 👍

My only concern with the concluding solution (e.g. composite tokens with a generic preset-1, preset-2, etc. naming convention) is that it doesn't account for Typography systems where different font-families are used.

.preset-1 { font-family: sans; ...more-type-styles }
.preset-2 { font-family: sans; ...more-type-styles }

.preset-3 { font-family: sans-serif; ...more-type-styles }
.preset-4 { font-family: sans-serif; ...more-type-styles }

The system is no longer "guessable".

Context-specific dichotomies Can be easily conflated with HTML tags and might prevent the usage of the right style in places

ex. <h2 class="heading-4">

ex. if the title caption prevents a someone from using that style in a nav even if it's the right choice

I'm wondering if the outlined problem is an actual concern we have. This is similar to @sarahill's screenshot and is my personal preference for flexible type systems. Not entirely sure how using composite token groups could prevent the usage of the right style as designers should be selecting the appropriate composite token (with no consideration of the underlying markup) and developers would grab those value straight from design files and apply the appropriate/semantic markup (regardless of the defined composite token).

(subjective) While using a system of composite tokens (heading1-7, body, caption) does remove the "guessability" factor, it enables the most flexibility in our type system with a minimal learning curve.

lgriffee commented 2 years ago

@aaronccasanova Yeah I also didn't think the concluding solution was exactly what we should use in our system. Good point about different font-families.

It's totally possible context-specific dichotomies are not a concern we have for our system—just wanted to bring it up to make sure 👍🏻

aveline commented 2 years ago

Summarized the discussion in this figjam file and landed on a basic font-size scale and line-height scale.

The font-size scale contains all the font size values for mobile and large screens. Responsive behaviour will be handled at the component level.

Alias tokens and composite tokens will be addressed in future issues.

Shopify / polaris