Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

Commit

Permalink
SimpleModal: fixes voiceover context switch for nested modals glitch
Browse files Browse the repository at this point in the history
  • Loading branch information
tinkertrain committed Jun 20, 2022
1 parent 4751c2e commit 018c260
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 2 deletions.
4 changes: 4 additions & 0 deletions src/components/SimpleModal/SimpleModal.jsx
Expand Up @@ -10,6 +10,7 @@ import {
SimpleModalUI,
} from './SimpleModal.css'
import { shouldUnsetDimensions } from './SimpleModal.utils'
import isNil from 'lodash.isnil'

function noop() {}

Expand Down Expand Up @@ -112,10 +113,12 @@ function SimpleModal({
if (withCloseButton) {
if (customCloseButton && React.isValidElement(customCloseButton)) {
const clickHandler = customCloseButton.props.onClick || onClose

return React.cloneElement(customCloseButton, {
onClick: clickHandler,
})
}

return (
<CloseModalButtonUI
aria-label="close modal button"
Expand Down Expand Up @@ -156,6 +159,7 @@ function SimpleModal({
>
<SimpleModalUI
aria-modal="true"
aria-hidden={!isNil(rest.ariaHidden) ? rest.ariaHidden : !show}
aria-labelledby={ariaLabelledBy}
className="SimpleModal"
dataCy={dataCy}
Expand Down
6 changes: 4 additions & 2 deletions src/components/SimpleModal/SimpleModal.storiesHelpers.js
Expand Up @@ -72,6 +72,7 @@ export const SimpleModalWithTrigger = () => {
show={state.isOpen1}
className="first-modal"
focusModalOnShow
ariaHidden={state.isOpen2}
trapFocus
onClose={e => {
dispatch('CLOSE_1')
Expand Down Expand Up @@ -100,8 +101,8 @@ export const SimpleModalWithTrigger = () => {
id="second-modal"
className="second-modal"
show={state.isOpen2}
height="150px"
width="150px"
height="200px"
width="200px"
focusModalOnShow
trapFocus
onClose={e => {
Expand All @@ -111,6 +112,7 @@ export const SimpleModalWithTrigger = () => {
closeOnClickOutside="modal"
>
<div>My second modal</div>
<Button kind="primary">Another button</Button>
</SimpleModal>
</Portal>
</>
Expand Down
3 changes: 3 additions & 0 deletions src/components/SimpleModal/SimpleModal.test.js
Expand Up @@ -19,6 +19,9 @@ describe('Show / no show', () => {
expect(
queryByTestId('simple-modal-overlay').querySelector('.SimpleModal')
).toBeInTheDocument()
expect(
queryByTestId('simple-modal-overlay').querySelector('.SimpleModal')
).toHaveAttribute('aria-hidden', 'false')
expect(document.activeElement).toBe(
queryByTestId('simple-modal-overlay').querySelector('.SimpleModal')
)
Expand Down

0 comments on commit 018c260

Please sign in to comment.