210: update typography line height tokens to be REM based #211

garrettbear · 2022-08-04T00:15:18Z

Switch REM based line height to match exactly with design

  --line-height-1: 1rem; // 16px
  --line-height-2: 1.25rem; // 20px
  --line-height-3: 1.375rem; // 22px
  --line-height-4: 1.5rem; // 24px
  --line-height-5: 2rem; // 32px
  --line-height-6: 2.5rem; // 40px
  --line-height-7: 3.5rem; // 56px

georgewrmarshall

LGTM! Nice work 💯

NidhiKJha

LGTM! 🚀

metamaskbot · 2022-08-04T20:40:24Z

Builds ready [5d9d148]

storybook: Storybook
all artifacts

georgewrmarshall

Could you add a screenshot of the typography ramps using the css vars compared to the CSS in JS so we can ensure they are the same? Hmm or even a couple of stories?

Heres the way I did it to check the SmallScreen css vars

export const SmallScreen: ComponentStory<typeof Text> = (...args) => {
  const smallScreenFontSizeBase = 16;
  const jsObject = {
    displayMD: {
      fontFamily: typography.sDisplayMD.fontFamily,
      fontSize: typography.sDisplayMD.fontSize,
      lineHeight: `${typography.sDisplayMD.lineHeight}px`,
      fontWeight: typography.sDisplayMD.fontWeight,
    },
    headingLG: {
      fontFamily: typography.sHeadingLG.fontFamily,
      fontSize: typography.sHeadingLG.fontSize,
      lineHeight: `${typography.sHeadingLG.lineHeight}px`,
      fontWeight: typography.sHeadingLG.fontWeight,
    },
    headingMD: {
      fontFamily: typography.sHeadingMD.fontFamily,
      fontSize: typography.sHeadingMD.fontSize,
      lineHeight: `${typography.sHeadingMD.lineHeight}px`,
      fontWeight: typography.sHeadingMD.fontWeight,
    },
    headingSM: {
      fontFamily: typography.sHeadingSM.fontFamily,
      fontSize: typography.sHeadingSM.fontSize,
      lineHeight: `${typography.sHeadingSM.lineHeight}px`,
      fontWeight: typography.sHeadingSM.fontWeight,
    },
    headingSMRegular: {
      fontFamily: typography.sHeadingSMRegular.fontFamily,
      fontSize: typography.sHeadingSMRegular.fontSize,
      lineHeight: `${typography.sHeadingSMRegular.lineHeight}px`,
      fontWeight: typography.sHeadingSMRegular.fontWeight,
    },
    bodyMD: {
      fontFamily: typography.sBodyMD.fontFamily,
      fontSize: typography.sBodyMD.fontSize,
      lineHeight: `${typography.sBodyMD.lineHeight}px`,
      fontWeight: typography.sBodyMD.fontWeight,
    },
    bodyMDBold: {
      fontFamily: typography.sBodyMDBold.fontFamily,
      fontSize: typography.sBodyMDBold.fontSize,
      lineHeight: `${typography.sBodyMDBold.lineHeight}px`,
      fontWeight: typography.sBodyMDBold.fontWeight,
    },
    bodySM: {
      fontFamily: typography.sBodySM.fontFamily,
      fontSize: typography.sBodySM.fontSize,
      lineHeight: `${typography.sBodySM.lineHeight}px`,
      fontWeight: typography.sBodySM.fontWeight,
    },
    bodySMBold: {
      fontFamily: typography.sBodySMBold.fontFamily,
      fontSize: typography.sBodySMBold.fontSize,
      lineHeight: `${typography.sBodySMBold.lineHeight}px`,
      fontWeight: typography.sBodySMBold.fontWeight,
    },
    bodyXS: {
      fontFamily: typography.sBodyXS.fontFamily,
      fontSize: typography.sBodyXS.fontSize,
      lineHeight: `${typography.sBodyXS.lineHeight}px`,
      fontWeight: typography.sBodyXS.fontWeight,
      letterSpacing: typography.sBodyXS.letterSpacing,
    },
  };

  const styles = {
    displayMD: {
      fontFamily: 'var(--typography-s-display-md-font-family)',
      fontSize: 'var(--typography-s-display-md-font-size)',
      lineHeight: 'var(--typography-s-display-md-line-height)',
      fontWeight: 'var(--typography-s-display-md-font-weight)',
    },
    headingLG: {
      fontFamily: 'var(--typography-s-heading-lg-font-family)',
      fontSize: 'var(--typography-s-heading-lg-font-size)',
      lineHeight: 'var(--typography-s-heading-lg-line-height)',
      fontWeight: 'var(--typography-s-heading-lg-font-weight)',
    },
    headingMD: {
      fontFamily: 'var(--typography-s-heading-md-font-family)',
      fontSize: 'var(--typography-s-heading-md-font-size)',
      lineHeight: 'var(--typography-s-heading-md-line-height)',
      fontWeight: 'var(--typography-s-heading-md-font-weight)',
    },
    headingSM: {
      fontFamily: 'var(--typography-s-heading-sm-font-family)',
      fontSize: 'var(--typography-s-heading-sm-font-size)',
      lineHeight: 'var(--typography-s-heading-sm-line-height)',
      fontWeight: 'var(--typography-s-heading-sm-font-weight)',
    },
    headingSMRegular: {
      fontFamily: 'var(--typography-s-heading-sm-regular-font-family)',
      fontSize: 'var(--typography-s-heading-sm-regular-font-size)',
      lineHeight: 'var(--typography-s-heading-sm-regular-line-height)',
      fontWeight: 'var(--typography-s-heading-sm-regular-font-weight)',
    },
    bodyMD: {
      fontFamily: 'var(--typography-s-body-md-font-family)',
      fontSize: 'var(--typography-s-body-md-font-size)',
      lineHeight: 'var(--typography-s-body-md-line-height)',
      fontWeight: 'var(--typography-s-body-md-font-weight)',
    },
    bodyMDBold: {
      fontFamily: 'var(--typography-s-body-md-bold-font-family)',
      fontSize: 'var(--typography-s-body-md-bold-font-size)',
      lineHeight: 'var(--typography-s-body-md-bold-line-height)',
      fontWeight: 'var(--typography-s-body-md-bold-font-weight)',
    },
    bodySM: {
      fontFamily: 'var(--typography-s-body-sm-font-family)',
      fontSize: 'var(--typography-s-body-sm-font-size)',
      lineHeight: 'var(--typography-s-body-sm-line-height)',
      fontWeight: 'var(--typography-s-body-sm-font-weight)',
    },
    bodySMBold: {
      fontFamily: 'var(--typography-s-body-sm-bold-font-family)',
      fontSize: 'var(--typography-s-body-sm-bold-font-size)',
      lineHeight: 'var(--typography-s-body-sm-bold-line-height)',
      fontWeight: 'var(--typography-s-body-sm-bold-font-weight)',
    },
    bodyXS: {
      fontFamily: 'var(--typography-s-body-xs-font-family)',
      fontSize: 'var(--typography-s-body-xs-font-size)',
      lineHeight: 'var(--typography-s-body-xs-line-height)',
      fontWeight: 'var(--typography-s-body-xs-font-weight)',
      letterSpacing: 'var(--typography-s-body-xs-letter-spacing)',
    },
  };

  return (
    <>
      <Text as="h1" style={styles.displayMD} {...args}>
        {`S DisplayMD ${jsObject.displayMD.fontSize}px/${
          jsObject.displayMD.lineHeight
        } ${jsObject.displayMD.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="h2" style={styles.headingLG} {...args}>
        {`S HeadingLG ${jsObject.headingLG.fontSize}px/${
          jsObject.headingLG.lineHeight
        } ${jsObject.headingLG.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="h3" style={styles.headingMD} {...args}>
        {`S HeadingMD ${jsObject.headingMD.fontSize}px/${
          jsObject.headingMD.lineHeight
        } ${jsObject.headingMD.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="h4" style={styles.headingSM} {...args}>
        {`S HeadingSM ${jsObject.headingSM.fontSize}px/${
          jsObject.headingSM.lineHeight
        } ${jsObject.headingSM.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="h4" style={styles.headingSMRegular} {...args}>
        {`S HeadingSM Regular ${jsObject.headingSMRegular.fontSize}px/${
          jsObject.headingSMRegular.lineHeight
        } ${jsObject.headingSMRegular.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="p" style={styles.bodyMDBold} {...args}>
        {`S BodyMD Bold ${jsObject.bodyMDBold.fontSize}px/${
          jsObject.bodyMDBold.lineHeight
        } ${jsObject.bodyMDBold.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="p" style={styles.bodyMD} {...args}>
        {`S BodyMD ${jsObject.bodyMD.fontSize}px/${
          jsObject.bodyMD.lineHeight
        } ${jsObject.bodyMD.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="p" style={styles.bodySMBold} {...args}>
        {`S BodySM Bold ${jsObject.bodySMBold.fontSize}px/${
          jsObject.bodySMBold.lineHeight
        } ${jsObject.bodySMBold.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="p" style={styles.bodySM} {...args}>
        {`S BodySM ${jsObject.bodySM.fontSize}px/${
          jsObject.bodySM.lineHeight
        } ${jsObject.bodySM.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
      <Text as="p" style={styles.bodyXS} {...args}>
        {`S BodyXS ${jsObject.bodyXS.fontSize}px/${
          jsObject.bodyXS.lineHeight
        } ${jsObject.bodyXS.fontSize / smallScreenFontSizeBase}rem`}
      </Text>
    </>
  );
};

metamaskbot · 2022-08-04T20:50:54Z

Builds ready [494e556]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-05T20:17:24Z

Builds ready [55e063e]

storybook: Storybook
all artifacts

src/js/typography/typography.ts

metamaskbot · 2022-08-08T16:03:45Z

Builds ready [3d07ef2]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:26:32Z

Builds ready [b895255]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:43:00Z

Builds ready [e97d739]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:45:04Z

Builds ready [cecc343]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:47:31Z

Builds ready [0f97561]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:52:45Z

Builds ready [83ec5ec]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:54:51Z

Builds ready [e44aeb0]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T16:56:25Z

Builds ready [aa13648]

storybook: Storybook
all artifacts

garrettbear · 2022-08-08T17:05:03Z

Comparison JS vs CSS Tokens

Small Screen JS

Small Screen CSS

Large Screen JS

Large Screen CSS

metamaskbot · 2022-08-08T17:08:19Z

Builds ready [9e11b9b]

storybook: Storybook
all artifacts

georgewrmarshall

Looking good. Left some suggestions. I think s display MD should use bold for the font-weight. @SaraCheikh can confirm. Should we also add DEPRECATED to the CSS typography ramps just so it's clear?

src/js/typography/typography.ts

metamaskbot · 2022-08-08T22:47:55Z

Builds ready [d47323e]

storybook: Storybook
all artifacts

Replaced Heading-SM-Regular by Body-LG-Medium (#194) * Replaced Heading-SM-Regular by Body-LG-Medium * Replaced Heading-SM-Regular by Body-LG-Medium * Adding BodyLGMedium to small screen and large screen * Adding deprecated tags Co-authored-by: georgewrmarshall <george.marshall@consensys.net> DS210: Update large body text line height DS-210: update font tokens, css based typeography stories, fix mobile sDisplayMD DS-210: fix display font test DS-210: use sans font token Replaced Heading-SM-Regular by Body-LG-Medium (#194) * Replaced Heading-SM-Regular by Body-LG-Medium * Replaced Heading-SM-Regular by Body-LG-Medium * Adding BodyLGMedium to small screen and large screen * Adding deprecated tags Co-authored-by: georgewrmarshall <george.marshall@consensys.net> DS-210: Large screen font story fix DS-210: fix small display md font weight DS210: fix deprecated tags

metamaskbot · 2022-08-08T22:53:05Z

Builds ready [eacdb32]

storybook: Storybook
all artifacts

metamaskbot · 2022-08-08T22:54:28Z

Builds ready [2a96d17]

storybook: Storybook
all artifacts

garrettbear · 2022-08-08T23:01:35Z

Fixed Small Display MD to use BOLD font weight now.
Fixed deprecated tag

georgewrmarshall

LGTM! 🔥

garrettbear requested review from NidhiKJha and georgewrmarshall August 4, 2022 00:15

garrettbear requested a review from a team as a code owner August 4, 2022 00:15

georgewrmarshall previously approved these changes Aug 4, 2022

View reviewed changes

NidhiKJha previously approved these changes Aug 4, 2022

View reviewed changes

garrettbear dismissed stale reviews from NidhiKJha and georgewrmarshall via 5d9d148 August 4, 2022 20:38

georgewrmarshall reviewed Aug 4, 2022

View reviewed changes

garrettbear force-pushed the feature/ds210-update-typography-tokens branch from 5d9d148 to 494e556 Compare August 4, 2022 20:49

garrettbear linked an issue Aug 4, 2022 that may be closed by this pull request

Update Typography Line Height Tokens #210

Closed

garrettbear commented Aug 5, 2022

View reviewed changes

src/js/typography/typography.ts Outdated Show resolved Hide resolved

garrettbear force-pushed the feature/ds210-update-typography-tokens branch 2 times, most recently from 3768dad to 83ec5ec Compare August 8, 2022 16:50

garrettbear force-pushed the feature/ds210-update-typography-tokens branch from e44aeb0 to aa13648 Compare August 8, 2022 16:54

georgewrmarshall suggested changes Aug 8, 2022

View reviewed changes

src/js/typography/typography.ts Outdated Show resolved Hide resolved

georgewrmarshall assigned garrettbear Aug 8, 2022

georgewrmarshall added the code Issue related to code work label Aug 8, 2022

georgewrmarshall added the typography Tokens related to typography label Aug 8, 2022

garrettbear force-pushed the feature/ds210-update-typography-tokens branch from b5c9f63 to eacdb32 Compare August 8, 2022 22:51

Merge branch 'main' into feature/ds210-update-typography-tokens

2a96d17

georgewrmarshall approved these changes Aug 9, 2022

View reviewed changes

georgewrmarshall merged commit a44b411 into main Aug 9, 2022

georgewrmarshall deleted the feature/ds210-update-typography-tokens branch August 9, 2022 19:13

210: update typography line height tokens to be REM based #211

210: update typography line height tokens to be REM based #211

Uh oh!

Conversation

garrettbear commented Aug 4, 2022

Uh oh!

georgewrmarshall left a comment

Choose a reason for hiding this comment

Uh oh!

NidhiKJha left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Aug 4, 2022

Uh oh!

georgewrmarshall left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Aug 4, 2022

Uh oh!

metamaskbot commented Aug 5, 2022

Uh oh!

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

garrettbear commented Aug 8, 2022

Comparison JS vs CSS Tokens

Small Screen JS

Small Screen CSS

Large Screen JS

Large Screen CSS

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

georgewrmarshall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

metamaskbot commented Aug 8, 2022

Uh oh!

garrettbear commented Aug 8, 2022

Uh oh!

georgewrmarshall left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

georgewrmarshall left a comment •

edited

Loading