From a7f75a38fbb8d2b0466308066f5bd7450a2a5a0e Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 8 Sep 2025 15:35:59 -0500 Subject: [PATCH 1/4] feat: remove sx prop from Announce, AriaAlert, AriaStatus --- packages/react/src/live-region/Announce.tsx | 82 ++++++++++--------- packages/react/src/live-region/AriaAlert.tsx | 37 ++++----- packages/react/src/live-region/AriaStatus.tsx | 45 +++++----- .../live-region/__tests__/Announce.test.tsx | 9 -- packages/react/src/utils/polymorphic2.d.ts | 23 ++++++ 5 files changed, 105 insertions(+), 91 deletions(-) create mode 100644 packages/react/src/utils/polymorphic2.d.ts diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index a0c56271544..15d697bb690 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/packages/react/src/live-region/Announce.tsx @@ -1,50 +1,56 @@ import {announceFromElement} from '@primer/live-region-element' import type React from 'react' import {useEffect, useRef, useState, type ElementRef} from 'react' -import Box from '../Box' import {useEffectOnce} from '../internal/hooks/useEffectOnce' import {useEffectCallback} from '../internal/hooks/useEffectCallback' - -export type AnnounceProps = React.ComponentPropsWithoutRef & { - /** - * Specify if the content of the element should be announced when this - * component is rendered and is not hidden - * @default false - */ - announceOnShow?: boolean - - /** - * Specify if the element is hidden - * @default false - */ - hidden?: boolean - - /** - * Provide a delay in milliseconds before the announcement is made. This will - * only work with `polite` announcements - */ - delayMs?: number - - /** - * The politeness level to use for the announcement - * @default 'polite' - */ - politeness?: 'assertive' | 'polite' -} +import type {PolymorphicProps} from '../utils/polymorphic2' + +export type AnnounceProps = PolymorphicProps< + 'div', + As, + { + /** + * Specify if the content of the element should be announced when this + * component is rendered and is not hidden + * @default false + */ + announceOnShow?: boolean + + /** + * Specify if the element is hidden + * @default false + */ + hidden?: boolean + + /** + * Provide a delay in milliseconds before the announcement is made. This will + * only work with `polite` announcements + */ + delayMs?: number + + /** + * The politeness level to use for the announcement + * @default 'polite' + */ + politeness?: 'assertive' | 'polite' + } +> /** * `Announce` is a component that will announce the text content of the * `children` passed in to screen readers using the given politeness level. It * will also announce any changes to the text content of `children` */ -export function Announce({ - announceOnShow = true, - children, - delayMs, - hidden = false, - politeness = 'polite', - ...rest -}: AnnounceProps) { +export function Announce(props: AnnounceProps) { + const { + as: BaseComponent = 'div', + announceOnShow = true, + children, + delayMs, + hidden = false, + politeness = 'polite', + ...rest + } = props const ref = useRef>(null) const [previousAnnouncementText, setPreviousAnnouncementText] = useState(null) const savedAnnouncement = useRef | null>(null) @@ -127,9 +133,9 @@ export function Announce({ }, []) return ( - + {children} - + ) } diff --git a/packages/react/src/live-region/AriaAlert.tsx b/packages/react/src/live-region/AriaAlert.tsx index a961180521d..c49cdc6290e 100644 --- a/packages/react/src/live-region/AriaAlert.tsx +++ b/packages/react/src/live-region/AriaAlert.tsx @@ -1,29 +1,26 @@ import type React from 'react' import {type ElementType} from 'react' import {Announce} from './Announce' -import type {SxProp} from '../sx' -export type AriaAlertProps = React.PropsWithChildren< - { - /** - * Customize the element type of the rendered container - */ - as?: As +export type AriaAlertProps = React.PropsWithChildren<{ + /** + * Customize the element type of the rendered container + */ + as?: As - /** - * Specify if the content of the element should be announced when this - * component is rendered and is not hidden - * @default true - */ - announceOnShow?: boolean + /** + * Specify if the content of the element should be announced when this + * component is rendered and is not hidden + * @default true + */ + announceOnShow?: boolean - /** - * Specify if the element is hidden - * @default false - */ - hidden?: boolean - } & SxProp -> + /** + * Specify if the element is hidden + * @default false + */ + hidden?: boolean +}> export function AriaAlert({ announceOnShow = true, diff --git a/packages/react/src/live-region/AriaStatus.tsx b/packages/react/src/live-region/AriaStatus.tsx index fa1f90df28c..b443b244093 100644 --- a/packages/react/src/live-region/AriaStatus.tsx +++ b/packages/react/src/live-region/AriaStatus.tsx @@ -1,34 +1,31 @@ import type React from 'react' import {type ElementType} from 'react' import {Announce} from './Announce' -import type {SxProp} from '../sx' -export type AriaStatusProps = React.PropsWithChildren< - { - /** - * Customize the element type of the rendered container - */ - as?: As +export type AriaStatusProps = React.PropsWithChildren<{ + /** + * Customize the element type of the rendered container + */ + as?: As - /** - * Specify if the content of the element should be announced when this - * component is rendered and is not hidden - * @default false - */ - announceOnShow?: boolean + /** + * Specify if the content of the element should be announced when this + * component is rendered and is not hidden + * @default false + */ + announceOnShow?: boolean - /** - * Specify if the element is hidden - * @default false - */ - hidden?: boolean + /** + * Specify if the element is hidden + * @default false + */ + hidden?: boolean - /** - * Provide a delay in milliseconds before the announcement is made - */ - delayMs?: number - } & SxProp -> + /** + * Provide a delay in milliseconds before the announcement is made + */ + delayMs?: number +}> export function AriaStatus({ announceOnShow = false, diff --git a/packages/react/src/live-region/__tests__/Announce.test.tsx b/packages/react/src/live-region/__tests__/Announce.test.tsx index d96ab799508..2940b5a7888 100644 --- a/packages/react/src/live-region/__tests__/Announce.test.tsx +++ b/packages/react/src/live-region/__tests__/Announce.test.tsx @@ -37,15 +37,6 @@ describe('Announce', () => { expect(container.firstChild).toHaveAttribute('data-testid', 'container') }) - it('should support styling via the `sx` prop', () => { - render( - - test - , - ) - expect(screen.getByTestId('container')).toHaveStyle('color: rgb(0, 0, 255)') - }) - it('should support customizing the container element with `as`', () => { render( diff --git a/packages/react/src/utils/polymorphic2.d.ts b/packages/react/src/utils/polymorphic2.d.ts new file mode 100644 index 00000000000..821157902d7 --- /dev/null +++ b/packages/react/src/utils/polymorphic2.d.ts @@ -0,0 +1,23 @@ +/** + * This file is an alternative to polymorphic.ts that hopes to support + * polyrmophic components in React. It explicitly hopes to make it easy to + * type the props of components and allow for explicitly setting the type of a + * component + */ + +import type {ElementType} from 'react' + +type AsProp = { + /** + * Customize the element type of the container element for the component + */ + as?: As +} + +type PolymorphicProps = Props & + AsProp & + (As extends React.ElementType + ? Omit, keyof Props> + : Omit, keyof Props>) + +export type {PolymorphicProps} From b9f3c54a8a5a3cd09cb83c26e2890f26762cfa0f Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 8 Sep 2025 15:36:45 -0500 Subject: [PATCH 2/4] chore: add changeset --- .changeset/spotty-colts-hear.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spotty-colts-hear.md diff --git a/.changeset/spotty-colts-hear.md b/.changeset/spotty-colts-hear.md new file mode 100644 index 00000000000..bbf482df079 --- /dev/null +++ b/.changeset/spotty-colts-hear.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Remove the `sx` prop from `Announce`, `AriaAlert`, and `AriaStatus` From 3583d7876a3d5cc40b0f44e293875d958eafa20d Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 8 Sep 2025 15:41:40 -0500 Subject: [PATCH 3/4] refactor: update polymorphic types in AriaAlert, AriaStatus --- packages/react/src/live-region/AriaAlert.tsx | 40 ++++++++-------- packages/react/src/live-region/AriaStatus.tsx | 48 +++++++++---------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/packages/react/src/live-region/AriaAlert.tsx b/packages/react/src/live-region/AriaAlert.tsx index c49cdc6290e..7f9e48fa47f 100644 --- a/packages/react/src/live-region/AriaAlert.tsx +++ b/packages/react/src/live-region/AriaAlert.tsx @@ -1,32 +1,32 @@ import type React from 'react' import {type ElementType} from 'react' import {Announce} from './Announce' +import type {PolymorphicProps} from '../utils/polymorphic2' -export type AriaAlertProps = React.PropsWithChildren<{ - /** - * Customize the element type of the rendered container - */ - as?: As +export type AriaAlertProps = PolymorphicProps< + 'div', + As, + { + /** + * Specify if the content of the element should be announced when this + * component is rendered and is not hidden + * @default true + */ + announceOnShow?: boolean - /** - * Specify if the content of the element should be announced when this - * component is rendered and is not hidden - * @default true - */ - announceOnShow?: boolean + /** + * Specify if the element is hidden + * @default false + */ + hidden?: boolean + } +> - /** - * Specify if the element is hidden - * @default false - */ - hidden?: boolean -}> - -export function AriaAlert({ +export function AriaAlert({ announceOnShow = true, children, ...rest -}: AriaAlertProps & React.ComponentPropsWithoutRef) { +}: AriaAlertProps) { return ( {children} diff --git a/packages/react/src/live-region/AriaStatus.tsx b/packages/react/src/live-region/AriaStatus.tsx index b443b244093..a9a4ff13aac 100644 --- a/packages/react/src/live-region/AriaStatus.tsx +++ b/packages/react/src/live-region/AriaStatus.tsx @@ -1,37 +1,37 @@ import type React from 'react' import {type ElementType} from 'react' import {Announce} from './Announce' +import type {PolymorphicProps} from '../utils/polymorphic2' -export type AriaStatusProps = React.PropsWithChildren<{ - /** - * Customize the element type of the rendered container - */ - as?: As +export type AriaStatusProps = PolymorphicProps< + 'div', + As, + { + /** + * Specify if the content of the element should be announced when this + * component is rendered and is not hidden + * @default false + */ + announceOnShow?: boolean - /** - * Specify if the content of the element should be announced when this - * component is rendered and is not hidden - * @default false - */ - announceOnShow?: boolean + /** + * Specify if the element is hidden + * @default false + */ + hidden?: boolean - /** - * Specify if the element is hidden - * @default false - */ - hidden?: boolean + /** + * Provide a delay in milliseconds before the announcement is made + */ + delayMs?: number + } +> - /** - * Provide a delay in milliseconds before the announcement is made - */ - delayMs?: number -}> - -export function AriaStatus({ +export function AriaStatus({ announceOnShow = false, children, ...rest -}: AriaStatusProps & React.ComponentPropsWithoutRef) { +}: AriaStatusProps) { return ( {children} From ac61422a54b934660d88ce3561daf29ad34079e3 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 12 Sep 2025 10:20:13 -0500 Subject: [PATCH 4/4] chore: fix lint and tsc issues --- .../react/src/live-region/__tests__/AriaAlert.test.tsx | 9 --------- .../react/src/live-region/__tests__/AriaStatus.test.tsx | 9 --------- .../src/utils/{polymorphic2.d.ts => polymorphic2.ts} | 2 +- 3 files changed, 1 insertion(+), 19 deletions(-) rename packages/react/src/utils/{polymorphic2.d.ts => polymorphic2.ts} (95%) diff --git a/packages/react/src/live-region/__tests__/AriaAlert.test.tsx b/packages/react/src/live-region/__tests__/AriaAlert.test.tsx index 44f7a216efb..3ea2d2548b1 100644 --- a/packages/react/src/live-region/__tests__/AriaAlert.test.tsx +++ b/packages/react/src/live-region/__tests__/AriaAlert.test.tsx @@ -31,15 +31,6 @@ describe('AriaAlert', () => { expect(container.firstChild).toHaveAttribute('data-testid', 'container') }) - it('should support styling via the `sx` prop', () => { - render( - - test - , - ) - expect(screen.getByTestId('container')).toHaveStyle('color: rgb(0, 0, 255)') - }) - it('should support customizing the container element with `as`', () => { render( diff --git a/packages/react/src/live-region/__tests__/AriaStatus.test.tsx b/packages/react/src/live-region/__tests__/AriaStatus.test.tsx index ad7290ea8d5..0e7380b6798 100644 --- a/packages/react/src/live-region/__tests__/AriaStatus.test.tsx +++ b/packages/react/src/live-region/__tests__/AriaStatus.test.tsx @@ -67,15 +67,6 @@ describe('AriaStatus', () => { expect(container.firstChild).toHaveAttribute('data-testid', 'container') }) - it('should support styling via the `sx` prop', () => { - render( - - test - , - ) - expect(screen.getByTestId('container')).toHaveStyle('color: rgb(0, 0, 255)') - }) - it('should support customizing the container element with `as`', () => { render( diff --git a/packages/react/src/utils/polymorphic2.d.ts b/packages/react/src/utils/polymorphic2.ts similarity index 95% rename from packages/react/src/utils/polymorphic2.d.ts rename to packages/react/src/utils/polymorphic2.ts index 821157902d7..b48264baa27 100644 --- a/packages/react/src/utils/polymorphic2.d.ts +++ b/packages/react/src/utils/polymorphic2.ts @@ -14,7 +14,7 @@ type AsProp = { as?: As } -type PolymorphicProps = Props & +type PolymorphicProps = Props & AsProp & (As extends React.ElementType ? Omit, keyof Props>