From c43094e708fb92d4f0d7d8869887a67e8dc32b06 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 3 Sep 2024 15:43:56 -0500 Subject: [PATCH 1/9] feat(Banner): update Banner to use CSS Modules behind feature flag --- e2e/components/Banner.test.ts | 57 +++ package.json | 5 +- packages/react/src/Banner/Banner.docs.json | 5 + packages/react/src/Banner/Banner.module.css | 192 ++++++++++ packages/react/src/Banner/Banner.test.tsx | 5 + packages/react/src/Banner/Banner.tsx | 347 +++++++++++------- packages/react/src/Link/Link.tsx | 12 +- .../internal/utils/toggleStyledComponent.tsx | 29 ++ 8 files changed, 503 insertions(+), 149 deletions(-) create mode 100644 packages/react/src/Banner/Banner.module.css create mode 100644 packages/react/src/internal/utils/toggleStyledComponent.tsx diff --git a/e2e/components/Banner.test.ts b/e2e/components/Banner.test.ts index 8182e8fe20f..e5316d15a3f 100644 --- a/e2e/components/Banner.test.ts +++ b/e2e/components/Banner.test.ts @@ -72,6 +72,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, + }, }, }) @@ -84,6 +102,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() @@ -96,6 +130,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/package.json b/package.json index 6fa6384f11d..943355ad704 100644 --- a/package.json +++ b/package.json @@ -100,5 +100,6 @@ ], "lint-staged": { "**/*.{js,ts,tsx,md,mdx}": "npm run lint" - } -} \ No newline at end of file + }, + "packageManager": "pnpm@8.15.4+sha256.cea6d0bdf2de3a0549582da3983c70c92ffc577ff4410cbf190817ddc35137c2" +} 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..aeda81ea27d --- /dev/null +++ b/packages/react/src/Banner/Banner.module.css @@ -0,0 +1,192 @@ +.Banner { + 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); + + @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, 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); +} + +.Banner :where(.BannerContainer) { + display: flex; + flex-wrap: wrap; + justify-content: space-between; +} + +.Banner[data-dismissible] .BannerContainer { + display: grid; + grid-template-columns: auto; + grid-template-rows: auto; +} + +/* BannerContent ---------------------------------------------------------- */ + +.BannerContent { + display: grid; + row-gap: var(--base-size-4, 0.25rem); + grid-column-start: 1; + margin-block: var(--base-size-8, 0.5rem); +} + +.Banner[data-title-hidden] .BannerContent { + margin-block: var(--base-size-6, 0.375rem); +} + +@media screen and (min-width: 544px) { + .BannerContent { + flex: 1 1 0%; + } +} + +.BannerTitle { + margin: 0; + font-size: inherit; + font-weight: var(--base-text-weight-semibold, 600); +} + +/* BannerIcon ------------------------------------------------------------- */ + +.BannerIcon { + display: grid; + place-items: center; + padding: var(--base-size-8, 0.5rem); +} + +.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); +} + +.Banner[data-title-hidden] .BannerIcon svg { + height: var(--base-size-16, 1rem); +} + +/* 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); +} + +/* BannerActions ---------------------------------------------------------- */ + +.BannerActionsContainer { + display: flex; + column-gap: var(--base-size-12, 0.5rem); + align-items: center; +} + +.BannerActions :where([data-primary-action='trailing']) { + display: none; +} + +@media screen and (min-width: 544px) { + .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, 0.375rem); +} + +.Banner[data-dismissible] .BannerActionsContainer[data-primary-action='trailing'] { + display: none; +} + +.Banner[data-dismissible] .BannerActionsContainer[data-primary-action='leading'] { + display: flex; +} + +/* Layout ------------------------------------------------------------------- */ + +@container banner (max-width: 500px) { + .BannerContainer { + display: grid; + grid-template-rows: auto auto; + } + + .BannerActions { + margin-block-end: var(--size-small, 0.375rem); + } + + .BannerActions [data-primary-action='trailing'] { + display: none; + } + + .BannerActions [data-primary-action='leading'] { + display: flex; + } +} + +@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 30231fe63c5..8d9dbbc9a80 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -1,4 +1,4 @@ -import cx from 'clsx' +import {clsx} from 'clsx' import React, {useEffect} from 'react' import styled from 'styled-components' import {AlertIcon, InfoIcon, StopIcon, CheckCircleIcon, XIcon} from '@primer/octicons-react' @@ -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] .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] .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] .BannerActionsContainer[data-primary-action='trailing'] { - display: none; - } + .BannerActions :where([data-primary-action='leading']) { + display: none; + } + } - &[data-dismissible] .BannerActionsContainer[data-primary-action='leading'] { - display: flex; - } + &[data-dismissible] .BannerActions { + margin-block-end: var(--base-size-6, 0.375rem); + } - /* BannerDismiss ---------------------------------------------------------- */ + &[data-dismissible] .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] .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} ) @@ -382,7 +429,7 @@ export type BannerDescriptionProps = React.ComponentPropsWithoutRef<'div'> export function BannerDescription({children, className, ...rest}: BannerDescriptionProps) { 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}
@@ -412,7 +477,7 @@ export type BannerPrimaryActionProps = Omit + ) @@ -422,7 +487,7 @@ export type BannerSecondaryActionProps = Omit + ) diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index c1b45ac2329..5dfc368ec64 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_team') const innerRef = React.useRef(null) @@ -98,8 +98,8 @@ const Link = forwardRef(({as: Component = 'a', className, ...props}, forwardedRe as={Component} className={cx(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 +} From 630f564609c88049c9efde574d47cf1baf7d0c7f Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 3 Sep 2024 15:44:23 -0500 Subject: [PATCH 2/9] chore: add changeset --- .changeset/odd-rings-applaud.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/odd-rings-applaud.md 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 From 3c182a8077e83a0c306efabed86cb35efe50b625 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 3 Sep 2024 15:45:50 -0500 Subject: [PATCH 3/9] chore: remove autogenerated pnpm packageManager change --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 943355ad704..8868498752a 100644 --- a/package.json +++ b/package.json @@ -100,6 +100,5 @@ ], "lint-staged": { "**/*.{js,ts,tsx,md,mdx}": "npm run lint" - }, - "packageManager": "pnpm@8.15.4+sha256.cea6d0bdf2de3a0549582da3983c70c92ffc577ff4410cbf190817ddc35137c2" + } } From 6a08c89b58275e4bb0e030a31c58e05ce56fa0e4 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 3 Sep 2024 15:48:46 -0500 Subject: [PATCH 4/9] chore: fix stylelint errors --- packages/react/src/Banner/Banner.module.css | 26 +++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css index aeda81ea27d..4f055861387 100644 --- a/packages/react/src/Banner/Banner.module.css +++ b/packages/react/src/Banner/Banner.module.css @@ -1,11 +1,11 @@ .Banner { display: grid; - grid-template-columns: auto minmax(0, 1fr) auto; - align-items: start; + padding: var(--base-size-8, var(--base-size-8)); 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); + grid-template-columns: auto minmax(0, 1fr) auto; + align-items: start; @supports (container-type: inline-size) { container: banner / inline-size; @@ -70,11 +70,11 @@ display: grid; row-gap: var(--base-size-4, 0.25rem); grid-column-start: 1; - margin-block: var(--base-size-8, 0.5rem); + margin-block: var(--base-size-8, var(--base-size-8)); } .Banner[data-title-hidden] .BannerContent { - margin-block: var(--base-size-6, 0.375rem); + margin-block: var(--base-size-6, var(--base-size-6)); } @media screen and (min-width: 544px) { @@ -94,14 +94,14 @@ .BannerIcon { display: grid; place-items: center; - padding: var(--base-size-8, 0.5rem); + padding: var(--base-size-8, var(--base-size-8)); } .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); + color: var(--banner-icon-fgColor); + fill: var(--banner-icon-fgColor); } .Banner[data-title-hidden] .BannerIcon svg { @@ -113,8 +113,8 @@ .BannerDismiss { display: grid; place-items: center; - padding: var(--base-size-8, 0.5rem); - margin-inline-start: var(--base-size-4, 0.25rem); + padding: var(--base-size-8, var(--base-size-8)); + margin-inline-start: var(--base-size-4, var(--base-size-4)); } .BannerDismiss svg { @@ -144,7 +144,7 @@ } .Banner[data-dismissible] .BannerActions { - margin-block-end: var(--base-size-6, 0.375rem); + margin-block-end: var(--base-size-6, var(--base-size-6)); } .Banner[data-dismissible] .BannerActionsContainer[data-primary-action='trailing'] { @@ -157,6 +157,7 @@ /* Layout ------------------------------------------------------------------- */ +/* stylelint-disable-next-line plugin/no-unsupported-browser-features */ @container banner (max-width: 500px) { .BannerContainer { display: grid; @@ -164,7 +165,7 @@ } .BannerActions { - margin-block-end: var(--size-small, 0.375rem); + margin-block-end: var(--base-size-6); } .BannerActions [data-primary-action='trailing'] { @@ -176,6 +177,7 @@ } } +/* stylelint-disable-next-line plugin/no-unsupported-browser-features */ @container banner (min-width: 500px) { .BannerContainer { display: grid; From 684b3ab81b3251961c6be9b456ebb911e03a1322 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 4 Sep 2024 15:58:02 -0500 Subject: [PATCH 5/9] chore: add back in underline for fallback case --- packages/react/src/Link/Link.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index 5dfc368ec64..3571f2d19ea 100644 --- a/packages/react/src/Link/Link.tsx +++ b/packages/react/src/Link/Link.tsx @@ -125,6 +125,7 @@ const Link = forwardRef(({as: Component = 'a', className, inline, underline, ... as={Component} className={className} data-inline={inline} + underline={underline} {...props} // @ts-ignore shh ref={innerRef} From 3f3e277729dbd4827d9525d174019793d056b1fb Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 9 Sep 2024 16:04:30 -0500 Subject: [PATCH 6/9] refactor: update autofix and eslint warning --- packages/react/src/Banner/Banner.module.css | 34 +++++++++---------- .../internal/utils/toggleStyledComponent.tsx | 1 + 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css index 4f055861387..b4ed645848f 100644 --- a/packages/react/src/Banner/Banner.module.css +++ b/packages/react/src/Banner/Banner.module.css @@ -1,6 +1,6 @@ .Banner { display: grid; - padding: var(--base-size-8, var(--base-size-8)); + padding: var(--base-size-8); background-color: var(--banner-bgColor); border: var(--borderWidth-thin, 1px) solid var(--banner-borderColor); border-radius: var(--borderRadius-medium); @@ -45,11 +45,11 @@ /* BannerContainer -------------------------------------------------------- */ .BannerContainer { - font-size: var(--text-body-size-medium, 0.875rem); + font-size: var(--text-body-size-medium); 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); + line-height: var(--text-body-lineHeight-medium)); + row-gap: var(--base-size-4); + column-gap: var(--base-size-4); } .Banner :where(.BannerContainer) { @@ -68,13 +68,13 @@ .BannerContent { display: grid; - row-gap: var(--base-size-4, 0.25rem); + row-gap: var(--base-size-4); grid-column-start: 1; - margin-block: var(--base-size-8, var(--base-size-8)); + margin-block: var(--base-size-8); } .Banner[data-title-hidden] .BannerContent { - margin-block: var(--base-size-6, var(--base-size-6)); + margin-block: var(--base-size-6); } @media screen and (min-width: 544px) { @@ -86,7 +86,7 @@ .BannerTitle { margin: 0; font-size: inherit; - font-weight: var(--base-text-weight-semibold, 600); + font-weight: var(--base-text-weight-semibold); } /* BannerIcon ------------------------------------------------------------- */ @@ -94,18 +94,18 @@ .BannerIcon { display: grid; place-items: center; - padding: var(--base-size-8, var(--base-size-8)); + padding: var(--base-size-8); } .BannerIcon svg { /* 20px is the line box height of the trailing action buttons */ - height: var(--base-size-20, 1.25rem); + 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, 1rem); + height: var(--base-size-16); } /* BannerDismiss ---------------------------------------------------------- */ @@ -113,8 +113,8 @@ .BannerDismiss { display: grid; place-items: center; - padding: var(--base-size-8, var(--base-size-8)); - margin-inline-start: var(--base-size-4, var(--base-size-4)); + padding: var(--base-size-8); + margin-inline-start: var(--base-size-4); } .BannerDismiss svg { @@ -125,7 +125,7 @@ .BannerActionsContainer { display: flex; - column-gap: var(--base-size-12, 0.5rem); + column-gap: var(--base-size-12); align-items: center; } @@ -133,7 +133,7 @@ display: none; } -@media screen and (min-width: 544px) { +@media screen and (--viewportRange-regular) { .BannerActions :where([data-primary-action='trailing']) { display: flex; } @@ -144,7 +144,7 @@ } .Banner[data-dismissible] .BannerActions { - margin-block-end: var(--base-size-6, var(--base-size-6)); + margin-block-end: var(--base-size-6); } .Banner[data-dismissible] .BannerActionsContainer[data-primary-action='trailing'] { diff --git a/packages/react/src/internal/utils/toggleStyledComponent.tsx b/packages/react/src/internal/utils/toggleStyledComponent.tsx index 2bd20624633..4a9bbd0e00a 100644 --- a/packages/react/src/internal/utils/toggleStyledComponent.tsx +++ b/packages/react/src/internal/utils/toggleStyledComponent.tsx @@ -2,6 +2,7 @@ import React from 'react' import {useFeatureFlag} from '../../FeatureFlags' type CSSModulesProps = { + // eslint-disable-next-line @typescript-eslint/no-explicit-any as?: string | React.ComponentType } From c95811841139fd35d58d448ea7fce25af0cb4c4f Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 13 Sep 2024 13:07:01 -0500 Subject: [PATCH 7/9] chore: update class order for tests --- packages/react/src/Banner/Banner.module.css | 6 +++--- packages/react/src/Banner/Banner.tsx | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css index b4ed645848f..2d87f62041b 100644 --- a/packages/react/src/Banner/Banner.module.css +++ b/packages/react/src/Banner/Banner.module.css @@ -58,7 +58,7 @@ justify-content: space-between; } -.Banner[data-dismissible] .BannerContainer { +.Banner[data-dismissible]:not([data-title-hidden='']) .BannerContainer { display: grid; grid-template-columns: auto; grid-template-rows: auto; @@ -147,11 +147,11 @@ margin-block-end: var(--base-size-6); } -.Banner[data-dismissible] .BannerActionsContainer[data-primary-action='trailing'] { +.Banner[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='trailing'] { display: none; } -.Banner[data-dismissible] .BannerActionsContainer[data-primary-action='leading'] { +.Banner[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='leading'] { display: flex; } diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index 4a7072958d1..5b664081dc2 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -415,8 +415,8 @@ export function BannerTitle(props: BannerTitleProps @@ -445,14 +445,14 @@ export function BannerActions({primaryAction, secondaryAction}: BannerActionsPro return (

@@ -461,8 +461,8 @@ export function BannerActions({primaryAction, secondaryAction}: BannerActionsPro
From 93cdc804e66f6a888538596a355ea1fea501c56a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 16 Sep 2024 15:19:27 -0500 Subject: [PATCH 8/9] chore: fix stylelint errors --- packages/react/src/Banner/Banner.module.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css index 2d87f62041b..dfc1542efd8 100644 --- a/packages/react/src/Banner/Banner.module.css +++ b/packages/react/src/Banner/Banner.module.css @@ -147,10 +147,12 @@ 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; } From c42a41203256ce3a7509bd68c79690774bcfaf02 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 16 Sep 2024 15:32:23 -0500 Subject: [PATCH 9/9] Update packages/react/src/Banner/Banner.module.css Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com> --- packages/react/src/Banner/Banner.module.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Banner/Banner.module.css b/packages/react/src/Banner/Banner.module.css index 2d87f62041b..423ddda24df 100644 --- a/packages/react/src/Banner/Banner.module.css +++ b/packages/react/src/Banner/Banner.module.css @@ -47,7 +47,7 @@ .BannerContainer { font-size: var(--text-body-size-medium); align-items: start; - line-height: var(--text-body-lineHeight-medium)); + line-height: var(--text-body-lineHeight-medium); row-gap: var(--base-size-4); column-gap: var(--base-size-4); }