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: allow typography components to specify any preset values #1794

Merged
merged 3 commits into from
Oct 31, 2023

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Oct 30, 2023

Summary:

Components involved:

  • Heading
  • Text

Work:

  • create a new prop for the relevant type presets (tier 2+)
  • mark the previously-defined props as deprecated
  • update examples to apply these props in code examples and stories (instead of the previous examples)
  • update/remove relevant snapshots

Some clear benefits to this approach:

  • makes it so that the typography prop values match those in the requisite designs
  • makes it so that the typography prop values match the token names in code
  • avoids the complexity of baking in the color tokens as variant prop values
  • simplifies components and snapshots (once the deprecated props are removed)

Validation steps:

  • applying the code to allow this new prop should not cause any snapshot changes
  • swap any existing token usages with the new preset-based variants does not cause a visual snapshot change

Test Plan:

  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Manually tested my changes, and here are the details:
    • Create an alpha publish and try out in edu-stack or traject as a sanity check if changes affect build or deploy, or are breaking, such as token changes, widely used component updates, hooks changes, and major dependency upgrades.

- support all the tier 2 and tier 3 typography tokens
- mark other ways to modify type as deprecated (but kept)
- add utility types to support typography types
@github-actions
Copy link

github-actions bot commented Oct 30, 2023

size-limit report 📦

Path Size
components 97.81 KB (+0.01% 🔺)
styles 34.33 KB (+4.02% 🔺)

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Merging #1794 (fcbbddd) into next (2d6d7c3) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             next    #1794      +/-   ##
==========================================
+ Coverage   92.56%   92.57%   +0.01%     
==========================================
  Files         147      147              
  Lines        2731     2735       +4     
  Branches      709      713       +4     
==========================================
+ Hits         2528     2532       +4     
  Misses        202      202              
  Partials        1        1              
Files Coverage Δ
src/components/Heading/Heading.tsx 94.73% <100.00%> (+0.61%) ⬆️
src/components/Text/Text.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@booc0mtaco
Copy link
Contributor Author

ex: Heading
Screenshot 2023-10-31 at 10 46 20

- generate simpler stories for the typography components
- update corresponding snapshots
- add relevant TODO comments for future review
@booc0mtaco
Copy link
Contributor Author

Before with Heading you had to try and grok how to map what was requested in design with what the code was using:

  • sometimes the names would match between the two, and other times you'd have to try and figure out what token h1 meant
  • some of the variant tokens matched existing color tokens, and other times they did not
  • one could theoretically set a variant token, then use a tailwind utility or other class to override the token, which bifurcates the API

Screenshot 2023-10-31 at 10 55 46

@booc0mtaco booc0mtaco requested a review from a team October 31, 2023 15:59
@booc0mtaco booc0mtaco marked this pull request as ready for review October 31, 2023 15:59
Copy link
Contributor

@timzchang timzchang left a comment

Choose a reason for hiding this comment

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

<3

@booc0mtaco
Copy link
Contributor Author

doing a merge commit so that we get two lines in the release notes, one for each component modified (and the docs PR doesn't increment the version on its own)

@booc0mtaco booc0mtaco merged commit 55e0d09 into next Oct 31, 2023
6 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EFI-1439 branch October 31, 2023 19:18
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