[Brand Refactor] Multiple component + recipe updates - #1363
Conversation
🦋 Changeset detectedLatest commit: 60046ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
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 |
🔍 Design token changes foundView CSS variable changes+ --base-size-88: 5.5rem;+ --base-size-60: 3.75rem;- --brand-LogoSuite-color-control-hover: var(--base-color-scale-gray-2);
+ --brand-LogoSuite-color-control-hover: var(--base-color-scale-gray-1);- --brand-LogoSuite-color-control-hover: var(--base-color-scale-gray-2);
+ --brand-LogoSuite-color-control-hover: var(--base-color-scale-gray-1); |
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
🟢 Bundle size report
|
There was a problem hiding this comment.
Pull request overview
This PR updates several Primer Brand React components, recipes, and design tokens to better align spacing, typography, and interaction animations with the latest design direction (notably Accordion + FlexSuite overview adjustments), while extending the base sizing scale and component token coverage.
Changes:
- Updated
Accordiontoggle indicator + open/close animation to use measured content height and a single animated indicator. - Added new base size tokens (
60,88) and exposed them through spacing-enabled components (notablyBox/Stack). - Refined component visuals via token/typography/spacing tweaks (e.g.,
Tabsitem typography,Hero/Cardspacing,TextCursorAnimationcursor color token, FlexSuite overview layout updates).
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/TextCursorAnimation/TextCursorAnimation.tsx | Imports new component token CSS for cursor color modes. |
| packages/react/src/TextCursorAnimation/TextCursorAnimation.module.css | Switches cursor mix color to a dedicated token var. |
| packages/react/src/Tabs/Tabs.module.css | Updates tab item typography tokens from 100 → 200. |
| packages/react/src/Stack/Stack.module.css.d.ts | Adds typings for new 60/88 spacing classes. |
| packages/react/src/Stack/Stack.module.css | Adds gap/padding classes for base sizes 60 and 88. |
| packages/react/src/recipes/Flexsuite/Overview/FlexSuiteAIOverview.tsx | Updates FlexSuite overview layout and pricing options panel structure. |
| packages/react/src/recipes/Flexsuite/Overview/FlexSuiteAIOverview.module.css | Adds top margin to tabs panel container for spacing adjustments. |
| packages/react/src/Hero/Hero.module.css | Tweaks action/trailing spacing values. |
| packages/react/src/constants.tsx | Extends BaseSizeScale to include 60 and 88. |
| packages/react/src/Card/Card.module.css | Adjusts heading bottom spacing. |
| packages/react/src/Box/Box.module.css.d.ts | Adds typings for new 60/88 spacing classes. |
| packages/react/src/Box/Box.module.css | Adds padding/margin variants for base sizes 60 and 88. |
| packages/react/src/Accordion/Accordion.tsx | Implements new toggle SVG indicator + measured-height open/close animation logic. |
| packages/react/src/Accordion/Accordion.test.tsx | Updates tests for delayed close animation + new indicator markup. |
| packages/react/src/Accordion/Accordion.module.css.d.ts | Updates typings for new classes (closing, toggle icon, content-inner). |
| packages/react/src/Accordion/Accordion.module.css | Reworks Accordion content/toggle animations (max-height + animated lines). |
| packages/design-tokens/src/tokens/functional/components/text-cursor-animation/colors.json | Adds cursor color token (light + dark mode values). |
| packages/design-tokens/src/tokens/functional/components/logosuite/colors.json | Adjusts LogoSuite hover color token for light mode. |
| packages/design-tokens/src/tokens/base/size/size.json | Adds base size token definitions including 60 and 88. |
| packages/design-tokens/scripts/build-tokens.js | Includes TextCursorAnimation token file in color-mode build list. |
| .changeset/tidy-text-cursor-color.md | Changeset for TextCursorAnimation cursor color tokens. |
| .changeset/tidy-tabs-item-typography.md | Changeset for Tabs typography update. |
| .changeset/tidy-logosuite-control-hover.md | Changeset for LogoSuite hover color update. |
| .changeset/tidy-hero-action-spacing.md | Changeset for Hero spacing update. |
| .changeset/tidy-card-heading-spacing.md | Changeset for Card heading spacing update. |
| .changeset/tidy-accordion-toggle-animation.md | Changeset for Accordion toggle + animation updates. |
| .changeset/add-base-size-tokens.md | Changeset for adding base size 60/88 and exposing via Box/Stack. |
Copilot's findings
- Files reviewed: 24/27 changed files
- Comments generated: 1
🟢 Unit test coverage changes foundUnit test coverage has been updated through this PR. Changes: 0 new tests, 0 removed tests, 0 improved, 1 decreased
|
| export type AccordionRootProps = DetailsHTMLAttributes<HTMLDetailsElement> & { | ||
| children: ReactElement<AccordionHeadingProps | AccordionContentProps>[] | ||
| variant?: 'default' | 'emphasis' | ||
| disableAnimation?: boolean |
There was a problem hiding this comment.
We're missing the docs and storybook control for the new prop.
There was a problem hiding this comment.
Added storybook story, but we don't have public docs for Accordion so left that out. Or did you mean FAQ @danielguillan?
There was a problem hiding this comment.
Should we document this prop in FAQ, or do we expect it to always animate? We can omit it if that's the only behavior we anticipate everywhere.
| event.preventDefault() | ||
|
|
||
| if (!isClosingRef.current) { | ||
| closeWithAnimation(details) | ||
| } |
There was a problem hiding this comment.
This seems to be breaking consumer's custom onClick. I think it should run the close logic only after consumer's onClick has had a chance to call preventDefault(). For example: the Always Exactly One Panel Open story breaks, since clicking on the open panel closes it.
There was a problem hiding this comment.
Nice catch Dani, thanks for this. Yeah there was an issue, which is now fixed but how did you spot that out of interest? was it comparing the prod to preview? Just curious as this slipped through my tests and want to put some checks in place for next time.
There was a problem hiding this comment.
I spotted the issue by clicking around the Always Exactly One Panel Open. I had to compare it with the production version though because I wasn't completely confident that the behavior was correct, even though the story name is quite clear.
|
|
||
| // TODO: consider generating the scale from style dictionary and serve from the brand-primitives package | ||
| export const BaseSizeScale = [4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 64, 80, 96, 112, 128] as const | ||
| export const BaseSizeScale = [4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 60, 64, 80, 88, 96, 112, 128] as const |
There was a problem hiding this comment.
Nit: my OCD can’t unsee this. 😅
Could we add the missing values so the 4px increments are complete up to 64, and the 8px increments stay consistent after that?
| export const BaseSizeScale = [4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 60, 64, 80, 88, 96, 112, 128] as const | |
| export const BaseSizeScale = [4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 52, 56, 60, 64, 72, 80, 88, 96, 112, 128] as const |
There was a problem hiding this comment.
Yeah i definitely see the issue, but just to clarify, this would expand the current token API beyond what is immediately required. AFAIK we don't need those additional values yet, but we could create load bearing dependency on them. Not sure we should do that right now. Do you have any thoughts on that Dani? Do you feel strongly enough that the trade off is worth. it, or are you happy leaving it as a nit that we can live with for now?
There was a problem hiding this comment.
I see the concern, but I don’t think this introduces much risk in practice. These values would still be part of the same scale logic rather than arbitrary additions, and the consistent increments make the scale more predictable and useful from a systems perspective.
That said, I’m fine not introducing them now if we don’t have an immediate need. We can leave it as-is and revisit later. 👍
Summary
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=159782690
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=186264527
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185437164
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185436819
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185436530
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185436530
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185436322
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185436229
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185434044
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185432260
Resolves https://github.com/orgs/github/projects/23995/views/1?pane=issue&itemId=185430224
Lots of different updates in this PR that are clearing items in the QA board. Mostly minor spacing/sizing/type changes but some bigger changes to Accordion and base size tokens (additive).
Towards https://github.com/github/brand-marketing-design/issues/2606
List of notable changes:
--base-size-60and--base-size-88What should reviewers focus on?
Steps to test:
Contributor checklist:
update snapshotslabel to the PR)Reviewer checklist:
Screenshots:
Screen.Recording.2026-06-08.at.15.03.24.mov
Screen.Recording.2026-06-08.at.15.02.53.mov