From e17c788d360626673e549d7b0f19e8e28e38770f Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Tue, 8 Apr 2025 00:27:01 +0000 Subject: [PATCH 1/3] Remove the CSS modules feature flag from the ActionList.Heading component --- .changeset/beige-bags-look.md | 5 + packages/react/src/ActionList/Heading.tsx | 106 +++++++--------------- 2 files changed, 36 insertions(+), 75 deletions(-) create mode 100644 .changeset/beige-bags-look.md diff --git a/.changeset/beige-bags-look.md b/.changeset/beige-bags-look.md new file mode 100644 index 00000000000..3b3fbb8755f --- /dev/null +++ b/.changeset/beige-bags-look.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Remove the CSS modules feature flag from the ActionList.Heading component diff --git a/packages/react/src/ActionList/Heading.tsx b/packages/react/src/ActionList/Heading.tsx index a100a3fef74..55f858f6db2 100644 --- a/packages/react/src/ActionList/Heading.tsx +++ b/packages/react/src/ActionList/Heading.tsx @@ -1,7 +1,5 @@ import React, {forwardRef} from 'react' -import type {BetterSystemStyleObject, SxProp} from '../sx' -import {merge} from '../sx' -import {defaultSxProp} from '../utils/defaultSxProp' +import type {SxProp} from '../sx' import {useRefObjectAsForwardedRef} from '../hooks' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {default as HeadingComponent} from '../Heading' @@ -10,9 +8,7 @@ import VisuallyHidden from '../_VisuallyHidden' import {ActionListContainerContext} from './ActionListContainerContext' import {invariant} from '../utils/invariant' import {clsx} from 'clsx' -import {useFeatureFlag} from '../FeatureFlags' import classes from './Heading.module.css' -import {actionListCssModulesFlag} from './featureflag' type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' type HeadingVariants = 'large' | 'medium' | 'small' @@ -23,75 +19,35 @@ export type ActionListHeadingProps = { className?: string } & SxProp -export const Heading = forwardRef( - ({as, size, children, sx = defaultSxProp, visuallyHidden = false, className, ...props}, forwardedRef) => { - const innerRef = React.useRef(null) - useRefObjectAsForwardedRef(forwardedRef, innerRef) - - const enabled = useFeatureFlag(actionListCssModulesFlag) - - const {headingId: headingId, variant: listVariant} = React.useContext(ListContext) - const {container} = React.useContext(ActionListContainerContext) - - // Semantic s don't have a place for headers within them, they should be aria-labelledby the menu button's name. - invariant( - container !== 'ActionMenu', - `ActionList.Heading shouldn't be used within an ActionMenu container. Menus are labelled by the menu button's name.`, - ) - - const styles = { - marginBottom: 2, - marginX: listVariant === 'full' ? 2 : 3, - } - - return ( - - {enabled ? ( - sx !== defaultSxProp ? ( - - {children} - - ) : ( - - {children} - - ) - ) : ( - (styles, sx)} - className={className} - {...props} - > - {children} - - )} - - ) - }, -) as PolymorphicForwardRefComponent +export const Heading = forwardRef(({as, size, children, visuallyHidden = false, className, ...props}, forwardedRef) => { + const innerRef = React.useRef(null) + useRefObjectAsForwardedRef(forwardedRef, innerRef) + + const {headingId: headingId, variant: listVariant} = React.useContext(ListContext) + const {container} = React.useContext(ActionListContainerContext) + + // Semantic s don't have a place for headers within them, they should be aria-labelledby the menu button's name. + invariant( + container !== 'ActionMenu', + `ActionList.Heading shouldn't be used within an ActionMenu container. Menus are labelled by the menu button's name.`, + ) + + return ( + + + {children} + + + ) +}) as PolymorphicForwardRefComponent Heading.displayName = 'ActionList.Heading' From b1f7b0a2d74413dd0f18562e7608d6dab906fa6b Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Tue, 8 Apr 2025 00:29:57 +0000 Subject: [PATCH 2/3] Remove flag check from test --- .../react/src/ActionList/Heading.test.tsx | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/packages/react/src/ActionList/Heading.test.tsx b/packages/react/src/ActionList/Heading.test.tsx index e87cd4133de..fdb2e73c3d7 100644 --- a/packages/react/src/ActionList/Heading.test.tsx +++ b/packages/react/src/ActionList/Heading.test.tsx @@ -3,7 +3,6 @@ import React from 'react' import theme from '../theme' import {ActionList} from '.' import {BaseStyles, ThemeProvider, ActionMenu} from '..' -import {FeatureFlags} from '../FeatureFlags' import {behavesAsComponent} from '../utils/testing' describe('ActionList.Heading', () => { @@ -62,28 +61,14 @@ describe('ActionList.Heading', () => { }) it('should support a custom `className` on the outermost element', () => { - const Element = () => { - return ( + expect(() => + HTMLRender( Filter by - - ) - } - const FeatureFlagElement = () => { - return ( - - - - ) - } - expect(HTMLRender().container.querySelector('h2')).toHaveClass('test-class-name') - expect(HTMLRender().container.querySelector('h2')).toHaveClass('test-class-name') + , + ).container.querySelector('h2'), + ).toHaveClass('test-class-name') }) }) From 670475d84dbdb7fa8927748f96b2059e96f60340 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Tue, 8 Apr 2025 00:56:42 +0000 Subject: [PATCH 3/3] Fix test --- packages/react/src/ActionList/Heading.test.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/react/src/ActionList/Heading.test.tsx b/packages/react/src/ActionList/Heading.test.tsx index fdb2e73c3d7..9eeadf5e588 100644 --- a/packages/react/src/ActionList/Heading.test.tsx +++ b/packages/react/src/ActionList/Heading.test.tsx @@ -61,14 +61,13 @@ describe('ActionList.Heading', () => { }) it('should support a custom `className` on the outermost element', () => { - expect(() => - HTMLRender( - - - Filter by - - , - ).container.querySelector('h2'), - ).toHaveClass('test-class-name') + const actionList = HTMLRender( + + + Filter by + + , + ) + expect(actionList.container.querySelector('h2')).toHaveClass('test-class-name') }) })