RiverBreakoutTabs + Card layout adjustments - #1393
Conversation
🦋 Changeset detectedLatest commit: 38460fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 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 |
🟢 No design token changes found |
…ctor-qa-fixes-jul-8
There was a problem hiding this comment.
Pull request overview
Adjusts layout spacing in Card and RiverBreakoutTabs to better match the updated design requirements, primarily by tightening vertical spacing and refining wide-breakpoint padding/interaction styling.
Changes:
- Reduced
Carddescription-to-link spacing from--base-size-32to--base-size-24. - Updated
RiverBreakoutTabsspacing at wide breakpoints (outer gridline spacing and wide action spacing). - Tweaked
RiverBreakoutTabstab/accordion indicator transform and added a heading color transition.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.module.css | Adjusts wide-breakpoint spacing variables, tablist action spacing, indicator transform, and heading transition. |
| packages/react/src/Card/Card.module.css | Reduces description bottom margin to tighten description-to-link spacing. |
| .changeset/river-breakout-tabs-wide-action-spacing.md | Adds a patch changeset documenting the RiverBreakoutTabs updates. |
| .changeset/card-description-link-spacing.md | Adds a patch changeset documenting the Card spacing change. |
Review details
- Files reviewed: 4/74 changed files
- Comments generated: 4
- Review effort level: Low
🟢 Bundle size report
|
🟢 No unit test coverage changes foundAll components and hooks with tests maintain the same coverage as the main branch. |
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
| --brand-RiverBreakout-variant-gridline-spacing-outerInline: var(--base-size-60); | ||
| --brand-RiverBreakout-variant-gridline-spacing-outerBlock: var(--base-size-60); |
There was a problem hiding this comment.
Can we move these updated values to design tokens please?
There was a problem hiding this comment.
@danielguillan are you looking to update the existing token values, which will impact multiple components or add new ones for RiverBreakoutTabs?
I'd rather avoid scope creeping by changing RiverBreakout right now. Adding an entire component token file just for two overrides feels a bit heavy handed, though I do see your argument for hoisting these. Can you clarify the intent first please.
There was a problem hiding this comment.
These are already defined and scoped to RiverBreakout so these lines are only overriding the existing tokens rather than defining new ones, right? Or maybe I'm missing something, sorry.
There was a problem hiding this comment.
noting for posterity that we resolved this in DMs. we'll leave hoisting these as the tokens touch 2 different river breakout components.
danielguillan
left a comment
danielguillan
left a comment
There was a problem hiding this comment.
Looks good. We just need to transfer some updated values for RiverBreakout to the design tokens file.
Summary
Resolves https://github.com/orgs/github/projects/23995/views/5?pane=issue&itemId=196247289
Resolves https://github.com/github/brand-experience/issues/387
Part of https://github.com/github/brand-experience/issues/289
List of notable changes:
What should reviewers focus on?
Steps to test:
Contributor checklist:
update snapshotslabel to the PR)Reviewer checklist:
Screenshots: