From b064ad403067b12a921804cab9624d2471be2298 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 6 Feb 2025 18:51:31 -0500 Subject: [PATCH 1/9] Add `disabled` prop to `ActionBar` --- packages/react/src/ActionBar/ActionBar.docs.json | 6 ++++++ .../src/ActionBar/ActionBar.examples.stories.tsx | 16 ++++++++++++++++ packages/react/src/ActionBar/ActionBar.tsx | 13 ++++++++++--- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.docs.json b/packages/react/src/ActionBar/ActionBar.docs.json index c658ac11d1f..7dd0a7bcf0b 100644 --- a/packages/react/src/ActionBar/ActionBar.docs.json +++ b/packages/react/src/ActionBar/ActionBar.docs.json @@ -75,6 +75,12 @@ "type": "string", "defaultValue": "", "description": "Use an aria label to describe the functionality of the button. Please refer to [our guidance on alt text](https://primer.style/guides/accessibility/alternative-text-for-images) for tips on writing good alternative text." + }, + { + "name": "disabled", + "type": "boolean", + "defaultValue": "", + "description": "Provides a disabled state for the button. The button will remain focusable, and have `aria-disabled` applied." } ], "passthrough": { diff --git a/packages/react/src/ActionBar/ActionBar.examples.stories.tsx b/packages/react/src/ActionBar/ActionBar.examples.stories.tsx index d80446b9d41..23be81dd12f 100644 --- a/packages/react/src/ActionBar/ActionBar.examples.stories.tsx +++ b/packages/react/src/ActionBar/ActionBar.examples.stories.tsx @@ -47,6 +47,22 @@ export const SmallActionBar = () => ( ) +export const WithDisabledItems = () => ( + + + + + + + + + + + + + +) + type CommentBoxProps = {'aria-label': string} export const CommentBox = (props: CommentBoxProps) => { diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 8740c89673c..22fa2910fb8 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -46,7 +46,7 @@ export type ActionBarProps = { className?: string } & A11yProps -export type ActionBarIconButtonProps = IconButtonProps +export type ActionBarIconButtonProps = {disabled?: boolean} & IconButtonProps const MORE_BTN_WIDTH = 86 @@ -215,7 +215,13 @@ export const ActionBar: React.FC> = prop if (menuItem.type === ActionList.Divider) { return } else { - const {children: menuItemChildren, onClick, icon: Icon, 'aria-label': ariaLabel} = menuItem.props + const { + children: menuItemChildren, + onClick, + icon: Icon, + 'aria-label': ariaLabel, + disabled, + } = menuItem.props return ( > = prop focusOnMoreMenuBtn() typeof onClick === 'function' && onClick(event) }} + disabled={disabled} > {Icon ? ( @@ -254,7 +261,7 @@ export const ActionBarIconButton = forwardRef((props: ActionBarIconButtonProps, const domRect = (ref as MutableRefObject).current.getBoundingClientRect() setChildrenWidth({text, width: domRect.width}) }, [ref, setChildrenWidth]) - return + return }) export const VerticalDivider = () => { From 9cf870aa1e3079a2eeebec337aaa377c857490b6 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Sun, 2 Mar 2025 10:02:35 -0600 Subject: [PATCH 2/9] Improve `disabled` support --- .../react/src/ActionBar/ActionBar.test.tsx | 14 ++++++ packages/react/src/ActionBar/ActionBar.tsx | 44 ++++++++++++++----- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.test.tsx b/packages/react/src/ActionBar/ActionBar.test.tsx index d9d9ab45827..6650ff9a969 100644 --- a/packages/react/src/ActionBar/ActionBar.test.tsx +++ b/packages/react/src/ActionBar/ActionBar.test.tsx @@ -32,4 +32,18 @@ describe('ActionBar', () => { const results = await axe.run(container) expect(results).toHaveNoViolations() }) + + it('should not trigger disabled button', () => { + const onClick = jest.fn() + const {getByRole} = HTMLRender( + + + , + ) + + const button = getByRole('button') + button.click() + + expect(onClick).not.toHaveBeenCalled() + }) }) diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 22fa2910fb8..3e3d87681df 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -252,17 +252,39 @@ export const ActionBar: React.FC> = prop ) } -export const ActionBarIconButton = forwardRef((props: ActionBarIconButtonProps, forwardedRef) => { - const backupRef = useRef(null) - const ref = (forwardedRef ?? backupRef) as RefObject - const {size, setChildrenWidth} = React.useContext(ActionBarContext) - useIsomorphicLayoutEffect(() => { - const text = props['aria-label'] ? props['aria-label'] : '' - const domRect = (ref as MutableRefObject).current.getBoundingClientRect() - setChildrenWidth({text, width: domRect.width}) - }, [ref, setChildrenWidth]) - return -}) +export const ActionBarIconButton = forwardRef( + ({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => { + const backupRef = useRef(null) + const ref = (forwardedRef ?? backupRef) as RefObject + const {size, setChildrenWidth} = React.useContext(ActionBarContext) + useIsomorphicLayoutEffect(() => { + const text = props['aria-label'] ? props['aria-label'] : '' + const domRect = (ref as MutableRefObject).current.getBoundingClientRect() + setChildrenWidth({text, width: domRect.width}) + }, [ref, setChildrenWidth]) + + const clickHandler = useCallback( + (event: React.MouseEvent) => { + if (disabled) { + return + } + onClick?.(event) + }, + [disabled, onClick], + ) + + return ( + + ) + }, +) export const VerticalDivider = () => { const ref = useRef(null) From 6890c893a18d7e91542798ab378bcb1b8088f400 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Sun, 2 Mar 2025 11:01:48 -0600 Subject: [PATCH 3/9] Add changeset --- .changeset/sixty-otters-lie.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sixty-otters-lie.md diff --git a/.changeset/sixty-otters-lie.md b/.changeset/sixty-otters-lie.md new file mode 100644 index 00000000000..fd11e567ce8 --- /dev/null +++ b/.changeset/sixty-otters-lie.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +ActionBar: Improves `disabled` state on `ActionBar.IconButton`; includes `disabled` state in overflow menu From 04ed5500fd1fb0ccf5941294f85e54f42df37b83 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 5 Mar 2025 13:43:37 -0800 Subject: [PATCH 4/9] Add handler for keydown events --- packages/react/src/ActionBar/ActionBar.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 3e3d87681df..a216b073f7e 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -253,7 +253,7 @@ export const ActionBar: React.FC> = prop } export const ActionBarIconButton = forwardRef( - ({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => { + ({disabled, onClick, onKeyDown, ...props}: ActionBarIconButtonProps, forwardedRef) => { const backupRef = useRef(null) const ref = (forwardedRef ?? backupRef) as RefObject const {size, setChildrenWidth} = React.useContext(ActionBarContext) @@ -273,12 +273,26 @@ export const ActionBarIconButton = forwardRef( [disabled, onClick], ) + const keyDownHandler = React.useCallback( + (event: React.KeyboardEvent) => { + if (disabled) return + if ([' ', 'Enter'].includes(event.key)) { + if (event.key === ' ') { + event.preventDefault() + } + onKeyDown?.(event) + } + }, + [disabled, onKeyDown], + ) + return ( From 5b7ff2d3a8b6200e25af0429515a73fff12e8675 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 6 Mar 2025 07:14:27 -0800 Subject: [PATCH 5/9] Add more tests --- .../react/src/ActionBar/ActionBar.test.tsx | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/packages/react/src/ActionBar/ActionBar.test.tsx b/packages/react/src/ActionBar/ActionBar.test.tsx index 6650ff9a969..846d59236be 100644 --- a/packages/react/src/ActionBar/ActionBar.test.tsx +++ b/packages/react/src/ActionBar/ActionBar.test.tsx @@ -1,6 +1,7 @@ import React from 'react' import {behavesAsComponent} from '../utils/testing' -import {render as HTMLRender} from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import {render as HTMLRender, act} from '@testing-library/react' import axe from 'axe-core' import ActionBar from './' @@ -46,4 +47,63 @@ describe('ActionBar', () => { expect(onClick).not.toHaveBeenCalled() }) + + it('should trigger non-disabled button', () => { + const onClick = jest.fn() + const {getByRole} = HTMLRender( + + + , + ) + + const button = getByRole('button') + button.click() + + expect(onClick).toHaveBeenCalled() + }) + + it('should not trigger disabled button with spacebar or enter', async () => { + const user = userEvent.setup() + const onKeyDown = jest.fn() + const {getByRole} = HTMLRender( + + + , + ) + + const button = getByRole('button') + + act(() => { + button.focus() + }) + + await user.keyboard('{Enter}') + + expect(onKeyDown).not.toHaveBeenCalled() + }) + + it('should trigger non-disabled button with spacebar or enter', async () => { + const user = userEvent.setup() + const onKeyDown = jest.fn() + const {getByRole} = HTMLRender( + + + , + ) + + const button = getByRole('button') + + act(() => { + button.focus() + }) + + await user.keyboard('{Enter}') + + expect(onKeyDown).toHaveBeenCalled() + }) }) From 2cfe096558213d34247335471957151be318d755 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 6 Mar 2025 07:27:10 -0800 Subject: [PATCH 6/9] Quick fix for consistency --- packages/react/src/ActionBar/ActionBar.tsx | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index a216b073f7e..5f216ca6788 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -252,6 +252,22 @@ export const ActionBar: React.FC> = prop ) } +interface RepoPickerProps { + id: string + name: string + ownerLogin: string + visibility: 'public' | 'private' | 'internal' + selected?: boolean +} + +// { +// id: string, required: false +// name: string, required: true +// ownerLogin: string, required: true +// visibility: 'public' | 'private' | 'internal', required: true +// selected: boolean, required: false +// } + export const ActionBarIconButton = forwardRef( ({disabled, onClick, onKeyDown, ...props}: ActionBarIconButtonProps, forwardedRef) => { const backupRef = useRef(null) @@ -265,9 +281,7 @@ export const ActionBarIconButton = forwardRef( const clickHandler = useCallback( (event: React.MouseEvent) => { - if (disabled) { - return - } + if (disabled) return onClick?.(event) }, [disabled, onClick], From 5dbed551e51f20211432386b6d27ffef53fa0db2 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 6 Mar 2025 07:27:55 -0800 Subject: [PATCH 7/9] clean up --- packages/react/src/ActionBar/ActionBar.tsx | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 5f216ca6788..ecbdb9e724a 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -252,22 +252,6 @@ export const ActionBar: React.FC> = prop ) } -interface RepoPickerProps { - id: string - name: string - ownerLogin: string - visibility: 'public' | 'private' | 'internal' - selected?: boolean -} - -// { -// id: string, required: false -// name: string, required: true -// ownerLogin: string, required: true -// visibility: 'public' | 'private' | 'internal', required: true -// selected: boolean, required: false -// } - export const ActionBarIconButton = forwardRef( ({disabled, onClick, onKeyDown, ...props}: ActionBarIconButtonProps, forwardedRef) => { const backupRef = useRef(null) From b0ba29f14ce8e6b9d6acf2ae47cc1870268ef45b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 6 Mar 2025 20:50:00 -0800 Subject: [PATCH 8/9] Remove `onKeyDown` --- packages/react/src/ActionBar/ActionBar.tsx | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index ecbdb9e724a..8cf96b676b0 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -253,7 +253,7 @@ export const ActionBar: React.FC> = prop } export const ActionBarIconButton = forwardRef( - ({disabled, onClick, onKeyDown, ...props}: ActionBarIconButtonProps, forwardedRef) => { + ({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => { const backupRef = useRef(null) const ref = (forwardedRef ?? backupRef) as RefObject const {size, setChildrenWidth} = React.useContext(ActionBarContext) @@ -271,26 +271,12 @@ export const ActionBarIconButton = forwardRef( [disabled, onClick], ) - const keyDownHandler = React.useCallback( - (event: React.KeyboardEvent) => { - if (disabled) return - if ([' ', 'Enter'].includes(event.key)) { - if (event.key === ' ') { - event.preventDefault() - } - onKeyDown?.(event) - } - }, - [disabled, onKeyDown], - ) - return ( From b6266152070a92790996bb2bd9b84297cf4048b9 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 6 Mar 2025 20:55:49 -0800 Subject: [PATCH 9/9] Adjust tests --- packages/react/src/ActionBar/ActionBar.test.tsx | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/react/src/ActionBar/ActionBar.test.tsx b/packages/react/src/ActionBar/ActionBar.test.tsx index 846d59236be..44a0c25bcf6 100644 --- a/packages/react/src/ActionBar/ActionBar.test.tsx +++ b/packages/react/src/ActionBar/ActionBar.test.tsx @@ -64,15 +64,10 @@ describe('ActionBar', () => { it('should not trigger disabled button with spacebar or enter', async () => { const user = userEvent.setup() - const onKeyDown = jest.fn() + const onClick = jest.fn() const {getByRole} = HTMLRender( - + , ) @@ -84,15 +79,15 @@ describe('ActionBar', () => { await user.keyboard('{Enter}') - expect(onKeyDown).not.toHaveBeenCalled() + expect(onClick).not.toHaveBeenCalled() }) it('should trigger non-disabled button with spacebar or enter', async () => { const user = userEvent.setup() - const onKeyDown = jest.fn() + const onClick = jest.fn() const {getByRole} = HTMLRender( - + , ) @@ -104,6 +99,6 @@ describe('ActionBar', () => { await user.keyboard('{Enter}') - expect(onKeyDown).toHaveBeenCalled() + expect(onClick).toHaveBeenCalled() }) })