fix(TokenTextContainer): Update the line-height to use a primer primitives CSS variable#7746
fix(TokenTextContainer): Update the line-height to use a primer primitives CSS variable#7746
Conversation
🦋 Changeset detectedLatest commit: 2fea2c4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Uh oh! @jonrohan, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:
Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
|
Update TokenTextContainer `line-height` to use primer primitives CSS variable `var(--base-text-lineHeight-normal)`
|
I'm skipping integration tests because this is just a CSS change |
There was a problem hiding this comment.
Pull request overview
Updates TokenTextContainer’s line-height to use a Primer primitives CSS custom property instead of the literal normal value, aligning behavior with the prior styled-system mapping.
Changes:
- Replace
line-height: normal;withline-height: var(--base-text-lineHeight-normal);in the Token text container CSS. - Remove the
stylelint-disable-next-line primer/typographysuppression that was only needed for the rawnormalvalue.
Show a summary per file
| File | Description |
|---|---|
packages/react/src/Token/_TokenTextContainer.module.css |
Switches line-height to a primitives-backed CSS variable for consistent typography behavior. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
While looking at https://github.com/github/github-ui/pull/18258 I found that the TokenTextContainer was using
line-height: normal;I think this might have been a bug when we converted the component from styled components. As the previous styled systemlineHeight: normalwould have converted to the "normal" value of the css variable.I'm fixing by using the CSS variable
var(--base-text-lineHeight-normal);Changelog
New
Changed
Fixing TokenTextContainer line-height to use primer primitives
Removed
Rollout strategy
Testing & Reviewing
I'm not expecting this to make too much of a difference in prod, the bug I noticed was very subtle line height shift. Here's a gif toggling between the change:
Merge checklist