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.

Sign up for GitHub

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

Jump to bottom

Add new text size scale, deprecate old sizes and Heading #4113

Merged
merged 5 commits into from
Sep 2, 2022
Merged

Add new text size scale, deprecate old sizes and Heading #4113

merged 5 commits into from
Sep 2, 2022

Conversation

markdalgleish
Copy link
Contributor

@markdalgleish markdalgleish commented Sep 1, 2022 •
edited
Loading

Fixes RNBW-4399
Figma link: https://www.figma.com/file/wFXhL8ZcaBnIlWSSQTLWPA/RDS-Core?node-id=12%3A222

What changed (plus any additional context for devs)

New standard text sizes are now available on the design system Text component to align with new standards from Figma. Note that line height variants form part of the size scale.

  • 11pt
  • 12pt
  • 13pt, 13pt / 135%, 13pt / 150%
  • 15pt, 15pt / 135%, 15pt / 150%
  • 17pt, 17pt / 135%, 17pt / 150%
  • 20pt, 20pt / 135%, 20pt / 150%
  • 22pt
  • 26pt
  • 30pt
  • 34pt
  • 44pt

All new heading sizes are now provided via Text. The Heading component is now marked as deprecated and will be removed in a future PR.

Default size values for Text (16px) and Heading (20px) have been removed since the previous defaults have been deprecated.

To make future migration work easier, the default weight value for Heading has been removed, forcing all usages to inline the default value of "heavy".

Heading/Text size migration guide

All existing sizes have been renamed to make it very clear that they shouldn't be used going forwards while maintaining support for existing usages. To align with the new naming convention, the old sizes also include their line height values.

Heading migration guide

Note that weight no longer has a default value. If no weight was defined, it should be "heavy".

  • <Heading> -> <Heading size="20px / 22px (Deprecated)" weight="heavy">
  • <Heading size="18px"> -> <Heading size="18px / 21px (Deprecated)" weight="heavy">
  • <Heading size="20px"> -> <Heading size="20px / 22px (Deprecated)" weight="heavy">
  • <Heading size="23px"> -> <Heading size="23px / 27px (Deprecated)" weight="heavy">
  • <Heading size="26px"> -> <Heading size="26px / 30px (Deprecated)" weight="heavy">
  • <Heading size="28px"> -> <Heading size="28px / 33px (Deprecated)" weight="heavy">
  • <Heading size="30px"> -> <Heading size="30px / 34px (Deprecated)" weight="heavy">
  • <Heading size="34px"> -> <Heading size="34px / 41px (Deprecated)" weight="heavy">
  • <Heading size="44px"> -> <Heading size="44px / 53px (Deprecated)" weight="heavy">

Text migration guide

  • <Text> -> <Text size="16px / 22px (Deprecated)">
  • <Text size="11px"> -> <Text size="11px / 13px (Deprecated)">
  • <Text size="12px"> -> <Text size="12px / 14px (Deprecated)">
  • <Text size="14px"> -> <Text size="14px / 19px (Deprecated)">
  • <Text size="15px"> -> <Text size="15px / 21px (Deprecated)">
  • <Text size="16px"> -> <Text size="16px / 22px (Deprecated)">
  • <Text size="18px"> -> <Text size="18px / 27px (Deprecated)">
  • <Text size="20px"> -> <Text size="20px / 24px (Deprecated)">
  • <Text size="23px"> -> <Text size="23px / 27px (Deprecated)">

What to test

Check that text is still rendering correctly in the app. TypeScript is doing most of the heavy lifting here, but any changes to JS files require a bit of extra scrutiny. The list of deprecated text sizes above is helpful when checking the diff, but I've also added some additional code to throw an error in dev mode when the requested text size is missing so it should get picked up in e2e tests.

Final checklist

  • Assigned individual reviewers?
  • Added labels? (team1/team2, critical path, release, dev QA)
  • Did you test both iOS and Android?
  • If your changes are visual, did you check both the light and dark themes?
  • Added e2e tests? If not, please specify why
  • If you added new critical path files, did you update the CODEOWNERS file?
  • If no dev QA label, did you add the PR to the QA Queue?

@markdalgleish markdalgleish added design system team1 dev QA PR that can be safely QA'd by devs for merge (feature-specific limited impact). QA during regression labels Sep 1, 2022
@linear
Copy link

linear bot commented Sep 1, 2022

RNBW-4399 Add new type scale, deprecate old values

The existing values will be left in the system so that we don't have to migrate everything at once, but marked as deprecated so that they won't be used in future work.

Spec: https://www.figma.com/file/wFXhL8ZcaBnIlWSSQTLWPA/RDS-Core?node-id=12%3A222

jxom
jxom reviewed Sep 1, 2022
View reviewed changes
src/components/ens-profile/RecordTags/RecordTags.tsx
@@ -81,17 +82,24 @@ export default function RecordTags({
);
}

const tagSizes = selectTextSizes(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@jxom jxom self-requested a review September 1, 2022 01:31
jxom
jxom reviewed Sep 1, 2022
View reviewed changes
src/design-system/typography/typeHierarchy.ts
@@ -1,6 +1,36 @@
/**
* ⚠️ IMPORTANT NOTE ⚠️
Copy link
Contributor

@jxom jxom Sep 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add a link to the Figma spec here so devs can visually reference what is in this file?

I guess they could just open up the docs though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how set in stone the Figma URL is, so I might come back to this one.

estrattonbailey
estrattonbailey approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful 💪

src/design-system/components/Text/useTextStyle.ts Show resolved Hide resolved
@jxom jxom self-requested a review September 1, 2022 21:00
@markdalgleish markdalgleish merged commit ea8194c into develop Sep 2, 2022
@markdalgleish markdalgleish deleted the @markdalgleish/RNBW-4399-new-type-scale branch September 2, 2022 00:20
estrattonbailey added a commit that referenced this pull request Sep 6, 2022
@estrattonbailey
Merge branch 'develop' of github.com:rainbow-me/rainbow into eric/rnb…
1971e2e 
…w-4378-clean-up-global-and-env-vars-in-app-move

* 'develop' of github.com:rainbow-me/rainbow: (40 commits)
  fix: quiet logs when linting in CI (#4070)
  analytics: add tracking for screen dimensions + fix nav events on android (#4088)
  fix native input cursor color (#4112)
  Profiles: use `name` text record instead of `me.rainbow.displayName` (#4046)
  chore: upgrade eslint-config-rainbow (#4118)
  backup PIN  (#4012)
  Remove long press on address in switching wallets (#3990)
  Version bump iOS v.1.7.4 & Android 144 (#4117)
  Fix slow image loading (#4052)
  Add new text size scale, deprecate old sizes and Heading (#4113)
  Disable campaings in e2e and fix testID of PromoSheet (#4107)
  🏛 chore: prettier everything (#4100)
  fix settings StatusIcon shadows on android (#4104)
  fix clear async storage crash (#4105)
  fix RNBW-4416 (#4103)
  🏛 Update to React Query 4 (#4060)
  swaps: init cross chain feature flag (#4091)
  enable arbiitrum swap details test (#4092)
  remove unsafe access to networkInfo (#4096)
  network check (#4094)
  ...
estrattonbailey added a commit that referenced this pull request Sep 6, 2022
@estrattonbailey
Merge branch 'develop' of github.com:rainbow-me/rainbow into eric/rnb…
b4c418e 
…w-4388-configure-jest-for-unit-tests

* 'develop' of github.com:rainbow-me/rainbow:
  Profiles: Support contenthash (#4067)
  🏛 GraphQL Clients/Codegen Foundations (#4086)
  fix: quiet logs when linting in CI (#4070)
  analytics: add tracking for screen dimensions + fix nav events on android (#4088)
  fix native input cursor color (#4112)
  Profiles: use `name` text record instead of `me.rainbow.displayName` (#4046)
  chore: upgrade eslint-config-rainbow (#4118)
  backup PIN  (#4012)
  Remove long press on address in switching wallets (#3990)
  Version bump iOS v.1.7.4 & Android 144 (#4117)
  Fix slow image loading (#4052)
  Add new text size scale, deprecate old sizes and Heading (#4113)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estrattonbailey estrattonbailey estrattonbailey approved these changes

@jxom jxom jxom approved these changes

@nicoglennon nicoglennon Awaiting requested review from nicoglennon

Assignees
No one assigned
Labels
design system dev QA PR that can be safely QA'd by devs for merge (feature-specific limited impact). QA during regression team1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@markdalgleish @estrattonbailey @jxom