From 8415bead990548e19ef20fd46b6bb40453358ae4 Mon Sep 17 00:00:00 2001 From: llastflowers Date: Wed, 27 May 2026 09:57:10 -0700 Subject: [PATCH 1/8] update PageHeader.tsx, add test, and update css to match new attribute name --- .../src/PageHeader/PageHeader.module.css | 24 ++++----- .../react/src/PageHeader/PageHeader.test.tsx | 45 +++++++++++++++++ packages/react/src/PageHeader/PageHeader.tsx | 50 +++++++++++++------ 3 files changed, 93 insertions(+), 26 deletions(-) diff --git a/packages/react/src/PageHeader/PageHeader.module.css b/packages/react/src/PageHeader/PageHeader.module.css index 992f1618d12..5a96a3745c0 100644 --- a/packages/react/src/PageHeader/PageHeader.module.css +++ b/packages/react/src/PageHeader/PageHeader.module.css @@ -35,7 +35,7 @@ We don't want these values to be overridden but still want to allow consumers to override them if needed. */ /* stylelint-disable selector-pseudo-class-disallowed-list -- :has() scoped to CSS Module, audited (github/github-ui#17224) */ - &:has([data-component='TitleArea'][data-size-variant='large']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); /* calc(48/32) */ @@ -43,7 +43,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant='medium']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ @@ -51,7 +51,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant='subtitle']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ @@ -61,7 +61,7 @@ /* Responsive size variants */ @media (--viewportRange-narrow) { - &:has([data-component='TitleArea'][data-size-variant-narrow='large']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-narrow='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -69,7 +69,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-narrow='medium']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-narrow='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -77,7 +77,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-narrow='subtitle']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-narrow='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -87,7 +87,7 @@ } @media (--viewportRange-regular) { - &:has([data-component='TitleArea'][data-size-variant-regular='large']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-regular='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -95,7 +95,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-regular='medium']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-regular='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -103,7 +103,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-regular='subtitle']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-regular='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -113,7 +113,7 @@ } @media (--viewportRange-wide) { - &:has([data-component='TitleArea'][data-size-variant-wide='large']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-wide='large']) { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -121,7 +121,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-wide='medium']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-wide='medium']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -129,7 +129,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-wide='subtitle']) { + &:has([data-component='PageHeader.TitleArea'][data-size-variant-wide='subtitle']) { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); diff --git a/packages/react/src/PageHeader/PageHeader.test.tsx b/packages/react/src/PageHeader/PageHeader.test.tsx index a66f648d4bc..67182f73467 100644 --- a/packages/react/src/PageHeader/PageHeader.test.tsx +++ b/packages/react/src/PageHeader/PageHeader.test.tsx @@ -20,6 +20,45 @@ describe('PageHeader', () => { implementsClassName(PageHeader.Actions, classes.Actions) implementsClassName(PageHeader.Description, classes.Description) implementsClassName(PageHeader.Navigation, classes.Navigation) + + it('renders data-component attributes for PageHeader and exported subcomponents', () => { + const {container} = render( + + ContextArea + ParentLink + ContextBar + + LeadingAction + Breadcrumbs + LeadingVisual + Title + TrailingVisual + TrailingAction + Actions + + ContextAreaActions + Description + Navigation + , + ) + + expect(container.firstChild).toHaveAttribute('data-component', 'PageHeader') + expect(container.querySelector('[data-component="PageHeader.ContextArea"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.ParentLink"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.ContextBar"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.TitleArea"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.ContextAreaActions"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.LeadingAction"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.Breadcrumbs"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.LeadingVisual"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.Title"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.TrailingVisual"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.TrailingAction"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.Actions"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.Description"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageHeader.Navigation"]')).toBeInTheDocument() + }) + it('respects the title variant prop', () => { const {getByText} = render( @@ -31,6 +70,7 @@ describe('PageHeader', () => { ) expect(getByText('Title')).toHaveStyle('font-size: 32px') }) + it('renders "aria-label" prop when Navigation is rendered as "nav" landmark', () => { const {getByLabelText, getByText} = render( @@ -45,6 +85,7 @@ describe('PageHeader', () => { expect(getByLabelText('Custom')).toBeInTheDocument() expect(getByText('Navigation')).toHaveAttribute('aria-label', 'Custom') }) + it('does not render "aria-label" prop when Navigation is rendered as "div"', () => { const {getByText} = render( @@ -71,6 +112,7 @@ describe('PageHeader', () => { consoleSpy.mockRestore() }) + it('does not render "role" attribute when not explicitly specified', () => { const {container} = render( @@ -81,6 +123,7 @@ describe('PageHeader', () => { ) expect(container.firstChild).not.toHaveAttribute('role') }) + it('renders "role" attribute when explicitly specified', () => { const {container} = render( @@ -91,6 +134,7 @@ describe('PageHeader', () => { ) expect(container.firstChild).toHaveAttribute('role', 'banner') }) + it('does not render "aria-label" attribute when not explicitly specified', () => { const {container} = render( @@ -101,6 +145,7 @@ describe('PageHeader', () => { ) expect(container.firstChild).not.toHaveAttribute('aria-label') }) + it('renders custom "aria-label" attribute when explicitly specified', () => { const {container} = render( diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 1acaadf06f3..96ce5c28620 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -69,7 +69,7 @@ const Root = React.forwardRef { - return child instanceof HTMLElement && child.getAttribute('data-component') === 'TitleArea' + return child instanceof HTMLElement && child.getAttribute('data-component') === 'PageHeader.TitleArea' }) // It is very unlikely to have a PageHeader without a TitleArea, but we still want to make sure we don't break the page if that happens. @@ -107,6 +107,7 @@ const Root = React.forwardRef> hidden = hiddenOnRegularAndWide, }) => { return ( -
+
{children}
) @@ -154,6 +159,7 @@ const ParentLink = React.forwardRef( aria-label={ariaLabel} muted className={clsx(classes.ParentLink, className)} + data-component="PageHeader.ParentLink" {...getHiddenDataAttributes(hidden)} href={href} > @@ -176,7 +182,11 @@ const ContextBar: React.FC> = ({ hidden = hiddenOnRegularAndWide, }) => { return ( -
+
{children}
) @@ -190,7 +200,11 @@ const ContextAreaActions: React.FC> = hidden = hiddenOnRegularAndWide, }) => { return ( -
+
{children}
) @@ -211,7 +225,7 @@ const TitleArea = React.forwardRef @@ -232,7 +246,7 @@ const LeadingAction: FCWithSlotMarker return (
{children} @@ -247,7 +261,7 @@ const Breadcrumbs: React.FC> = ({chil return (
{children} @@ -260,7 +274,7 @@ const LeadingVisual: React.FC> = ({ch return (
{children} @@ -276,7 +290,7 @@ const Title: React.FC> = ({children, classNa return ( > = ({ return (
{children} @@ -311,7 +325,7 @@ const TrailingAction: React.FC> = ({ return (
{children} @@ -323,7 +337,11 @@ export type ActionsProps = React.PropsWithChildren const Actions = ({children, className, hidden = false}: ActionsProps) => { return ( -
+
{children}
) @@ -332,7 +350,11 @@ const Actions = ({children, className, hidden = false}: ActionsProps) => { // PageHeader.Description: The description area of the header. Visible on all viewports const Description: React.FC> = ({children, className, hidden = false}) => { return ( -
+
{children}
) @@ -364,7 +386,7 @@ const Navigation: React.FC> = ({ aria-label={BaseComponent === 'nav' ? ariaLabel : undefined} aria-labelledby={BaseComponent === 'nav' ? ariaLabelledBy : undefined} className={clsx(classes.Navigation, className)} - data-component="PH_Navigation" + data-component="PageHeader.Navigation" {...getHiddenDataAttributes(hidden)} > {children} From eea0b7dbd02059e9fd0e318ddf8f77cc44c24d53 Mon Sep 17 00:00:00 2001 From: llastflowers Date: Wed, 27 May 2026 13:22:09 -0700 Subject: [PATCH 2/8] update PageLayout.tsx and add test --- .../react/src/PageLayout/PageLayout.test.tsx | 19 ++++++++++++++++++ packages/react/src/PageLayout/PageLayout.tsx | 6 ++++++ .../__snapshots__/PageLayout.test.tsx.snap | 20 +++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/packages/react/src/PageLayout/PageLayout.test.tsx b/packages/react/src/PageLayout/PageLayout.test.tsx index 1e0a02a3941..1b1774a82e6 100644 --- a/packages/react/src/PageLayout/PageLayout.test.tsx +++ b/packages/react/src/PageLayout/PageLayout.test.tsx @@ -102,6 +102,25 @@ describe('PageLayout', async () => { expect(getByText('Pane')).toBeVisible() }) + it('renders data-component attributes for PageLayout and exported subcomponents', () => { + const {container} = render( + + Header + Content + Pane + Sidebar + Footer + , + ) + + expect(container.querySelector('[data-component="PageLayout"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageLayout.Header"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageLayout.Content"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageLayout.Pane"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageLayout.Sidebar"]')).toBeInTheDocument() + expect(container.querySelector('[data-component="PageLayout.Footer"]')).toBeInTheDocument() + }) + it('should support labeling landmarks through `aria-label`', () => { render( diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index bd1ff29733a..7d26c2d62c2 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -130,6 +130,7 @@ const RootWrapper = memo( } as React.CSSProperties } className={clsx(classes.PageLayoutRoot, className)} + data-component="PageLayout" data-has-sidebar={hasSidebar || undefined} > {children} @@ -570,6 +571,7 @@ const Header: FCWithSlotMarker> =
> ref={contentWrapperRef} aria-label={label} aria-labelledby={labelledBy} + data-component="PageLayout.Content" style={style} className={clsx(classes.ContentWrapper, className)} {...getResponsiveAttributes('is-hidden', hidden)} @@ -896,6 +899,7 @@ const Pane = React.forwardRef> =