diff --git a/.changeset/banner-leading-visual-types.md b/.changeset/banner-leading-visual-types.md new file mode 100644 index 00000000000..2cbf00dadba --- /dev/null +++ b/.changeset/banner-leading-visual-types.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Banner: Restrict custom leading visuals to the info and upsell variants \ No newline at end of file diff --git a/packages/react/src/Banner/Banner.figma.tsx b/packages/react/src/Banner/Banner.figma.tsx index bc06283cdd3..e91a20af1eb 100644 --- a/packages/react/src/Banner/Banner.figma.tsx +++ b/packages/react/src/Banner/Banner.figma.tsx @@ -35,16 +35,20 @@ const componentProps = { figma.connect(Banner, 'https://www.figma.com/design/GCvY3Qv8czRgZgvl1dG6lp/Primer-Web?node-id=34303-2712&m=dev', { props: componentProps, - example: ({dismissible, variant, icon, secondaryAction, primaryAction, description, title, hideTitle}) => ( - - ), + example: ({dismissible, variant, icon, secondaryAction, primaryAction, description, title, hideTitle}) => { + // icon is only supported for the info and upsell variants + const variantAndVisualProps = variant === 'info' || variant === 'upsell' ? {variant, icon} : {variant} + + return ( + + ) + }, }) diff --git a/packages/react/src/Banner/Banner.stories.tsx b/packages/react/src/Banner/Banner.stories.tsx index 986e5d5ee5f..d7db79a8c4a 100644 --- a/packages/react/src/Banner/Banner.stories.tsx +++ b/packages/react/src/Banner/Banner.stories.tsx @@ -38,9 +38,19 @@ const iconMap = { } export const Playground: StoryObj = { - render: ({onDismiss, primaryAction, secondaryAction, leadingVisual, ...rest}) => { - // Map the string selection to the actual icon component - const leadingVisualElement = leadingVisual && iconMap[leadingVisual as keyof typeof iconMap] + render: ({ + onDismiss, + primaryAction, + secondaryAction, + leadingVisual, + variant, + icon: _deprecatedIconIgnored, + ...rest + }) => { + const variantAndVisualProps = + variant === 'info' || variant === 'upsell' + ? {variant, leadingVisual: iconMap[leadingVisual as keyof typeof iconMap]} + : {variant} return ( @@ -52,7 +62,7 @@ export const Playground: StoryObj = { secondaryAction={ secondaryAction ? {secondaryAction} : null } - leadingVisual={leadingVisualElement} + {...variantAndVisualProps} {...rest} /> @@ -65,7 +75,7 @@ export const Playground: StoryObj = { secondaryAction={ secondaryAction ? {secondaryAction} : null } - leadingVisual={leadingVisualElement} + {...variantAndVisualProps} {...rest} /> diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index 73d771ad36a..de2a76c0547 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -238,12 +238,15 @@ describe('Banner', () => { rerender(} />) expect(screen.getByTestId('icon')).toBeInTheDocument() + // @ts-expect-error unsupported variant used to verify runtime fallback behavior rerender(} />) expect(screen.queryByTestId('icon')).toBe(null) + // @ts-expect-error unsupported variant used to verify runtime fallback behavior rerender(} />) expect(screen.queryByTestId('icon')).toBe(null) + // @ts-expect-error unsupported variant used to verify runtime fallback behavior rerender(} />) expect(screen.queryByTestId('icon')).toBe(null) }) @@ -258,12 +261,15 @@ describe('Banner', () => { rerender(} />) expect(screen.getByTestId('leading-visual')).toBeInTheDocument() + // @ts-expect-error unsupported variant used to verify runtime fallback behavior rerender(} />) expect(screen.queryByTestId('leading-visual')).toBe(null) + // @ts-expect-error unsupported variant used to verify runtime fallback behavior rerender(} />) expect(screen.queryByTestId('leading-visual')).toBe(null) + // @ts-expect-error unsupported variant used to verify runtime fallback behavior rerender(} />) expect(screen.queryByTestId('leading-visual')).toBe(null) }) diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index 0fb19589f75..4f73f2fa454 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -10,6 +10,7 @@ import classes from './Banner.module.css' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' export type BannerVariant = 'critical' | 'info' | 'success' | 'upsell' | 'warning' +type BannerVariantsWithCustomVisual = 'info' | 'upsell' type BannerContextValue = { titleId: string @@ -17,7 +18,7 @@ type BannerContextValue = { const BannerContext = React.createContext(undefined) -export type BannerProps = React.ComponentPropsWithoutRef<'section'> & { +interface BannerBaseProps extends Omit, 'title'> { /** * Provide an optional label to override the default name for the Banner * landmark region @@ -41,17 +42,6 @@ export type BannerProps = React.ComponentPropsWithoutRef<'section'> & { */ hideTitle?: boolean - /** - * Provide a custom icon for the Banner. This is only available when `variant` is `info` or `upsell` - * @deprecated Use `leadingVisual` instead - */ - icon?: React.ReactNode - - /** - * Provide a custom leading visual for the Banner. This is only available when `variant` is `info` or `upsell` - */ - leadingVisual?: React.ReactNode - /** * Optionally provide a handler to be called when the banner is dismissed. * Providing this prop will show a dismiss button. @@ -74,11 +64,6 @@ export type BannerProps = React.ComponentPropsWithoutRef<'section'> & { */ title?: React.ReactNode - /** - * Specify the type of the Banner - */ - variant?: BannerVariant - /** * Specify the layout of the Banner. Compact layout will reduce the padding. */ @@ -95,7 +80,33 @@ export type BannerProps = React.ComponentPropsWithoutRef<'section'> & { flush?: boolean } -const iconForVariant: Record = { +type VariantAndLeadingVisualProps = + | { + /** + * Specify the type of the Banner, default = info + */ + variant?: BannerVariantsWithCustomVisual + + /** + * Provide a custom leading visual for the Banner. This is only available when `variant` is `info` or `upsell` + */ + leadingVisual?: React.ReactNode + + /** + * Provide a custom icon for the Banner. This is only available when `variant` is `info` or `upsell` + * @deprecated Use `leadingVisual` instead + */ + icon?: React.ReactNode + } + | { + variant: Exclude + icon?: never + leadingVisual?: never + } + +export type BannerProps = BannerBaseProps & VariantAndLeadingVisualProps + +const defaultIconForVariant: Record = { critical: , info: , success: , @@ -170,7 +181,7 @@ export const Banner = React.forwardRef(function Banner data-flush={flush ? '' : undefined} > - {visual && supportsCustomIcon ? visual : iconForVariant[variant]} + {visual && supportsCustomIcon ? visual : defaultIconForVariant[variant]}