From f9ff62ea59e9b368751492bec3260ecef0413cc7 Mon Sep 17 00:00:00 2001 From: Trevor Pierce <1Copenut@users.noreply.github.com> Date: Tue, 12 Mar 2024 10:55:05 -0500 Subject: [PATCH] [BUG][EuiModal] Fix VoiceOver + Safari escaping focus trap (#7564) Co-authored-by: Cee Chen --- changelogs/upcoming/7564.md | 5 + src-docs/src/views/modal/confirm_modal.tsx | 84 +++++---- .../src/views/modal/confirm_modal_loading.tsx | 64 +++---- src-docs/src/views/modal/modal.tsx | 62 +++---- src-docs/src/views/modal/modal_example.js | 16 +- src-docs/src/views/modal/modal_form.tsx | 167 ++++++++++-------- src-docs/src/views/modal/modal_width.tsx | 62 ++++--- .../__snapshots__/confirm_modal.test.tsx.snap | 4 + .../modal/__snapshots__/modal.test.tsx.snap | 2 + src/components/modal/confirm_modal.tsx | 8 +- src/components/modal/modal.tsx | 8 + 11 files changed, 262 insertions(+), 220 deletions(-) create mode 100644 changelogs/upcoming/7564.md diff --git a/changelogs/upcoming/7564.md b/changelogs/upcoming/7564.md new file mode 100644 index 00000000000..c65b672ceee --- /dev/null +++ b/changelogs/upcoming/7564.md @@ -0,0 +1,5 @@ +**Accessibility** + +- Updated `EuiModal` to set an `aria-modal` attribute and a default `dialog` role +- Updated `EuiConfirmModal` to set a default `alertdialog` role +- Fixed `EuiModal` and `EuiConfirmModal` to properly trap Safari+VoiceOver's virtual cursor diff --git a/src-docs/src/views/modal/confirm_modal.tsx b/src-docs/src/views/modal/confirm_modal.tsx index ea7b94008ee..07fd5ae9900 100644 --- a/src-docs/src/views/modal/confirm_modal.tsx +++ b/src-docs/src/views/modal/confirm_modal.tsx @@ -5,6 +5,7 @@ import { EuiConfirmModal, EuiFlexGroup, EuiFlexItem, + useGeneratedHtmlId, } from '../../../../src'; export default () => { @@ -17,48 +18,11 @@ export default () => { const closeDestroyModal = () => setIsDestroyModalVisible(false); const showDestroyModal = () => setIsDestroyModalVisible(true); - let modal; - - if (isModalVisible) { - modal = ( - -

- Your subscription and benefits increase immediately. If you change to - a lower subscription later, it will not take affect until the next - billing cycle. -

-
- ); - } - - let destroyModal; - - if (isDestroyModalVisible) { - destroyModal = ( - -

You will lose all unsaved changes made to this dashboard.

-
- ); - } + const modalTitleId = useGeneratedHtmlId(); + const destroyModalTitleId = useGeneratedHtmlId(); return ( -
+ <> Show confirm modal @@ -69,8 +33,42 @@ export default () => { - {modal} - {destroyModal} -
+ + {isModalVisible && ( + +

+ Your subscription and benefits increase immediately. If you change + to a lower subscription later, it will not take affect until the + next billing cycle. +

+
+ )} + + {isDestroyModalVisible && ( + +

You will lose all unsaved changes made to this dashboard.

+
+ )} + ); }; diff --git a/src-docs/src/views/modal/confirm_modal_loading.tsx b/src-docs/src/views/modal/confirm_modal_loading.tsx index 86214b49fb2..d49b6797d5d 100644 --- a/src-docs/src/views/modal/confirm_modal_loading.tsx +++ b/src-docs/src/views/modal/confirm_modal_loading.tsx @@ -5,6 +5,7 @@ import { EuiConfirmModal, EuiFormRow, EuiFieldText, + useGeneratedHtmlId, } from '../../../../src'; export default () => { @@ -36,40 +37,39 @@ export default () => { setValue(e.target.value); }; - let modal; - - if (isModalVisible) { - modal = ( - { - closeModal(); - window.alert('Shame on you!'); - }} - confirmButtonText="Delete" - cancelButtonText="Cancel" - buttonColor="danger" - initialFocus="[name=delete]" - confirmButtonDisabled={value.toLowerCase() !== 'delete'} - isLoading={isLoading} - > - - - - - ); - } + const modalTitleId = useGeneratedHtmlId(); return ( -
+ <> Show loading confirm modal - {modal} -
+ + {isModalVisible && ( + { + closeModal(); + window.alert('Shame on you!'); + }} + confirmButtonText="Delete" + cancelButtonText="Cancel" + buttonColor="danger" + initialFocus="[name=delete]" + confirmButtonDisabled={value.toLowerCase() !== 'delete'} + isLoading={isLoading} + > + + + + + )} + ); }; diff --git a/src-docs/src/views/modal/modal.tsx b/src-docs/src/views/modal/modal.tsx index 78ec5578e2b..b59de46465d 100644 --- a/src-docs/src/views/modal/modal.tsx +++ b/src-docs/src/views/modal/modal.tsx @@ -8,8 +8,9 @@ import { EuiModalHeader, EuiModalHeaderTitle, EuiCodeBlock, -} from '../../../../src/components'; -import { EuiSpacer } from '../../../../src/components/spacer'; + EuiSpacer, + useGeneratedHtmlId, +} from '../../../../src'; export default () => { const [isModalVisible, setIsModalVisible] = useState(false); @@ -17,22 +18,27 @@ export default () => { const closeModal = () => setIsModalVisible(false); const showModal = () => setIsModalVisible(true); - let modal; + const modalTitleId = useGeneratedHtmlId(); - if (isModalVisible) { - modal = ( - - - Modal title - + return ( + <> + Show modal + + {isModalVisible && ( + + + + Modal title + + - - This modal has the following setup: - - - {` + + This modal has the following setup: + + + {` - + @@ -45,22 +51,16 @@ export default () => { `} - - + + - - - Close - - - - ); - } - - return ( -
- Show modal - {modal} -
+ + + Close + + +
+ )} + ); }; diff --git a/src-docs/src/views/modal/modal_example.js b/src-docs/src/views/modal/modal_example.js index ef6a3d5d839..cd741dfe10d 100644 --- a/src-docs/src/views/modal/modal_example.js +++ b/src-docs/src/views/modal/modal_example.js @@ -30,9 +30,9 @@ const confirmModalLoadingSource = require('!!raw-loader!./confirm_modal_loading' import ModalWidth from './modal_width'; const modalWidthSource = require('!!raw-loader!./modal_width'); -const modalSnippet = ` +const modalSnippet = ` - + @@ -44,9 +44,9 @@ const modalSnippet = ` `; -const modalWidthSnippet = ` +const modalWidthSnippet = ` - + @@ -58,9 +58,9 @@ const modalWidthSnippet = ` `; -const modalFormSnippet = ` +const modalFormSnippet = ` - + @@ -75,7 +75,9 @@ const modalFormSnippet = ` const confirmModalSnippet = [ ` { - const [isModalVisible, setIsModalVisible] = useState(false); - const [isSwitchChecked, setIsSwitchChecked] = useState(true); - const [superSelectvalue, setSuperSelectValue] = useState('option_one'); - - const modalFormId = useGeneratedHtmlId({ prefix: 'modalForm' }); +const superSelectOptions = [ + { + value: 'option_one', + inputDisplay: 'Option one', + dropdownDisplay: ( + <> + Option one + +

Has a short description giving more detail to the option.

+
+ + ), + }, + { + value: 'option_two', + inputDisplay: 'Option two', + dropdownDisplay: ( + <> + Option two + +

Has a short description giving more detail to the option.

+
+ + ), + }, + { + value: 'option_three', + inputDisplay: 'Option three', + dropdownDisplay: ( + <> + Option three + +

Has a short description giving more detail to the option.

+
+ + ), + }, +]; + +const ExampleForm = ({ id }: Partial) => { const modalFormSwitchId = useGeneratedHtmlId({ prefix: 'modalFormSwitch' }); + const [isSwitchChecked, setIsSwitchChecked] = useState(true); const onSwitchChange = () => setIsSwitchChecked((isSwitchChecked) => !isSwitchChecked); - const closeModal = () => setIsModalVisible(false); - - const showModal = () => setIsModalVisible(true); + const [superSelectvalue, setSuperSelectValue] = useState('option_one'); + const onSuperSelectChange = (value: string) => { + setSuperSelectValue(value); + }; - const superSelectOptions = [ - { - value: 'option_one', - inputDisplay: 'Option one', - dropdownDisplay: ( - <> - Option one - -

Has a short description giving more detail to the option.

-
- - ), - }, - { - value: 'option_two', - inputDisplay: 'Option two', - dropdownDisplay: ( - <> - Option two - -

Has a short description giving more detail to the option.

-
- - ), - }, - { - value: 'option_three', - inputDisplay: 'Option three', - dropdownDisplay: ( - <> - Option three - -

Has a short description giving more detail to the option.

-
- - ), - }, - ]; - - const formSample = ( - + return ( + { ); +}; - const onSuperSelectChange = (value: string) => { - setSuperSelectValue(value); - }; - - let modal; - - if (isModalVisible) { - modal = ( - - - Modal title - - - {formSample} +export default () => { + const [isModalVisible, setIsModalVisible] = useState(false); + const closeModal = () => setIsModalVisible(false); + const showModal = () => setIsModalVisible(true); - - Cancel + const modalFormId = useGeneratedHtmlId({ prefix: 'modalForm' }); + const modalTitleId = useGeneratedHtmlId(); - - Save - - - - ); - } return ( -
+ <> Show form modal - {modal} -
+ + {isModalVisible && ( + + + + Modal title + + + + + + + + + Cancel + + + Save + + + + )} + ); }; diff --git a/src-docs/src/views/modal/modal_width.tsx b/src-docs/src/views/modal/modal_width.tsx index 9896b72352d..20e09ab9e57 100644 --- a/src-docs/src/views/modal/modal_width.tsx +++ b/src-docs/src/views/modal/modal_width.tsx @@ -9,6 +9,7 @@ import { EuiModalHeaderTitle, EuiCodeBlock, EuiSpacer, + useGeneratedHtmlId, } from '../../../../src'; export default () => { @@ -17,22 +18,31 @@ export default () => { const closeModal = () => setIsModalVisible(false); const showModal = () => setIsModalVisible(true); - let modal; + const modalTitle = useGeneratedHtmlId(); - if (isModalVisible) { - modal = ( - - - Modal title - + return ( + <> + Show modal with custom width + + {isModalVisible && ( + + + + Modal title + + - - This modal has the following setup: - - - {` + + This modal has the following setup: + + + {` - + @@ -45,22 +55,16 @@ export default () => { `} - - + + - - - Close - - - - ); - } - - return ( -
- Show modal with custom width - {modal} -
+ + + Close + + +
+ )} + ); }; diff --git a/src/components/modal/__snapshots__/confirm_modal.test.tsx.snap b/src/components/modal/__snapshots__/confirm_modal.test.tsx.snap index ac5c6906ba4..279e2aa4019 100644 --- a/src/components/modal/__snapshots__/confirm_modal.test.tsx.snap +++ b/src/components/modal/__snapshots__/confirm_modal.test.tsx.snap @@ -12,8 +12,10 @@ Array [ >