-
Notifications
You must be signed in to change notification settings - Fork 622
chore(Token): Remove the CSS modules feature flag from the Token component #6014
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
Conversation
🦋 Changeset detectedLatest commit: 32a0353 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 |
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/376647 |
🟢 golden-jobs completed with status |
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the CSS modules feature flag from the Token component, drops the old styled-components toggles, and wires all Token subcomponents to use CSS modules via BoxWithFallback
.
- Remove all
useFeatureFlag
andtoggleStyledComponent
logic forprimer_react_css_modules_ga
. - Swap out styled-component wrappers for
BoxWithFallback
+ CSS modules classes. - Clean up TokenBase and its children (_TokenTextContainer, _RemoveTokenButton, AvatarToken, IssueLabelToken, etc.) to use native elements and CSS modules directly.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/Token/_TokenTextContainer.tsx | Replaced styled-component toggle with BoxWithFallback + CSS module class. |
src/Token/_RemoveTokenButton.tsx | Dropped styled-system variants & feature-flag logic; wrapped in BoxWithFallback . |
src/Token/TokenBase.tsx | Collapsed dual-branch into a single BoxWithFallback ; removed styled-component. |
src/Token/Token.tsx | Removed feature-flag guard and style merges; simplified markup. |
src/Token/Token.module.css | Added leading visual container styles. |
src/Token/IssueLabelToken.tsx | Removed feature-flag guard and styled-system code. |
src/Token/AvatarToken.tsx | Dropped styled-component wrapper; replaced with native <span> . |
.changeset/tame-trains-hide.md | Bump to minor and note CSS modules flag removal. |
Comments suppressed due to low confidence (2)
packages/react/src/Token/TokenBase.tsx:74
- The handler no longer invokes
onRemove
when Backspace or Delete is pressed. Reintroduce logic to callonRemove()
inside this callback when those keys are detected to preserve keyboard-removal behavior.
onKeyDown={(event: KeyboardEvent<HTMLSpanElement & HTMLAnchorElement & HTMLButtonElement>) => {
packages/react/src/Token/TokenBase.tsx:74
- New keyboard-removal logic should be covered by unit tests to ensure
onRemove
fires on Backspace/Delete. Add tests targeting this keydown behavior.
onKeyDown={(event: KeyboardEvent<HTMLSpanElement & HTMLAnchorElement & HTMLButtonElement>) => {
return ( | ||
<StyledTokenBase | ||
<BoxWithFallback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The className
and id
props are destructured but not forwarded into BoxWithFallback
, so any custom classes or identifiers on TokenBase are dropped. Add className={className}
and id={id}
to the component.
Copilot uses AI. Check for mistakes.
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
…nList.Divider component (#6022) Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com>
…nList.TrailingAction component (#6021) Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com>
Co-authored-by: Marie Lucca <[email protected]>
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
…onent (#6014) Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com> Co-authored-by: Josh Black <[email protected]> Co-authored-by: Jamie Shark <[email protected]> Co-authored-by: Marie Lucca <[email protected]>
…onent (#6014) Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com> Co-authored-by: Josh Black <[email protected]> Co-authored-by: Jamie Shark <[email protected]> Co-authored-by: Marie Lucca <[email protected]>
Closes https://github.com/github/primer/issues/4367
Changelog
New
Changed
Removed
Remove the CSS modules feature flag from the Token component
Rollout strategy
Testing & Reviewing
Merge checklist