From 6c9121e217e4815c7c6ab3c03474b2ca632f05e9 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 17 Sep 2024 17:11:22 -0500 Subject: [PATCH] feat(Banner): update Banner to use CSS Modules behind feature flag (#4913) * feat(Banner): update Banner to use CSS Modules behind feature flag * chore: add changeset * chore: remove autogenerated pnpm packageManager change * chore: fix stylelint errors * chore: add back in underline for fallback case * refactor: update autofix and eslint warning * chore: update class order for tests * chore: fix stylelint errors * Update packages/react/src/Banner/Banner.module.css Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com> --------- Co-authored-by: Josh Black Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com> --- .changeset/odd-rings-applaud.md | 5 + e2e/components/Banner.test.ts | 57 +++ packages/react/src/Banner/Banner.docs.json | 5 + packages/react/src/Banner/Banner.module.css | 196 ++++++++++ packages/react/src/Banner/Banner.test.tsx | 5 + packages/react/src/Banner/Banner.tsx | 339 +++++++++++------- packages/react/src/Link/Link.tsx | 13 +- .../internal/utils/toggleStyledComponent.tsx | 30 ++ 8 files changed, 507 insertions(+), 143 deletions(-) create mode 100644 .changeset/odd-rings-applaud.md create mode 100644 packages/react/src/Banner/Banner.module.css create mode 100644 packages/react/src/internal/utils/toggleStyledComponent.tsx diff --git a/.changeset/odd-rings-applaud.md b/.changeset/odd-rings-applaud.md new file mode 100644 index 00000000000..84454c5969e --- /dev/null +++ b/.changeset/odd-rings-applaud.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Update Banner to use CSS Modules behind feature flag diff --git a/e2e/components/Banner.test.ts b/e2e/components/Banner.test.ts index edf83659a0e..bc1a884dcb8 100644 --- a/e2e/components/Banner.test.ts +++ b/e2e/components/Banner.test.ts @@ -77,6 +77,24 @@ test.describe('Banner', () => { id: story.id, globals: { colorScheme: theme, + featureFlags: { + primer_react_css_modules_team: true, + }, + }, + }) + + // Default state + expect(await page.screenshot()).toMatchSnapshot(`Banner.${story.title}.${theme}.png`) + }) + + test('default (styled-components) @vrt', async ({page}) => { + await visit(page, { + id: story.id, + globals: { + colorScheme: theme, + featureFlags: { + primer_react_css_modules_team: false, + }, }, }) @@ -89,6 +107,22 @@ test.describe('Banner', () => { id: story.id, globals: { colorScheme: theme, + featureFlags: { + primer_react_css_modules_team: true, + }, + }, + }) + await expect(page).toHaveNoViolations() + }) + + test('axe (styled-components) @aat', async ({page}) => { + await visit(page, { + id: story.id, + globals: { + colorScheme: theme, + featureFlags: { + primer_react_css_modules_team: false, + }, }, }) await expect(page).toHaveNoViolations() @@ -101,6 +135,29 @@ test.describe('Banner', () => { test(`${name} @vrt`, async ({page}) => { await visit(page, { id: story.id, + globals: { + featureFlags: { + primer_react_css_modules_team: true, + }, + }, + }) + const width = viewports[name] + + await page.setViewportSize({ + width, + height: 667, + }) + expect(await page.screenshot()).toMatchSnapshot(`Banner.${story.title}.${name}.png`) + }) + + test(`${name} (styled-components) @vrt`, async ({page}) => { + await visit(page, { + id: story.id, + globals: { + featureFlags: { + primer_react_css_modules_team: false, + }, + }, }) const width = viewports[name] diff --git a/packages/react/src/Banner/Banner.docs.json b/packages/react/src/Banner/Banner.docs.json index e17bbe3b4bd..9451a4dbef5 100644 --- a/packages/react/src/Banner/Banner.docs.json +++ b/packages/react/src/Banner/Banner.docs.json @@ -11,6 +11,11 @@ "type": "string", "description": "Provide an optional label to override the default name for the Banner landmark region" }, + { + "name": "className", + "type": "string", + "description": "Provide an optional className to add to the outermost element rendered by the Banner" + }, { "name": "description", "type": "React.ReactNode", diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css new file mode 100644 index 00000000000..e97e542b524 --- /dev/null +++ b/packages/react/src/Banner/Banner.module.css @@ -0,0 +1,196 @@ +.Banner { + display: grid; + padding: var(--base-size-8); + background-color: var(--banner-bgColor); + border: var(--borderWidth-thin, 1px) solid var(--banner-borderColor); + border-radius: var(--borderRadius-medium); + grid-template-columns: auto minmax(0, 1fr) auto; + align-items: start; + + @supports (container-type: inline-size) { + container: banner / inline-size; + } + + &[data-variant='critical'] { + --banner-bgColor: var(--bgColor-danger-muted); + --banner-borderColor: var(--borderColor-danger-muted); + --banner-icon-fgColor: var(--fgColor-danger); + } + + &[data-variant='info'] { + --banner-bgColor: var(--bgColor-accent-muted); + --banner-borderColor: var(--borderColor-accent-muted); + --banner-icon-fgColor: var(--fgColor-accent); + } + + &[data-variant='success'] { + --banner-bgColor: var(--bgColor-success-muted); + --banner-borderColor: var(--borderColor-success-muted); + --banner-icon-fgColor: var(--fgColor-success); + } + + &[data-variant='upsell'] { + --banner-bgColor: var(--bgColor-upsell-muted); + --banner-borderColor: var(--borderColor-upsell-muted); + --banner-icon-fgColor: var(--fgColor-upsell); + } + + &[data-variant='warning'] { + --banner-bgColor: var(--bgColor-attention-muted); + --banner-borderColor: var(--borderColor-attention-muted); + --banner-icon-fgColor: var(--fgColor-attention); + } +} + +/* BannerContainer -------------------------------------------------------- */ + +.BannerContainer { + font-size: var(--text-body-size-medium); + align-items: start; + line-height: var(--text-body-lineHeight-medium); + row-gap: var(--base-size-4); + column-gap: var(--base-size-4); +} + +.Banner :where(.BannerContainer) { + display: flex; + flex-wrap: wrap; + justify-content: space-between; +} + +.Banner[data-dismissible]:not([data-title-hidden='']) .BannerContainer { + display: grid; + grid-template-columns: auto; + grid-template-rows: auto; +} + +/* BannerContent ---------------------------------------------------------- */ + +.BannerContent { + display: grid; + row-gap: var(--base-size-4); + grid-column-start: 1; + margin-block: var(--base-size-8); +} + +.Banner[data-title-hidden] .BannerContent { + margin-block: var(--base-size-6); +} + +@media screen and (min-width: 544px) { + .BannerContent { + flex: 1 1 0%; + } +} + +.BannerTitle { + margin: 0; + font-size: inherit; + font-weight: var(--base-text-weight-semibold); +} + +/* BannerIcon ------------------------------------------------------------- */ + +.BannerIcon { + display: grid; + place-items: center; + padding: var(--base-size-8); +} + +.BannerIcon svg { + /* 20px is the line box height of the trailing action buttons */ + height: var(--base-size-20); + color: var(--banner-icon-fgColor); + fill: var(--banner-icon-fgColor); +} + +.Banner[data-title-hidden] .BannerIcon svg { + height: var(--base-size-16); +} + +/* BannerDismiss ---------------------------------------------------------- */ + +.BannerDismiss { + display: grid; + place-items: center; + padding: var(--base-size-8); + margin-inline-start: var(--base-size-4); +} + +.BannerDismiss svg { + color: var(--banner-icon-fgColor); +} + +/* BannerActions ---------------------------------------------------------- */ + +.BannerActionsContainer { + display: flex; + column-gap: var(--base-size-12); + align-items: center; +} + +.BannerActions :where([data-primary-action='trailing']) { + display: none; +} + +@media screen and (--viewportRange-regular) { + .BannerActions :where([data-primary-action='trailing']) { + display: flex; + } + + .BannerActions :where([data-primary-action='leading']) { + display: none; + } +} + +.Banner[data-dismissible] .BannerActions { + margin-block-end: var(--base-size-6); +} + +/* stylelint-disable-next-line selector-max-specificity */ +.Banner[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='trailing'] { + display: none; +} + +/* stylelint-disable-next-line selector-max-specificity */ +.Banner[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='leading'] { + display: flex; +} + +/* Layout ------------------------------------------------------------------- */ + +/* stylelint-disable-next-line plugin/no-unsupported-browser-features */ +@container banner (max-width: 500px) { + .BannerContainer { + display: grid; + grid-template-rows: auto auto; + } + + .BannerActions { + margin-block-end: var(--base-size-6); + } + + .BannerActions [data-primary-action='trailing'] { + display: none; + } + + .BannerActions [data-primary-action='leading'] { + display: flex; + } +} + +/* stylelint-disable-next-line plugin/no-unsupported-browser-features */ +@container banner (min-width: 500px) { + .BannerContainer { + display: grid; + grid-template-columns: auto auto; + } + + .BannerActions [data-primary-action='trailing'] { + display: flex; + } + + .BannerActions [data-primary-action='leading'] { + display: none; + } +} diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index d0c99438b82..7771c22cf3c 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -29,6 +29,11 @@ describe('Banner', () => { expect(screen.getByRole('heading', {name: 'test'})).toBeInTheDocument() }) + it('should support a custom `className` on the outermost element', () => { + const {container} = render() + expect(container.firstChild).toHaveClass('test') + }) + it('should label the landmark element with the corresponding variant label text', () => { render() expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Information')) diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index a60340c459a..5b664081dc2 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -6,6 +6,9 @@ import {Button, IconButton} from '../Button' import {get} from '../constants' import {VisuallyHidden} from '../internal/components/VisuallyHidden' import {useMergedRefs} from '../internal/hooks/useMergedRefs' +import {useFeatureFlag} from '../FeatureFlags' +import classes from './Banner.module.css' +import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' type BannerVariant = 'critical' | 'info' | 'success' | 'upsell' | 'warning' @@ -16,6 +19,12 @@ export type BannerProps = React.ComponentPropsWithoutRef<'section'> & { */ 'aria-label'?: string + /** + * Provide an optional className to add to the outermost element rendered by + * the Banner + */ + className?: string + /** * Provide an optional description for the Banner. This should provide * supplemental information about the Banner @@ -78,10 +87,13 @@ const labels: Record = { warning: 'Warning', } +const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_team' + export const Banner = React.forwardRef(function Banner( { 'aria-label': label, children, + className, description, hideTitle, icon, @@ -98,6 +110,7 @@ export const Banner = React.forwardRef(function Banner const hasActions = primaryAction || secondaryAction const bannerRef = React.useRef(null) const ref = useMergedRefs(forwardRef, bannerRef) + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const supportsCustomIcon = variant === 'info' || variant === 'upsell' if (__DEV__) { @@ -127,16 +140,36 @@ export const Banner = React.forwardRef(function Banner {...rest} aria-label={label ?? labels[variant]} as="section" + className={clsx(className, { + [classes.Banner]: enabled, + })} data-dismissible={onDismiss ? '' : undefined} data-title-hidden={hideTitle ? '' : undefined} data-variant={variant} tabIndex={-1} ref={ref} > - -
{icon && supportsCustomIcon ? icon : iconForVariant[variant]}
-
-
+ {!enabled ? : null} +
+ {icon && supportsCustomIcon ? icon : iconForVariant[variant]} +
+
+
{title ? ( hideTitle ? ( @@ -155,7 +188,10 @@ export const Banner = React.forwardRef(function Banner @@ -164,167 +200,170 @@ export const Banner = React.forwardRef(function Banner ) }) -/** - * For styling, it's important that the icons and the text have the same height - * for alignment to occur in multi-line scenarios. Currently, we use a - * line-height of `20px` so that means that the height of icons should match - * that value. - */ -const StyledBanner = styled.div` - display: grid; - grid-template-columns: auto minmax(0, 1fr) auto; - align-items: start; - background-color: var(--banner-bgColor); - border: var(--borderWidth-thin, 1px) solid var(--banner-borderColor); - padding: var(--base-size-8, 0.5rem); - border-radius: var(--borderRadius-medium, ${get('radii.2')}); - - @supports (container-type: inline-size) { - container: banner / inline-size; - } - - &[data-variant='critical'] { - --banner-bgColor: ${get('colors.danger.subtle')}; - --banner-borderColor: ${get('colors.danger.muted')}; - --banner-icon-fgColor: ${get('colors.danger.fg')}; - } +const StyledBanner = toggleStyledComponent( + CSS_MODULES_FEATURE_FLAG, + /** + * For styling, it's important that the icons and the text have the same height + * for alignment to occur in multi-line scenarios. Currently, we use a + * line-height of `20px` so that means that the height of icons should match + * that value. + */ + styled.div` + display: grid; + grid-template-columns: auto minmax(0, 1fr) auto; + align-items: start; + background-color: var(--banner-bgColor); + border: var(--borderWidth-thin, 1px) solid var(--banner-borderColor); + padding: var(--base-size-8, 0.5rem); + border-radius: var(--borderRadius-medium, ${get('radii.2')}); - &[data-variant='info'] { - --banner-bgColor: ${get('colors.accent.subtle')}; - --banner-borderColor: ${get('colors.accent.muted')}; - --banner-icon-fgColor: ${get('colors.accent.fg')}; - } + @supports (container-type: inline-size) { + container: banner / inline-size; + } - &[data-variant='success'] { - --banner-bgColor: ${get('colors.success.subtle')}; - --banner-borderColor: ${get('colors.success.muted')}; - --banner-icon-fgColor: ${get('colors.success.fg')}; - } + &[data-variant='critical'] { + --banner-bgColor: ${get('colors.danger.subtle')}; + --banner-borderColor: ${get('colors.danger.muted')}; + --banner-icon-fgColor: ${get('colors.danger.fg')}; + } - &[data-variant='upsell'] { - --banner-bgColor: var(--bgColor-upsell-muted, ${get('colors.done.subtle')}); - --banner-borderColor: var(--borderColor-upsell-muted, ${get('colors.done.muted')}); - --banner-icon-fgColor: var(--fgColor-upsell-muted, ${get('colors.done.fg')}); - } + &[data-variant='info'] { + --banner-bgColor: ${get('colors.accent.subtle')}; + --banner-borderColor: ${get('colors.accent.muted')}; + --banner-icon-fgColor: ${get('colors.accent.fg')}; + } - &[data-variant='warning'] { - --banner-bgColor: ${get('colors.attention.subtle')}; - --banner-borderColor: ${get('colors.attention.muted')}; - --banner-icon-fgColor: ${get('colors.attention.fg')}; - } + &[data-variant='success'] { + --banner-bgColor: ${get('colors.success.subtle')}; + --banner-borderColor: ${get('colors.success.muted')}; + --banner-icon-fgColor: ${get('colors.success.fg')}; + } - /* BannerIcon ------------------------------------------------------------- */ + &[data-variant='upsell'] { + --banner-bgColor: var(--bgColor-upsell-muted, ${get('colors.done.subtle')}); + --banner-borderColor: var(--borderColor-upsell-muted, ${get('colors.done.muted')}); + --banner-icon-fgColor: var(--fgColor-upsell-muted, ${get('colors.done.fg')}); + } - .BannerIcon { - display: grid; - place-items: center; - padding: var(--base-size-8, 0.5rem); - } + &[data-variant='warning'] { + --banner-bgColor: ${get('colors.attention.subtle')}; + --banner-borderColor: ${get('colors.attention.muted')}; + --banner-icon-fgColor: ${get('colors.attention.fg')}; + } - .BannerIcon svg { - color: var(--banner-icon-fgColor); - fill: var(--banner-icon-fgColor); - /* 20px is the line box height of the trailing action buttons */ - height: var(--base-size-20, 1.25rem); - } + /* BannerIcon ------------------------------------------------------------- */ - &[data-title-hidden=''] .BannerIcon svg { - height: var(--base-size-16, 1rem); - } + .BannerIcon { + display: grid; + place-items: center; + padding: var(--base-size-8, 0.5rem); + } - /* BannerContainer -------------------------------------------------------- */ + .BannerIcon svg { + color: var(--banner-icon-fgColor); + fill: var(--banner-icon-fgColor); + /* 20px is the line box height of the trailing action buttons */ + height: var(--base-size-20, 1.25rem); + } - .BannerContainer { - font-size: var(--text-body-size-medium, 0.875rem); - align-items: start; - line-height: var(--text-body-lineHeight-medium, calc(20 / 14)); - row-gap: var(--base-size-4, 0.25rem); - column-gap: var(--base-size-4, 0.25rem); - } + &[data-title-hidden=''] .BannerIcon svg { + height: var(--base-size-16, 1rem); + } - & :where(.BannerContainer) { - display: flex; - flex-wrap: wrap; - justify-content: space-between; - } + /* BannerContainer -------------------------------------------------------- */ - &[data-dismissible]:not([data-title-hidden='']) .BannerContainer { - display: grid; - grid-template-columns: auto; - grid-template-rows: auto; - } + .BannerContainer { + font-size: var(--text-body-size-medium, 0.875rem); + align-items: start; + line-height: var(--text-body-lineHeight-medium, calc(20 / 14)); + row-gap: var(--base-size-4, 0.25rem); + column-gap: var(--base-size-4, 0.25rem); + } - /* BannerContent ---------------------------------------------------------- */ + & :where(.BannerContainer) { + display: flex; + flex-wrap: wrap; + justify-content: space-between; + } - .BannerContent { - display: grid; - row-gap: var(--base-size-4, 0.25rem); - grid-column-start: 1; - margin-block: var(--base-size-8, 0.5rem); - } + &[data-dismissible]:not([data-title-hidden='']) .BannerContainer { + display: grid; + grid-template-columns: auto; + grid-template-rows: auto; + } - &[data-title-hidden=''] .BannerContent { - margin-block: var(--base-size-6, 0.375rem); - } + /* BannerContent ---------------------------------------------------------- */ - @media screen and (min-width: 544px) { .BannerContent { - flex: 1 1 0%; + display: grid; + row-gap: var(--base-size-4, 0.25rem); + grid-column-start: 1; + margin-block: var(--base-size-8, 0.5rem); } - } - .BannerTitle { - margin: 0; - font-size: inherit; - font-weight: var(--base-text-weight-semibold, 600); - } + &[data-title-hidden=''] .BannerContent { + margin-block: var(--base-size-6, 0.375rem); + } - /* BannerActions ---------------------------------------------------------- */ - .BannerActionsContainer { - display: flex; - column-gap: var(--base-size-12, 0.5rem); - align-items: center; - } + @media screen and (min-width: 544px) { + .BannerContent { + flex: 1 1 0%; + } + } - .BannerActions :where([data-primary-action='trailing']) { - display: none; - } + .BannerTitle { + margin: 0; + font-size: inherit; + font-weight: var(--base-text-weight-semibold, 600); + } - @media screen and (min-width: 544px) { - .BannerActions :where([data-primary-action='trailing']) { + /* BannerActions ---------------------------------------------------------- */ + .BannerActionsContainer { display: flex; + column-gap: var(--base-size-12, 0.5rem); + align-items: center; } - .BannerActions :where([data-primary-action='leading']) { + .BannerActions :where([data-primary-action='trailing']) { display: none; } - } - &[data-dismissible] .BannerActions { - margin-block-end: var(--base-size-6, 0.375rem); - } + @media screen and (min-width: 544px) { + .BannerActions :where([data-primary-action='trailing']) { + display: flex; + } - &[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='trailing'] { - display: none; - } + .BannerActions :where([data-primary-action='leading']) { + display: none; + } + } - &[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='leading'] { - display: flex; - } + &[data-dismissible] .BannerActions { + margin-block-end: var(--base-size-6, 0.375rem); + } - /* BannerDismiss ---------------------------------------------------------- */ + &[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='trailing'] { + display: none; + } - .BannerDismiss { - display: grid; - place-items: center; - padding: var(--base-size-8, 0.5rem); - margin-inline-start: var(--base-size-4, 0.25rem); - } + &[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='leading'] { + display: flex; + } - .BannerDismiss svg { - color: var(--banner-icon-fgColor); - } -` + /* BannerDismiss ---------------------------------------------------------- */ + + .BannerDismiss { + display: grid; + place-items: center; + padding: var(--base-size-8, 0.5rem); + margin-inline-start: var(--base-size-4, 0.25rem); + } + + .BannerDismiss svg { + color: var(--banner-icon-fgColor); + } + `, +) const BannerContainerQuery = ` @container banner (max-width: 500px) { @@ -371,8 +410,16 @@ export type BannerTitleProps = { export function BannerTitle(props: BannerTitleProps) { const {as: Heading = 'h2', className, children, ...rest} = props + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) return ( - + {children} ) @@ -394,13 +441,31 @@ export type BannerActionsProps = { } export function BannerActions({primaryAction, secondaryAction}: BannerActionsProps) { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) return ( -
-
+
+
{secondaryAction ?? null} {primaryAction ?? null}
-
+
{primaryAction ?? null} {secondaryAction ?? null}
diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index 1326259fd12..33d0562e477 100644 --- a/packages/react/src/Link/Link.tsx +++ b/packages/react/src/Link/Link.tsx @@ -61,7 +61,7 @@ const StyledLink = styled.a` ${sx}; ` -const Link = forwardRef(({as: Component = 'a', className, ...props}, forwardedRef) => { +const Link = forwardRef(({as: Component = 'a', className, inline, underline, ...props}, forwardedRef) => { const enabled = useFeatureFlag('primer_react_css_modules_staff') const innerRef = React.useRef(null) @@ -98,8 +98,8 @@ const Link = forwardRef(({as: Component = 'a', className, ...props}, forwardedRe as={Component} className={clsx(className, classes.Link)} data-muted={props.muted} - data-inline={props.inline} - data-underline={props.underline} + data-inline={inline} + data-underline={underline} {...props} // @ts-ignore shh ref={innerRef} @@ -111,8 +111,8 @@ const Link = forwardRef(({as: Component = 'a', className, ...props}, forwardedRe +} + +/** + * Utility to toggle rendering a styled component or a "plain" component based + * on the provided `as` prop. When the provided feature flag is enabled, the + * props will be passed through to an element or component created with the `as` + * prop. When it is disabled, the provided styled component is used instead. + * + * @param flag - the feature flag that will control whether or not the provided + * styled component is used + * @param Component - the styled component that will be used if the feature flag + * is disabled + */ +export function toggleStyledComponent(flag: string, Component: React.ComponentType

) { + const Wrapper = React.forwardRef(function Wrapper({as: BaseComponent = 'div', ...rest}, ref) { + const enabled = useFeatureFlag(flag) + if (enabled) { + return + } + return + }) + + return Wrapper +}