diff --git a/.changeset/metal-steaks-unite.md b/.changeset/metal-steaks-unite.md new file mode 100644 index 00000000000..f59cd8df573 --- /dev/null +++ b/.changeset/metal-steaks-unite.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Remove the CSS modules feature flag from Banner diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index 13fecdc6fb8..59975b0f63c 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -115,9 +115,9 @@ describe('Banner', () => { />, ) - expect(screen.getByRole('button', {name: 'test primary action'})).toBeInTheDocument() + expect(screen.queryAllByRole('button', {name: 'test primary action', hidden: true}).length).toBe(2) - await user.click(screen.getByRole('button', {name: 'test primary action'})) + await user.click(screen.queryAllByText('test primary action')[0]) expect(onClick).toHaveBeenCalledTimes(1) }) @@ -132,9 +132,9 @@ describe('Banner', () => { />, ) - expect(screen.getByRole('button', {name: 'test secondary action'})).toBeInTheDocument() + expect(screen.queryAllByRole('button', {name: 'test secondary action', hidden: true}).length).toBe(2) - await user.click(screen.getByRole('button', {name: 'test secondary action'})) + await user.click(screen.queryAllByText('test secondary action')[0]) expect(onClick).toHaveBeenCalledTimes(1) }) @@ -148,8 +148,8 @@ describe('Banner', () => { />, ) - expect(screen.getByRole('button', {name: 'test primary action'})).toBeInTheDocument() - expect(screen.getByRole('button', {name: 'test secondary action'})).toBeInTheDocument() + expect(screen.queryAllByRole('button', {name: 'test primary action', hidden: true}).length).toBe(2) + expect(screen.queryAllByRole('button', {name: 'test secondary action', hidden: true}).length).toBe(2) }) it('should call `onDismiss` when the dismiss button is activated', async () => { diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index c61f130fd2e..e1c7a1119e8 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -1,14 +1,10 @@ import {clsx} from 'clsx' import React, {forwardRef, useEffect} from 'react' -import styled from 'styled-components' import {AlertIcon, InfoIcon, StopIcon, CheckCircleIcon, XIcon} from '@primer/octicons-react' import {Button, IconButton, type ButtonProps} from '../Button' -import {get} from '../constants' import {VisuallyHidden} from '../VisuallyHidden' import {useMergedRefs} from '../internal/hooks/useMergedRefs' -import {useFeatureFlag} from '../FeatureFlags' import classes from './Banner.module.css' -import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' type BannerVariant = 'critical' | 'info' | 'success' | 'upsell' | 'warning' @@ -88,8 +84,6 @@ const labels: Record = { warning: 'Warning', } -const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_ga' - export const Banner = React.forwardRef(function Banner( { 'aria-label': label, @@ -111,7 +105,6 @@ 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__) { @@ -137,40 +130,19 @@ export const Banner = React.forwardRef(function Banner } return ( - - {!enabled ? : null} -
- {icon && supportsCustomIcon ? icon : iconForVariant[variant]} -
-
-
+
{icon && supportsCustomIcon ? icon : iconForVariant[variant]}
+
+
{title ? ( hideTitle ? ( @@ -189,221 +161,15 @@ export const Banner = React.forwardRef(function Banner ) : null} - + ) }) -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. - */ - 'div', - 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')}; - } - - &[data-variant='info'] { - --banner-bgColor: ${get('colors.accent.subtle')}; - --banner-borderColor: ${get('colors.accent.muted')}; - --banner-icon-fgColor: ${get('colors.accent.fg')}; - } - - &[data-variant='success'] { - --banner-bgColor: ${get('colors.success.subtle')}; - --banner-borderColor: ${get('colors.success.muted')}; - --banner-icon-fgColor: ${get('colors.success.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='warning'] { - --banner-bgColor: ${get('colors.attention.subtle')}; - --banner-borderColor: ${get('colors.attention.muted')}; - --banner-icon-fgColor: ${get('colors.attention.fg')}; - } - - /* 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); - } - - &[data-title-hidden=''] .BannerIcon svg { - height: var(--base-size-16, 1rem); - } - - /* 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); - } - - & :where(.BannerContainer) { - display: flex; - flex-wrap: wrap; - justify-content: space-between; - } - - &[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, 0.25rem); - grid-column-start: 1; - margin-block: var(--base-size-8, 0.5rem); - } - - &[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); - } - - /* 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; - } - } - - &[data-dismissible]:not([data-title-hidden]) .BannerActions { - margin-block-end: var(--base-size-6, 0.375rem); - } - - &[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='trailing'] { - display: none; - } - - &[data-dismissible]:not([data-title-hidden]) .BannerActionsContainer[data-primary-action='leading'] { - display: flex; - } - - /* 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) { - .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; - min-height: var(--base-size-32, 2rem); - } - - .BannerActions [data-primary-action="leading"] { - display: none; - } - } -` - type HeadingElement = 'h2' | 'h3' | 'h4' | 'h5' | 'h6' export type BannerTitleProps = { @@ -413,16 +179,8 @@ 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} ) @@ -444,31 +202,13 @@ 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}