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(Heading)!: remove deprecated props and refactor usages #1872

Merged
merged 4 commits into from
Mar 1, 2024

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Mar 1, 2024

Summary:

  • remove size prop (in favor of as)

size, marked as deprecated before, is now removed from Heading. Instead of size, you can now use combinations of as and preset to get access to both default and custom typography presets. Here's the mapping for the <h*> tags to the preset:

size => as default preset
h1 headline-lg
h2 headline-md
h3 headline-sm
h4 title-md
h5 title-sm
h6 title-xs
h7 (this has been removed entirely)

If you used any of the size values that map to the HTML heading tags, you can update the prop name from size to as. If preset is specified, and it matches the default value from the table above, you can remove preset.

If you used any of the size values that map to existing presets, you can update the prop name from size to preset. Before, the component would convert the size value to a relevant default as tag. You can use the inverse of the table (reading right to left) to determine which as tag to use if one is not provided.

NOTE: as now has a default value of "h1". You will get accessibility errors if as is not provided in the latter cases.

  • remove variant prop (in favor of utility classes)

The variant prop did some of the work of a utility class, by mapping key values to color token usages. However, these named variants didn't match the token naming scheme, and did not provide a set of values that spanned the full set of possible use cases. With this change, we remove this prop, allowing Heading to inherit color like normal. Internal components that need to specify a color can now do so in component styles, and this makes it easier to customize the color of any heading using component or utility class values.

variant EDS Token Used
error --eds-theme-color-text-utility-error
brand --eds-theme-color-text-brand-default
info --eds-color-info-700
inherit none (this is the default behavior)
neutral-subtle --eds-theme-color-text-neutral-subtle
neutral-medium --eds-theme-color-text-neutral-default
neutral-strong --eds-theme-color-text-neutral-strong
success --eds-theme-color-text-utility-success
warning --eds-theme-color-text-utility-warning
white --eds-theme-color-text-neutral-default-inverse`

When removing variant you can use the table above to refactor to the desired utility class, token, or custom class. Refer to this document for the names of tailwind classes to convert to if used.

Examples

<Heading ... variant="error"> =>:

  • <Heading ... className="text-utility-error">
  • <div className="text-utility-error"><Heading ...></div>

NOTE: variant="info" used a tier-1 token directly, which is a violation. Since this is no longer provided internally, this is a reminder that only tier 2 and above tokens should be used in styling and design.

  • refactor usages in composed components

Some composed components used or borrowed from the Heading component API. You will see no changes in the following components' usages:

  • PageLevelBanner
  • Section (to be deprecated)
  • Tabs

Heading and Modal

Modal has one change to its API in support of this cleanup. Instead of specifying size like before, we now use preset (which comes from Heading). By default, Modal instances use h2 for the level in their titles. Again, you can reference the table above under Heading for how to update the prop value once moving over to preset.

  • update documentation and stories

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.

- remove `size` prop (in favor of `as`)

Instructions TBD

- remove `variant` prop (in favor of utility classes)

Instructions TBD

- refactor usages in composed components

Some composed components used or borrowed from the Heading component
API:

* List
* Of
* Components
* TBD

Instructions TBD

- update documentation and stories
Copy link

codecov bot commented Mar 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.88%. Comparing base (552945a) to head (3ab76a2).

Additional details and impacted files
@@            Coverage Diff             @@
##              v14    #1872      +/-   ##
==========================================
+ Coverage   96.85%   96.88%   +0.03%     
==========================================
  Files         108      108              
  Lines        2128     2122       -6     
  Branches      554      550       -4     
==========================================
- Hits         2061     2056       -5     
+ Misses         65       64       -1     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@booc0mtaco booc0mtaco marked this pull request as ready for review March 1, 2024 17:06
@booc0mtaco booc0mtaco requested a review from a team March 1, 2024 20:22
* Given a certain HeadingElement, what is the default preset to use?
*/
const headingPresetMap: Record<HeadingElement, Preset> = {
h1: 'headline-lg',
Copy link
Contributor

@ahuth ahuth Mar 1, 2024

Choose a reason for hiding this comment

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

Idea:

When using the component, it feels like I'd rather write this:

<Heading as="h1" preset="h2" />

so that I'm describing a preset relative to other headings.

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 think I follow and that could work. The intent was to allow preset to always match the typography name from design to start, so that engineers don't have to reason about which level means what preset.

I plan to host a workshop for the migration to 14, and can see how people think thru what to use

Copy link
Contributor

@ahuth ahuth left a comment

Choose a reason for hiding this comment

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

LGTM

@booc0mtaco booc0mtaco merged commit a5d6f76 into v14 Mar 1, 2024
8 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EFI-1645/Heading branch March 1, 2024 21:56
@booc0mtaco booc0mtaco mentioned this pull request Mar 5, 2024
booc0mtaco added a commit that referenced this pull request Mar 5, 2024
## [14.0.0](v13.12.0...v14.0.0) (2024-03-05)

[Storybook](https://61313967cde49b003ae2a860-pjuqfmockf.chromatic.com/)

### ⚠ BREAKING CHANGES

* remove Grid and Section from exports (#1876)
* **Text:** remove deprecated props (#1873)
* **Heading:** remove deprecated props and refactor usages (#1872)
* **Link:** remove and reset deprecated props (#1871)
* **Select:** remove deprecated props (#1870)
* **Label:** remove deprecated props (#1868)
* **InputField:** remove unused prop requiredLabel (#1869)
* **InlineNotification:** remove deprecated props from component (#1867)
* **Card:** deprecate orientation prop (#1866)
* **Badge:** deprecate name prop on component (#1865)
* **Skeleton:** deprecate .Rect subcomponent mapping (#1864)
* **Tag:** remove deprecated props and update API (#1862)
* **Tooltip:** remove deprecated props (#1861)
* **Tabs:** combine Tab sub-component into Tabs
* remove deprecated components
* output a single, cjs build

### Features

* **Badge:** deprecate name prop on component ([#1865](#1865)) ([cff5d5e](cff5d5e))
* **Card:** deprecate orientation prop ([#1866](#1866)) ([bb160c1](bb160c1))
* **Heading:** remove deprecated props and refactor usages ([#1872](#1872)) ([a5d6f76](a5d6f76))
* **InlineNotification:** remove deprecated props from component ([#1867](#1867)) ([efc6b43](efc6b43))
* **InputField:** remove unused prop requiredLabel ([#1869](#1869)) ([f9d73dd](f9d73dd))
* **Label:** remove deprecated props ([#1868](#1868)) ([907cc39](907cc39))
* **Link:** remove and reset deprecated props ([#1871](#1871)) ([552945a](552945a))
* remove deprecated components ([2538aa6](2538aa6))
* remove Grid and Section from exports ([#1876](#1876)) ([a0ec79a](a0ec79a))
* **Select:** remove deprecated props ([#1870](#1870)) ([e0ad463](e0ad463))
* **Skeleton:** deprecate .Rect subcomponent mapping ([#1864](#1864)) ([9d3ffec](9d3ffec))
* **Tabs:** combine Tab sub-component into Tabs ([0479863](0479863))
* **Tag:** remove deprecated props and update API ([#1862](#1862)) ([dbfbaa3](dbfbaa3))
* **Text:** remove deprecated props ([#1873](#1873)) ([13fbc18](13fbc18))
* **Tooltip:** remove deprecated props ([#1861](#1861)) ([d444607](d444607))


### Bug Fixes

* output a single, cjs build ([523130e](523130e))
* remove unnecessary type definitions script ([be341c0](be341c0))
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