Skip to content

Commit

Permalink
Default ConfirmationDialog to focusing cancel when the confirmation…
Browse files Browse the repository at this point in the history
… is a dangerous one (#2185)

* allow configuration of the initial focus in a confirmation dialog

* changeset

* explicitly set the autoFocus when danger to cancel

* intermediate value

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
  • Loading branch information
mattcosta7 and siddharthkp committed Aug 1, 2022
1 parent 5321f1c commit 3756a1e
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-pianos-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Set ConfirmationDialog initial focus based on the confirmationButtonVariant. When `danger` autoFocus the cancel button, otherwise autoFocus the confirmation button
6 changes: 4 additions & 2 deletions src/Dialog/ConfirmationDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,17 @@ export const ConfirmationDialog: React.FC<React.PropsWithChildren<ConfirmationDi
const onConfirmButtonClick = useCallback(() => {
onClose('confirm')
}, [onClose])
const isConfirmationDangerous = confirmButtonType === 'danger'
const cancelButton: DialogButtonProps = {
content: cancelButtonContent,
onClick: onCancelButtonClick
onClick: onCancelButtonClick,
autoFocus: isConfirmationDangerous
}
const confirmButton: DialogButtonProps = {
content: confirmButtonContent,
buttonType: confirmButtonType,
onClick: onConfirmButtonClick,
autoFocus: true
autoFocus: !isConfirmationDangerous
}
const footerButtons = [cancelButton, confirmButton]
return (
Expand Down
25 changes: 23 additions & 2 deletions src/__tests__/ConfirmationDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {behavesAsComponent, checkExports} from '../utils/testing'

expect.extend(toHaveNoViolations)

const Basic = () => {
const Basic = ({confirmButtonType}: Pick<React.ComponentProps<typeof ConfirmationDialog>, 'confirmButtonType'>) => {
const [isOpen, setIsOpen] = useState(false)
const buttonRef = useRef<HTMLButtonElement>(null)
const onDialogClose = useCallback(() => setIsOpen(false), [])
Expand All @@ -32,6 +32,7 @@ const Basic = () => {
onClose={onDialogClose}
cancelButtonContent="Secondary"
confirmButtonContent="Primary"
confirmButtonType={confirmButtonType}
>
Lorem ipsum dolor sit Pippin good dog.
</ConfirmationDialog>
Expand Down Expand Up @@ -95,7 +96,7 @@ describe('ConfirmationDialog', () => {
cleanup()
})

it('focuses the primary action when opened', async () => {
it('focuses the primary action when opened and the confirmButtonType is not set', async () => {
const {getByText} = HTMLRender(<Basic />)
act(() => {
fireEvent.click(getByText('Show dialog'))
Expand All @@ -105,6 +106,26 @@ describe('ConfirmationDialog', () => {
cleanup()
})

it('focuses the primary action when opened and the confirmButtonType is not danger', async () => {
const {getByText} = HTMLRender(<Basic confirmButtonType="primary" />)
act(() => {
fireEvent.click(getByText('Show dialog'))
})
expect(getByText('Primary')).toEqual(document.activeElement)
expect(getByText('Secondary')).not.toEqual(document.activeElement)
cleanup()
})

it('focuses the secondary action when opened and the confirmButtonType is danger', async () => {
const {getByText} = HTMLRender(<Basic confirmButtonType="danger" />)
act(() => {
fireEvent.click(getByText('Show dialog'))
})
expect(getByText('Primary')).not.toEqual(document.activeElement)
expect(getByText('Secondary')).toEqual(document.activeElement)
cleanup()
})

it('supports nested `focusTrap`s', async () => {
const {getByText} = HTMLRender(<ShorthandHookFromActionMenu />)
act(() => {
Expand Down

0 comments on commit 3756a1e

Please sign in to comment.