Skip to content

Banner: Fix types to restrict leadingVisual to variants info and upsell#8151

Open
siddharthkp wants to merge 7 commits into
mainfrom
siddharthkp/banner-leading-visual-types
Open

Banner: Fix types to restrict leadingVisual to variants info and upsell#8151
siddharthkp wants to merge 7 commits into
mainfrom
siddharthkp/banner-leading-visual-types

Conversation

@siddharthkp

@siddharthkp siddharthkp commented Jul 13, 2026

Copy link
Copy Markdown
Member

Summary

The leadingVisual and deprecated icon props on Banner are only rendered when variant is info or upsell. For every other variant (critical, success, warning), the Banner ignores a custom visual and always renders its built-in variant icon.

Until now the types allowed leadingVisual/icon to be passed alongside any variant, so this compiled even though it does nothing at runtime:

<Banner variant="critical" leadingVisual={<SomeIcon />} title="Critical" />

This PR narrows BannerProps into a discriminated union so custom visuals are only accepted for the variants that actually render them. The type surface now matches the runtime behavior.

Rollout strategy

  • Patch release (debatable but this is a type-only narrowing; runtime behavior is unchanged)
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Storybook)
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge
  • (GitHub staff only) Integration tests pass at github/github-ui

@changeset-bot

changeset-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9f72c79

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

This PR includes changesets to release 2 packages
Name Type
@primer/react Major
@primer/mcp Patch

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 added staff Author is a staff member integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. If this doesn't work, you can also use the original workflow here. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@siddharthkp siddharthkp changed the title Restrict Banner custom visual types Banner: Type custom visuals as info/upsell only Jul 13, 2026
@siddharthkp siddharthkp changed the title Banner: Type custom visuals as info/upsell only Banner: Fix types to make impossible combinations variant and leadingVisual not allowed Jul 13, 2026
@siddharthkp siddharthkp changed the title Banner: Fix types to make impossible combinations variant and leadingVisual not allowed Banner: Fix types to make ignored combinations variant and leadingVisual throw an error Jul 13, 2026
@siddharthkp siddharthkp changed the title Banner: Fix types to make ignored combinations variant and leadingVisual throw an error Banner: Fix types to restrict leadingVisual to variant=info|upsell Jul 13, 2026
@siddharthkp siddharthkp changed the title Banner: Fix types to restrict leadingVisual to variant=info|upsell Banner: Fix types to restrict leadingVisual to variant=info and upsell Jul 13, 2026
@siddharthkp siddharthkp changed the title Banner: Fix types to restrict leadingVisual to variant=info and upsell Banner: Fix types to restrict leadingVisual to variants info and upsell Jul 13, 2026
@github-actions github-actions Bot temporarily deployed to storybook-preview-8151 July 13, 2026 11:54 Inactive
@github-actions github-actions Bot requested a deployment to storybook-preview-8151 July 13, 2026 11:59 Abandoned
@github-actions github-actions Bot temporarily deployed to storybook-preview-8151 July 13, 2026 12:10 Inactive
@siddharthkp siddharthkp marked this pull request as ready for review July 13, 2026 12:43
@siddharthkp siddharthkp requested a review from a team as a code owner July 13, 2026 12:43
@siddharthkp siddharthkp requested review from Copilot and jonrohan July 13, 2026 12:43
@github-actions github-actions Bot requested a deployment to storybook-preview-8151 July 13, 2026 12:46 Abandoned

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 aligns @primer/react’s Banner TypeScript API with its existing runtime behavior by restricting custom visuals (leadingVisual and deprecated icon) to the info and upsell variants.

Changes:

  • Refactored BannerProps into a discriminated union so leadingVisual/icon are only accepted for info/upsell.
  • Updated unit tests and Storybook/Figma examples to comply with the new type restrictions (including @ts-expect-error cases to validate runtime fallback).
  • Added a changeset documenting the breaking type change.
Show a summary per file
File Description
packages/react/src/Banner/Banner.tsx Converts BannerProps to a discriminated union restricting custom visuals to info/upsell.
packages/react/src/Banner/Banner.test.tsx Adds @ts-expect-error coverage for disallowed prop combinations while asserting runtime fallback behavior.
packages/react/src/Banner/Banner.stories.tsx Adjusts Playground rendering to only pass leadingVisual for info/upsell to satisfy the new types.
packages/react/src/Banner/Banner.figma.tsx Updates Figma code-connect example to conditionally pass icon only for supported variants.
.changeset/banner-leading-visual-types.md Adds a changeset for the type-surface change (currently marked as major).

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +1 to +5
---
'@primer/react': major
---

Banner: Restrict custom leading visuals to the info and upsell variants No newline at end of file
@primer-integration

Copy link
Copy Markdown

Integration test results from github/github-ui PR:

Passed  CI   Passed
Passed  VRT   Passed
Passed  Projects   Passed

All checks passed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants