Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 59 additions & 14 deletions packages/react/src/Banner/Banner.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {Banner} from '../Banner'
describe('Banner', () => {
it('should render as a region element', () => {
render(<Banner title="test" />)
expect(screen.getByRole('region', {name: 'Information'})).toBeInTheDocument()
expect(screen.getByRole('region', {name: 'test'})).toBeInTheDocument()
expect(screen.getByRole('heading', {name: 'test'})).toBeInTheDocument()
})

Expand All @@ -15,39 +15,84 @@ describe('Banner', () => {
expect(render(<Element />).container.firstChild).toHaveClass('test-class-name')
})

it('should label the landmark element with the corresponding variant label text', () => {
it('should label the landmark element with the title content by default', () => {
render(<Banner title="test" />)
expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Information'))
expect(screen.getByRole('region')).toHaveAccessibleName('test')
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
expect(screen.getByRole('region')).not.toHaveAttribute('aria-label')
})

it('should label the landmark element with the label for the critical variant', () => {
it('should label the landmark element with the title content for the critical variant', () => {
render(<Banner title="test" variant="critical" />)
expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Critical'))
expect(screen.getByRole('region')).toHaveAccessibleName('test')
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
})

it('should label the landmark element with the label for the info variant', () => {
it('should label the landmark element with the title content for the info variant', () => {
render(<Banner title="test" variant="info" />)
expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Information'))
expect(screen.getByRole('region')).toHaveAccessibleName('test')
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
})

it('should label the landmark element with the label for the success variant', () => {
it('should label the landmark element with the title content for the success variant', () => {
render(<Banner title="test" variant="success" />)
expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Success'))
expect(screen.getByRole('region')).toHaveAccessibleName('test')
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
})

it('should label the landmark element with the label for the upsell variant', () => {
it('should label the landmark element with the title content for the upsell variant', () => {
render(<Banner title="test" variant="upsell" />)
expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Recommendation'))
expect(screen.getByRole('region')).toHaveAccessibleName('test')
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
})

it('should label the landmark element with the label for the warning variant', () => {
it('should label the landmark element with the title content for the warning variant', () => {
render(<Banner title="test" variant="warning" />)
expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Warning'))
expect(screen.getByRole('region')).toHaveAccessibleName('test')
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
})

it('should support the `aria-label` prop to override the default label for the landmark', () => {
render(<Banner aria-label="Test" title="test" variant="warning" />)
expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'Test')
expect(screen.getByRole('region')).not.toHaveAttribute('aria-labelledby')
})

it('should support the `aria-labelledby` prop to override the default labeling for the landmark', () => {
render(
<>
<h2 id="custom-title">Custom Title</h2>
<Banner aria-labelledby="custom-title" title="test" variant="warning" />
</>,
)
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby', 'custom-title')
expect(screen.getByRole('region')).not.toHaveAttribute('aria-label')
expect(screen.getByRole('region')).toHaveAccessibleName('Custom Title')
})

it('should default to using aria-labelledby pointing to the title when no explicit labeling is provided', () => {
render(<Banner title="test" variant="warning" />)
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
expect(screen.getByRole('region')).not.toHaveAttribute('aria-label')
expect(screen.getByRole('region')).toHaveAccessibleName('test')
})

it('should use aria-labelledby pointing to Banner.Title when used as children', () => {
render(
<Banner variant="info">
<Banner.Title>Custom Banner Title</Banner.Title>
</Banner>,
)
expect(screen.getByRole('region')).toHaveAttribute('aria-labelledby')
expect(screen.getByRole('region')).not.toHaveAttribute('aria-label')
expect(screen.getByRole('region')).toHaveAccessibleName('Custom Banner Title')
})

it('should use aria-label when explicitly provided (even if empty)', () => {
render(<Banner aria-label="" title="test" variant="warning" />)
const section = document.querySelector('section')
expect(section).toHaveAttribute('aria-label', '')
expect(section).not.toHaveAttribute('aria-labelledby')
})

it('should default the title to a h2', () => {
Expand All @@ -67,7 +112,7 @@ describe('Banner', () => {
it('should rendering a description with the `description` prop', () => {
render(<Banner title="test" description="test-description" />)
expect(screen.getByText('test-description')).toBeInTheDocument()
expect(screen.getByRole('region', {name: 'Information'})).toContainElement(screen.getByText('test-description'))
expect(screen.getByRole('region', {name: 'test'})).toContainElement(screen.getByText('test-description'))
})

it('should support a primary action', async () => {
Expand Down
102 changes: 64 additions & 38 deletions packages/react/src/Banner/Banner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {AlertIcon, InfoIcon, StopIcon, CheckCircleIcon, XIcon} from '@primer/oct
import {Button, IconButton, type ButtonProps} from '../Button'
import {VisuallyHidden} from '../VisuallyHidden'
import {useMergedRefs} from '../internal/hooks/useMergedRefs'
import {useId} from '../hooks'
import classes from './Banner.module.css'
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'

Expand All @@ -16,6 +17,12 @@ export type BannerProps = React.ComponentPropsWithoutRef<'section'> & {
*/
'aria-label'?: string

/**
* Provide an optional labelledby to override the default labelledby for the Banner
* landmark region
*/
'aria-labelledby'?: string

/**
* Provide an optional className to add to the outermost element rendered by
* the Banner
Expand Down Expand Up @@ -82,9 +89,16 @@ const labels: Record<BannerVariant, string> = {
warning: 'Warning',
}

const BannerContext = React.createContext<{
titleId?: string
}>({
titleId: undefined,
})

export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner(
{
'aria-label': label,
'aria-labelledby': labelledby,
children,
className,
description,
Expand All @@ -103,8 +117,14 @@ 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 titleId = useId()
const supportsCustomIcon = variant === 'info' || variant === 'upsell'

// Determine what should be used for the landmark labeling
const shouldUseLabelledBy = label === undefined && !labelledby
const finalLabelledBy = labelledby || (shouldUseLabelledBy ? titleId : undefined)
const finalLabel = label !== undefined ? label : !finalLabelledBy ? labels[variant] : undefined

if (__DEV__) {
// This hook is called consistently depending on the environment
// eslint-disable-next-line react-compiler/react-compiler
Expand All @@ -129,43 +149,46 @@ export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner
}

return (
<section
{...rest}
aria-label={label ?? labels[variant]}
className={clsx(className, classes.Banner)}
data-dismissible={onDismiss ? '' : undefined}
data-title-hidden={hideTitle ? '' : undefined}
data-variant={variant}
tabIndex={-1}
ref={ref}
>
<div className={classes.BannerIcon}>{icon && supportsCustomIcon ? icon : iconForVariant[variant]}</div>
<div className={classes.BannerContainer}>
<div className={classes.BannerContent}>
{title ? (
hideTitle ? (
<VisuallyHidden>
<BannerTitle>{title}</BannerTitle>
</VisuallyHidden>
) : (
<BannerTitle>{title}</BannerTitle>
)
) : null}
{description ? <BannerDescription>{description}</BannerDescription> : null}
{children}
<BannerContext.Provider value={{titleId: shouldUseLabelledBy && !title ? titleId : undefined}}>
<section
{...rest}
aria-label={finalLabel}
aria-labelledby={finalLabelledBy}
className={clsx(className, classes.Banner)}
data-dismissible={onDismiss ? '' : undefined}
data-title-hidden={hideTitle ? '' : undefined}
data-variant={variant}
tabIndex={-1}
ref={ref}
>
<div className={classes.BannerIcon}>{icon && supportsCustomIcon ? icon : iconForVariant[variant]}</div>
<div className={classes.BannerContainer}>
<div className={classes.BannerContent}>
{title ? (
hideTitle ? (
<VisuallyHidden>
<BannerTitle id={shouldUseLabelledBy ? titleId : undefined}>{title}</BannerTitle>
</VisuallyHidden>
) : (
<BannerTitle id={shouldUseLabelledBy ? titleId : undefined}>{title}</BannerTitle>
)
) : null}
{description ? <BannerDescription>{description}</BannerDescription> : null}
{children}
</div>
{hasActions ? <BannerActions primaryAction={primaryAction} secondaryAction={secondaryAction} /> : null}
</div>
{hasActions ? <BannerActions primaryAction={primaryAction} secondaryAction={secondaryAction} /> : null}
</div>
{dismissible ? (
<IconButton
aria-label="Dismiss banner"
onClick={onDismiss}
className={classes.BannerDismiss}
icon={XIcon}
variant="invisible"
/>
) : null}
</section>
{dismissible ? (
<IconButton
aria-label="Dismiss banner"
onClick={onDismiss}
className={classes.BannerDismiss}
icon={XIcon}
variant="invisible"
/>
) : null}
</section>
</BannerContext.Provider>
)
})

Expand All @@ -177,9 +200,12 @@ export type BannerTitleProps<As extends HeadingElement> = {
} & React.ComponentPropsWithoutRef<As extends 'h2' ? 'h2' : As>

export function BannerTitle<As extends HeadingElement>(props: BannerTitleProps<As>) {
const {as: Heading = 'h2', className, children, ...rest} = props
const {as: Heading = 'h2', className, children, id, ...rest} = props
const context = React.useContext(BannerContext)
const finalId = id || context.titleId

return (
<Heading {...rest} className={clsx(className, classes.BannerTitle)} data-banner-title="">
<Heading {...rest} id={finalId} className={clsx(className, classes.BannerTitle)} data-banner-title="">
{children}
</Heading>
)
Expand Down