Skip to content

Fix river breakout tabs custom background - #1375

Merged
rezrah merged 5 commits into
mainfrom
rezrah/riverbreakout-tabs-ref-bug
Jun 18, 2026
Merged

Fix river breakout tabs custom background #1375
rezrah merged 5 commits into
mainfrom
rezrah/riverbreakout-tabs-ref-bug

Conversation

@rezrah

@rezrah rezrah commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Resolves https://github.com/github/brand-experience/issues/228

Previously the ref-only approach we recommended had runtime issues. User needed to traverse the dom until they matched the visual child, and this would unmount on tab changes.

Now, a new backgroundVisual prop on the root renders the custom backdrop once. First class API. No refs needed. No unmounting issue.

Also updated the design slightly, because previous version was incorrectly using a full-bleed dither background.

🔗 Preview

List of notable changes:

  • Added first-class API to resolve the original issue
  • Fixed some design issues, like not going full-bleed with dither, now matches designs
  • Fixed a minor issue in Hero trailingComponents (unrelated to the main change)

What should reviewers focus on?

  • Check the overall effect matches the intended design

Steps to test:

  1. Go through the examples in this storybook link for river breakout tabs
  2. Code review

Contributor checklist:

  • All new and existing CI checks pass
  • Tests prove that the feature works and covers both happy and unhappy paths
  • Any drop in coverage, breaking changes or regressions have been documented above
  • UI Changes contain new visual snapshots (generated by adding update snapshots label to the PR)
  • All developer debugging and non-functional logging has been removed
  • Related issues have been referenced in the PR description

Reviewer checklist:

  • Check that pull request and proposed changes adhere to our contribution guidelines and code of conduct
  • Check that tests prove the feature works and covers both happy and unhappy paths
  • Check that there aren't other open Pull Requests for the same update/change

Screenshots:

Please try to provide before and after screenshots or videos

Before After
Screenshot 2026-06-16 at 12-12-49 components-riverbreakouttabs-examples--with-images Screenshot 2026-06-16 at 12-12-25 components-riverbreakouttabs-examples--with-images

Copilot AI review requested due to automatic review settings June 15, 2026 14:51
@rezrah
rezrah requested a review from a team as a code owner June 15, 2026 14:51
@changeset-bot

changeset-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cf9c042

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@primer/react-brand Minor
@primer/brand-docs Minor
@primer/brand-css Minor
@primer/brand-primitives Minor
@primer/brand-e2e Minor
@primer/brand-fonts Minor
@primer/brand-config Minor
@primer/brand-storybook Minor

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

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🟢 No design token changes found

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🟢 Bundle size report

CheckMainBranchChange
UMD — full bundle (JS)99.28 kB99.35 kB⬆️ +65 B (+0.1%)
UMD — full bundle (CSS)65.32 kB65.46 kB⬆️ +148 B (+0.2%)
ESM — full bundle (JS + CSS)1.49 MB1.49 MB⬆️ +1013 B (+0.1%)
ESM — tree-shaken simple (Button)68.20 kB68.33 kB⬆️ +130 B (+0.2%)
ESM — tree-shaken complex (ActionMenu)76.98 kB77.11 kB⬆️ +130 B (+0.2%)

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🟢 Unit test coverage changes found

Unit test coverage has been updated through this PR.

Changes: 0 new tests, 0 removed tests, 1 improved, 0 decreased

Component/Hook Statements Functions Branches Change
RiverBreakoutTabs 87.8% 88.2% 81.3% 83.0% +1.7%

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a first-class API to RiverBreakoutTabs for rendering a persistent decorative background behind the shared visual region, avoiding the prior ref/DOM-traversal approach and preventing unmounting across tab changes.

Changes:

  • Added backgroundVisual and imagePosition props to RiverBreakoutTabs, with corresponding rendering + styling changes for both wide (tabs) and narrow (accordion) layouts.
  • Added unit tests covering the new props in the wide (tab) layout.
  • Updated Storybook examples, docs, and a changeset to reflect the new API (plus a small Hero story styling tweak).
Show a summary per file
File Description
packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.tsx Adds backgroundVisual + imagePosition props and renders a background layer behind shared visuals.
packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.test.tsx Adds tests asserting backgroundVisual renders once and modifier classes apply (wide layout).
packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.module.css Adds layering/padding rules and new modifier classes for background + image positioning.
packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.module.css.d.ts Updates generated CSS module typings for the new class names.
packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.examples.stories.tsx Migrates examples from DOM-based background injection to the new backgroundVisual prop.
packages/react/src/Hero/Hero.examples.stories.tsx Adjusts trailing component text styling (variant="muted").
apps/next-docs/content/components/RiverBreakoutTabs/index.mdx Documents new props in the component props table.
.changeset/river-breakout-tabs-background-visual.md Publishes the new props as a minor change with usage snippet.

Copilot's findings

  • Files reviewed: 7/12 changed files
  • Comments generated: 4

Comment thread packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.tsx
Comment thread packages/react/src/river/RiverBreakoutTabs/RiverBreakoutTabs.tsx
Comment thread apps/next-docs/content/components/RiverBreakoutTabs/index.mdx Outdated
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🟢 No visual differences found

Our visual comparison tests did not find any differences in the UI.

@danielguillan danielguillan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the right solution, simple and straightforward. Great work @rezrah!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@jesussandreas Just double checking that the paddings around the visual are correct and also, if there are use cases where we might want to keep the option to make the background span the full width of the page.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@danielguillan yeah, they look okay too me for the most part. those are very UI dependent as well anyways.

@rezrah
rezrah requested a review from jesussandreas June 16, 2026 11:14
@jesussandreas

Copy link
Copy Markdown
Collaborator

small change here big dawg.

the spacing between heading, you have it set at 20px.

its only 20px bottom of the icon. the spacing between heading and paragraph should be 12px.

and the link spacing top is set at 12px which is correct but in the design i have bake in 4px~, so lets set that to 16px.

Screenshot 2026-06-16 at 10 27 38 AM

@jesussandreas jesussandreas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

beautiful

@rezrah
rezrah merged commit 67fff83 into main Jun 18, 2026
17 checks passed
@rezrah
rezrah deleted the rezrah/riverbreakout-tabs-ref-bug branch June 18, 2026 23:49
@primer primer Bot mentioned this pull request Jun 17, 2026
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.

4 participants