Skip to content

Commit

Permalink
feat: Focus the primary action in 'ConfirmationDialog' (#1471)
Browse files Browse the repository at this point in the history
* feat: Focus the primary action in 'ConfirmationDialog'

* chore: Add changeset

* chore: Add tests for 'ConfirmationDialog'

* Remove systemPropArray

It was removed in #1473

Co-authored-by: Cole Bemis <colebemis@github.com>
  • Loading branch information
smockle and colebemis committed Sep 30, 2021
1 parent 8171b02 commit f1cebb7
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/popular-singers-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

Change the button which receives focus when a 'ConfirmationDialog' opens from the secondary (e.g. 'Cancel') to the primary (e.g. 'OK'). Fixes github/primer#313.
6 changes: 3 additions & 3 deletions src/Dialog/ConfirmationDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,13 @@ export const ConfirmationDialog: React.FC<ConfirmationDialogProps> = props => {
}, [onClose])
const cancelButton: DialogButtonProps = {
content: cancelButtonContent,
onClick: onCancelButtonClick,
autoFocus: true
onClick: onCancelButtonClick
}
const confirmButton: DialogButtonProps = {
content: confirmButtonContent,
buttonType: confirmButtonType,
onClick: onConfirmButtonClick
onClick: onConfirmButtonClick,
autoFocus: true
}
const footerButtons = [cancelButton, confirmButton]
return (
Expand Down
23 changes: 18 additions & 5 deletions src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import styled from 'styled-components'
import Button, {ButtonPrimary, ButtonDanger, ButtonProps} from '../Button'
import Box from '../Box'
import {get, SystemCommonProps, SystemPositionProps, COMMON, POSITION} from '../constants'
import {useOnEscapePress} from '../hooks'
import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks'
import {useFocusTrap} from '../hooks/useFocusTrap'
import sx, {SxProp} from '../sx'
import StyledOcticon from '../StyledOcticon'
Expand Down Expand Up @@ -36,6 +36,12 @@ export type DialogButtonProps = ButtonProps & {
* focus this button automatically when the dialog appears.
*/
autoFocus?: boolean

/**
* A reference to the rendered Button’s DOM node, used together with
* `autoFocus` for `focusTrap`’s `initialFocus`.
*/
ref?: React.RefObject<HTMLButtonElement>
}

/**
Expand Down Expand Up @@ -250,16 +256,23 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
onClose,
role = 'dialog',
width = 'xlarge',
height = 'auto'
height = 'auto',
footerButtons = []
} = props
const dialogLabelId = useSSRSafeId()
const dialogDescriptionId = useSSRSafeId()
const autoFocusedFooterButtonRef = useRef<HTMLButtonElement>(null)
for (const footerButton of footerButtons) {
if (footerButton.autoFocus) {
footerButton.ref = autoFocusedFooterButtonRef
}
}
const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId}

const dialogRef = useRef<HTMLDivElement>(null)
const combinedRef = useCombinedRefs(dialogRef, forwardedRef)
const backdropRef = useRef<HTMLDivElement>(null)
useFocusTrap({containerRef: dialogRef, restoreFocusOnCleanUp: true})
useFocusTrap({containerRef: dialogRef, restoreFocusOnCleanUp: true, initialFocusRef: autoFocusedFooterButtonRef})

useOnEscapePress(
(event: KeyboardEvent) => {
Expand Down Expand Up @@ -338,7 +351,7 @@ const buttonTypes = {
danger: ButtonDanger
}
const Buttons: React.FC<{buttons: DialogButtonProps[]}> = ({buttons}) => {
const autoFocusRef = useRef<HTMLButtonElement>(null)
const autoFocusRef = useProvidedRefOrCreate<HTMLButtonElement>(buttons.find(button => button.autoFocus)?.ref)
let autoFocusCount = 0
const [hasRendered, setHasRendered] = useState(0)
useEffect(() => {
Expand All @@ -348,7 +361,7 @@ const Buttons: React.FC<{buttons: DialogButtonProps[]}> = ({buttons}) => {
} else {
setHasRendered(hasRendered + 1)
}
}, [hasRendered])
}, [autoFocusRef, hasRendered])

return (
<>
Expand Down
120 changes: 120 additions & 0 deletions src/__tests__/ConfirmationDialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import 'babel-polyfill'
import {render as HTMLRender, cleanup, act, fireEvent} from '@testing-library/react'
import {axe, toHaveNoViolations} from 'jest-axe'
import React, {useCallback, useRef, useState} from 'react'

import {ActionMenu} from '../ActionMenu'
import BaseStyles from '../BaseStyles'
import Box from '../Box'
import Button from '../Button/Button'
import {ConfirmationDialog, useConfirm} from '../Dialog/ConfirmationDialog'
import theme from '../theme'
import {ThemeProvider} from '../ThemeProvider'
import {SSRProvider} from '../utils/ssr'
import {behavesAsComponent, checkExports} from '../utils/testing'

expect.extend(toHaveNoViolations)

const Basic = () => {
const [isOpen, setIsOpen] = useState(false)
const buttonRef = useRef<HTMLButtonElement>(null)
const onDialogClose = useCallback(() => setIsOpen(false), [])
return (
<ThemeProvider theme={theme}>
<SSRProvider>
<BaseStyles>
<Button ref={buttonRef} onClick={() => setIsOpen(!isOpen)}>
Show dialog
</Button>
{isOpen && (
<ConfirmationDialog
title="Confirm"
onClose={onDialogClose}
cancelButtonContent="Secondary"
confirmButtonContent="Primary"
>
Lorem ipsum dolor sit Pippin good dog.
</ConfirmationDialog>
)}
</BaseStyles>
</SSRProvider>
</ThemeProvider>
)
}

const ShorthandHookFromActionMenu = () => {
const confirm = useConfirm()
const [text, setText] = useState('Show menu')
const onButtonClick = useCallback(async () => {
if (
await confirm({
title: 'Confirm',
content: 'Confirm',
cancelButtonContent: 'Secondary',
confirmButtonContent: 'Primary'
})
) {
setText('Confirmed')
}
}, [confirm])
return (
<ThemeProvider theme={theme}>
<SSRProvider>
<BaseStyles>
<Box display="flex" flexDirection="column" alignItems="flex-start">
<ActionMenu
renderAnchor={props => <Button {...props}>{text}</Button>}
items={[{text: 'Show dialog', onAction: onButtonClick}]}
/>
</Box>
</BaseStyles>
</SSRProvider>
</ThemeProvider>
)
}

describe('ConfirmationDialog', () => {
behavesAsComponent({
Component: ConfirmationDialog,
toRender: () => <Basic />,
options: {skipAs: true, skipSx: true}
})

checkExports('Dialog/ConfirmationDialog', {
default: undefined,
useConfirm,
ConfirmationDialog
})

it('should have no axe violations', async () => {
const spy = jest.spyOn(console, 'warn').mockImplementation()
const {container} = HTMLRender(<Basic />)
spy.mockRestore()
const results = await axe(container)
expect(results).toHaveNoViolations()
cleanup()
})

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

it('supports nested `focusTrap`s', async () => {
const {getByText} = HTMLRender(<ShorthandHookFromActionMenu />)
act(() => {
fireEvent.click(getByText('Show menu'))
})
act(() => {
fireEvent.click(getByText('Show dialog'))
})
expect(getByText('Primary')).toEqual(document.activeElement)
expect(getByText('Secondary')).not.toEqual(document.activeElement)
cleanup()
})
})
89 changes: 89 additions & 0 deletions src/__tests__/__snapshots__/ConfirmationDialog.tsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ConfirmationDialog renders consistently 1`] = `
.c0 {
font-family: -apple-system,BlinkMacSystemFont,"Segoe UI",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji";
line-height: 1.5;
color: #24292f;
}
.c1 {
position: relative;
display: inline-block;
padding: 6px 16px;
font-family: inherit;
font-weight: 600;
line-height: 20px;
white-space: nowrap;
vertical-align: middle;
cursor: pointer;
-webkit-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
user-select: none;
border-radius: 6px;
-webkit-appearance: none;
-moz-appearance: none;
appearance: none;
-webkit-text-decoration: none;
text-decoration: none;
text-align: center;
font-size: 14px;
color: #24292f;
background-color: #f6f8fa;
border: 1px solid rgba(27,31,36,0.15);
box-shadow: 0 1px 0 rgba(27,31,36,0.04),inset 0 1px 0 rgba(255,255,255,0.25);
}
.c1:hover {
-webkit-text-decoration: none;
text-decoration: none;
}
.c1:focus {
outline: none;
}
.c1:disabled {
cursor: default;
}
.c1:disabled svg {
opacity: 0.6;
}
.c1:hover {
background-color: #f3f4f6;
border-color: rgba(27,31,36,0.15);
}
.c1:focus {
border-color: rgba(27,31,36,0.15);
box-shadow: 0 0 0 3px rgba(9,105,218,0.3);
}
.c1:active {
background-color: hsla(220,14%,94%,1);
box-shadow: inset 0 0.15em 0.3em rgba(27,31,36,0.15);
}
.c1:disabled {
color: #57606a;
background-color: #f6f8fa;
border-color: rgba(27,31,36,0.15);
}
<div
className="c0"
color="text.primary"
data-portal-root={true}
fontFamily="normal"
>
<button
className="c1"
onClick={[Function]}
>
Show dialog
</button>
</div>
`;

0 comments on commit f1cebb7

Please sign in to comment.