From 45cac0538c9becce40d2af5dcf475c832d705037 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 12 Aug 2024 10:10:06 -0500 Subject: [PATCH 1/4] Update Banner.tsx --- packages/react/src/Banner/Banner.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index ffc72811a9e..61ab62f3cc7 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -99,6 +99,7 @@ export const Banner = React.forwardRef(function Banner const hasActions = primaryAction || secondaryAction const bannerRef = React.useRef(null) const ref = useMergedRefs(forwardRef, bannerRef) + const supportsCustomIcon = variant === 'info' || variant === 'upsell' if (__DEV__) { // This hook is called consistently depending on the environment @@ -134,7 +135,7 @@ export const Banner = React.forwardRef(function Banner ref={ref} > -
{icon && variant === 'info' ? icon : iconForVariant[variant]}
+
{icon && supportsCustomIcon ? icon : iconForVariant[variant]}
{title ? ( From b174f4e323d75a568c773491986dccced2c86f72 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 12 Aug 2024 11:45:28 -0500 Subject: [PATCH 2/4] feat(Banner): add support for custom icons when variant="upsell" --- packages/react/src/Banner/Banner.docs.json | 4 ++-- .../src/Banner/Banner.features.stories.tsx | 13 ++++++++++++ packages/react/src/Banner/Banner.test.tsx | 20 +++++++++++++++++++ packages/react/src/Banner/Banner.tsx | 3 +-- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/react/src/Banner/Banner.docs.json b/packages/react/src/Banner/Banner.docs.json index eb682524e0a..e17bbe3b4bd 100644 --- a/packages/react/src/Banner/Banner.docs.json +++ b/packages/react/src/Banner/Banner.docs.json @@ -18,13 +18,13 @@ }, { "name": "hideTitle", - "type": "boolean", + "type": "boolean", "description": "Whether to hide the title visually." }, { "name": "icon", "type": "React.ReactNode", - "description": "Provide an icon for the banner" + "description": "Provide a custom icon for the Banner. This is only available when `variant` is `info` or `upsell`" }, { "name": "onDismiss", diff --git a/packages/react/src/Banner/Banner.features.stories.tsx b/packages/react/src/Banner/Banner.features.stories.tsx index 3c3bbe243fa..04f98d78948 100644 --- a/packages/react/src/Banner/Banner.features.stories.tsx +++ b/packages/react/src/Banner/Banner.features.stories.tsx @@ -1,4 +1,5 @@ import React from 'react' +import {CopilotIcon} from '@primer/octicons-react' import {action} from '@storybook/addon-actions' import type {Meta} from '@storybook/react' import {Banner} from '../Banner' @@ -202,3 +203,15 @@ export const WithActions = () => { /> ) } + +export const CustomIcon = () => { + return ( + } + onDismiss={action('onDismiss')} + variant="upsell" + /> + ) +} diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index 0664cc0c692..19abf12255c 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -180,6 +180,26 @@ describe('Banner', () => { expect(screen.queryByTestId('icon')).toBe(null) }) + it('should support a custom icon only for upsell variants', () => { + const CustomIcon = jest.fn(() => ) + const {rerender} = render( + } />, + ) + expect(screen.getByTestId('icon')).toBeInTheDocument() + + rerender(} />) + expect(screen.queryByTestId('icon')).toBe(null) + + rerender(} />) + expect(screen.queryByTestId('icon')).toBe(null) + + rerender(} />) + expect(screen.queryByTestId('icon')).toBe(null) + + rerender(} />) + expect(screen.queryByTestId('icon')).toBe(null) + }) + describe('Banner.Title', () => { it('should render as a h2 element by default', () => { render( diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index 61ab62f3cc7..ca8fc65a044 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -28,8 +28,7 @@ export type BannerProps = React.ComponentPropsWithoutRef<'section'> & { hideTitle?: boolean /** - * Provide an icon for the banner. - * Note: Only `variant="info"` banners should use custom icons + * Provide a custom icon for the Banner. This is only available when `variant` is `info` or `upsell` */ icon?: React.ReactNode From 5a940f1378cabb0c310cc0a5b16eb4ff1156ccdf Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 12 Aug 2024 11:46:07 -0500 Subject: [PATCH 3/4] chore: add changeset --- .changeset/twelve-tables-leave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/twelve-tables-leave.md diff --git a/.changeset/twelve-tables-leave.md b/.changeset/twelve-tables-leave.md new file mode 100644 index 00000000000..718952e665b --- /dev/null +++ b/.changeset/twelve-tables-leave.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add support for custom icons when a Banner is variant="upsell" From ede98f8178e2dc8c0d5faa3a0ced3d8b1d5c453c Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 12 Aug 2024 13:13:33 -0500 Subject: [PATCH 4/4] test(Banner): update test for info and upsell custom icons --- packages/react/src/Banner/Banner.test.tsx | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index 19abf12255c..d0c99438b82 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -160,31 +160,14 @@ describe('Banner', () => { expect(container.firstChild).toHaveAttribute('data-testid', 'test') }) - it('should support a custom icon only for info variants', () => { + it('should support a custom icon for info and upsell variants', () => { const CustomIcon = jest.fn(() => ) const {rerender} = render( } />, ) expect(screen.getByTestId('icon')).toBeInTheDocument() - rerender(} />) - expect(screen.queryByTestId('icon')).toBe(null) - - rerender(} />) - expect(screen.queryByTestId('icon')).toBe(null) - rerender(} />) - expect(screen.queryByTestId('icon')).toBe(null) - - rerender(} />) - expect(screen.queryByTestId('icon')).toBe(null) - }) - - it('should support a custom icon only for upsell variants', () => { - const CustomIcon = jest.fn(() => ) - const {rerender} = render( - } />, - ) expect(screen.getByTestId('icon')).toBeInTheDocument() rerender(} />) @@ -193,9 +176,6 @@ describe('Banner', () => { rerender(} />) expect(screen.queryByTestId('icon')).toBe(null) - rerender(} />) - expect(screen.queryByTestId('icon')).toBe(null) - rerender(} />) expect(screen.queryByTestId('icon')).toBe(null) })