Skip to content

Commit

Permalink
chore(Banner): Remove the CSS modules feature flag from Banner (#5282)
Browse files Browse the repository at this point in the history 
* Remove feature flag from Banner

* Create metal-steaks-unite.md

* Fix test

* Update Banner tests to use queryAllByRole
  • Loading branch information
@jonrohan
jonrohan authored Dec 6, 2024
1 parent b08f770 commit d6fe52e
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 277 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-steaks-unite.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": minor
---

Remove the CSS modules feature flag from Banner
12 changes: 6 additions & 6 deletions packages/react/src/Banner/Banner.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand All @@ -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)
})

Expand All @@ -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 () => {
Expand Down
282 changes: 11 additions & 271 deletions packages/react/src/Banner/Banner.tsx
View file
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -88,8 +84,6 @@ const labels: Record<BannerVariant, string> = {
warning: 'Warning',
}

const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_ga'

export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner(
{
'aria-label': label,
Expand All @@ -111,7 +105,6 @@ export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner
const hasActions = primaryAction || secondaryAction
const bannerRef = React.useRef<HTMLElement>(null)
const ref = useMergedRefs(forwardRef, bannerRef)
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
const supportsCustomIcon = variant === 'info' || variant === 'upsell'

if (__DEV__) {
Expand All @@ -137,40 +130,19 @@ export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner
}

return (
<StyledBanner
<section
{...rest}
aria-label={label ?? labels[variant]}
as="section"
className={clsx(className, {
[classes.Banner]: enabled,
})}
className={clsx(className, classes.Banner)}
data-dismissible={onDismiss ? '' : undefined}
data-title-hidden={hideTitle ? '' : undefined}
data-variant={variant}
tabIndex={-1}
ref={ref}
>
{!enabled ? <style>{BannerContainerQuery}</style> : null}
<div
className={clsx({
BannerIcon: !enabled,
[classes.BannerIcon]: enabled,
})}
>
{icon && supportsCustomIcon ? icon : iconForVariant[variant]}
</div>
<div
className={clsx({
BannerContainer: !enabled,
[classes.BannerContainer]: enabled,
})}
>
<div
className={clsx({
BannerContent: !enabled,
[classes.BannerContent]: enabled,
})}
>
<div className={classes.BannerIcon}>{icon && supportsCustomIcon ? icon : iconForVariant[variant]}</div>
<div className={classes.BannerContainer}>
<div className={classes.BannerContent}>
{title ? (
hideTitle ? (
<VisuallyHidden>
Expand All @@ -189,221 +161,15 @@ export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner
<IconButton
aria-label="Dismiss banner"
onClick={onDismiss}
className={clsx({
BannerDismiss: !enabled,
[classes.BannerDismiss]: enabled,
})}
className={classes.BannerDismiss}
icon={XIcon}
variant="invisible"
/>
) : null}
</StyledBanner>
</section>
)
})

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<As extends HeadingElement> = {
Expand All @@ -413,16 +179,8 @@ export type BannerTitleProps<As extends HeadingElement> = {

export function BannerTitle<As extends HeadingElement>(props: BannerTitleProps<As>) {
const {as: Heading = 'h2', className, children, ...rest} = props
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
return (
<Heading
{...rest}
className={clsx(className, {
[classes.BannerTitle]: enabled,
BannerTitle: !enabled,
})}
data-banner-title=""
>
<Heading {...rest} className={clsx(className, classes.BannerTitle)} data-banner-title="">
{children}
</Heading>
)
Expand All @@ -444,31 +202,13 @@ export type BannerActionsProps = {
}

export function BannerActions({primaryAction, secondaryAction}: BannerActionsProps) {
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
return (
<div
className={clsx({
[classes.BannerActions]: enabled,
BannerActions: !enabled,
})}
>
<div
className={clsx({
[classes.BannerActionsContainer]: enabled,
BannerActionsContainer: !enabled,
})}
data-primary-action="trailing"
>
<div className={classes.BannerActions}>
<div className={classes.BannerActionsContainer} data-primary-action="trailing">
{secondaryAction ?? null}
{primaryAction ?? null}
</div>
<div
className={clsx({
[classes.BannerActionsContainer]: enabled,
BannerActionsContainer: !enabled,
})}
data-primary-action="leading"
>
<div className={classes.BannerActionsContainer} data-primary-action="leading">
{primaryAction ?? null}
{secondaryAction ?? null}
</div>
Expand Down

0 comments on commit d6fe52e

Please sign in to comment.