From e4ec572043e22bd2626dbcfd204fc22a7335282c Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Mon, 28 Feb 2022 13:27:12 -0500 Subject: [PATCH] fix(modal): sheet modal now allows input focusing when backdrop disabled (#24840) resolves #24581 --- core/src/components/modal/gestures/sheet.ts | 44 ++++++++++++++++--- core/src/components/modal/readme.md | 27 ++++++++++++ core/src/components/modal/test/sheet/e2e.ts | 30 +++++++++++++ .../components/modal/test/sheet/index.html | 5 +++ core/src/components/modal/test/test.utils.ts | 2 +- core/src/utils/overlays.ts | 11 +++++ core/src/utils/test/utils.ts | 19 +++++--- core/stencil.config.ts | 3 ++ core/tsconfig.json | 6 ++- 9 files changed, 134 insertions(+), 13 deletions(-) diff --git a/core/src/components/modal/gestures/sheet.ts b/core/src/components/modal/gestures/sheet.ts index 1b4230311e..59060ba45e 100644 --- a/core/src/components/modal/gestures/sheet.ts +++ b/core/src/components/modal/gestures/sheet.ts @@ -42,6 +42,32 @@ export const createSheetGesture = ( const backdropAnimation = animation.childAnimations.find(ani => ani.id === 'backdropAnimation'); const maxBreakpoint = breakpoints[breakpoints.length - 1]; + const enableBackdrop = () => { + baseEl.style.setProperty('pointer-events', 'auto'); + backdropEl.style.setProperty('pointer-events', 'auto'); + + /** + * When the backdrop is enabled, elements such + * as inputs should not be focusable outside + * the sheet. + */ + baseEl.classList.remove('ion-disable-focus-trap'); + } + + const disableBackdrop = () => { + baseEl.style.setProperty('pointer-events', 'none'); + backdropEl.style.setProperty('pointer-events', 'none'); + + /** + * When the backdrop is enabled, elements such + * as inputs should not be focusable outside + * the sheet. + * Adding this class disables focus trapping + * for the sheet temporarily. + */ + baseEl.classList.add('ion-disable-focus-trap'); + } + /** * After the entering animation completes, * we need to set the animation to go from @@ -62,9 +88,12 @@ export const createSheetGesture = ( * ion-backdrop and .modal-wrapper always have pointer-events: auto * applied, so the modal content can still be interacted with. */ - const backdropEnabled = currentBreakpoint > backdropBreakpoint - baseEl.style.setProperty('pointer-events', backdropEnabled ? 'auto' : 'none'); - backdropEl.style.setProperty('pointer-events', backdropEnabled ? 'auto' : 'none'); + const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint; + if (shouldEnableBackdrop) { + enableBackdrop(); + } else { + disableBackdrop(); + } } if (contentEl && currentBreakpoint !== maxBreakpoint) { @@ -190,9 +219,12 @@ export const createSheetGesture = ( * Backdrop should become enabled * after the backdropBreakpoint value */ - const backdropEnabled = currentBreakpoint > backdropBreakpoint; - baseEl.style.setProperty('pointer-events', backdropEnabled ? 'auto' : 'none'); - backdropEl.style.setProperty('pointer-events', backdropEnabled ? 'auto' : 'none'); + const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint; + if (shouldEnableBackdrop) { + enableBackdrop(); + } else { + disableBackdrop(); + } gesture.enable(true); }); diff --git a/core/src/components/modal/readme.md b/core/src/components/modal/readme.md index 5e39bb105c..837b96bffd 100644 --- a/core/src/components/modal/readme.md +++ b/core/src/components/modal/readme.md @@ -140,6 +140,33 @@ interface ModalOptions { interface ModalAttributes extends JSXBase.HTMLAttributes {} ``` +## Accessibility + +### Keyboard Navigation + +| Key | Function | +| ----- | ------------------- | +| `Esc` | Dismisses the modal | + +### Screen Readers + +Modals have the `aria-modal` attribute applied. This attribute can cause assistive technologies to limit navigation to the modal element's contents. As a result, using gestures that move to the next or previous items may not focus elements outside of the modal. This applies even when the backdrop is disabled in sheet modals using the `backdropBreakpoint` property. + +Assistive technologies will not limit navigation to the modal element's contents if developers manually move focus. However, manually moving focus outside of a modal is not supported in Ionic for modals that have focus trapping enabled. + +See https://w3c.github.io/aria/#aria-modal for more information. + +### Focus Trapping + +When a modal is presented, focus will be trapped inside of the presented modal. Users can focus other interactive elements inside the modal but will never be able to focus interactive elements outside the modal while the modal is presented. For applications that present multiple stacked modals, focus will be trapped on the modal that was presented last. + +Sheet modals that have had their backdrop disabled by the `backdropBreakpoint` property are not subject to focus trapping. + +### Sheet Modals + +Sheet modals allow users to interact with content behind the modal when the `backdropBreakpoint` property is used. The backdrop will be disabled up to and including the specified `backdropBreakpoint` and will be enabled after it. + +When the backdrop is disabled, users will be able to interact with elements outside the sheet modal using a pointer or keyboard. Assistive technologies may not focus outside the sheet modal by default due to the usage of `aria-modal`. We recommend avoiding features such as autofocus here as it can cause assistive technologies to jump between two interactive contexts without warning the user. diff --git a/core/src/components/modal/test/sheet/e2e.ts b/core/src/components/modal/test/sheet/e2e.ts index a63dff96d0..0a0b7b045f 100644 --- a/core/src/components/modal/test/sheet/e2e.ts +++ b/core/src/components/modal/test/sheet/e2e.ts @@ -1,5 +1,6 @@ import { newE2EPage } from '@stencil/core/testing'; import { testModal } from '../test.utils'; +import { getActiveElement, getActiveElementParent } from '@utils/test'; const DIRECTORY = 'sheet'; @@ -87,3 +88,32 @@ test('should click to present another modal when backdrop is inactive', async () const customModal = await page.find('.custom-height'); expect(customModal).not.toBe(null); }); + +test('input should be focusable when backdrop is inactive', async () => { + const page = await newE2EPage({ url: '/src/components/modal/test/sheet?ionic:_testing=true' }); + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + + await page.click('#backdrop-inactive'); + + await ionModalDidPresent.next(); + + await page.click('#root-input'); + + const parentEl = await getActiveElementParent(page); + expect(parentEl.id).toEqual('root-input'); +}); + +test('input should not be focusable when backdrop is active', async () => { + const page = await newE2EPage({ url: '/src/components/modal/test/sheet?ionic:_testing=true' }); + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + + await page.click('#backdrop-active'); + + await ionModalDidPresent.next(); + + await page.click('#root-input'); + await page.waitForChanges(); + + const parentEl = await getActiveElement(page); + expect(parentEl.tagName).toEqual('ION-BUTTON'); +}); diff --git a/core/src/components/modal/test/sheet/index.html b/core/src/components/modal/test/sheet/index.html index 5eb650f219..efd10630e9 100644 --- a/core/src/components/modal/test/sheet/index.html +++ b/core/src/components/modal/test/sheet/index.html @@ -85,6 +85,11 @@ + + Input outside modal + + + Present Sheet Modal Present Sheet Modal (Custom Breakpoints) Present Sheet Modal (Max breakpoint is not 1) diff --git a/core/src/components/modal/test/test.utils.ts b/core/src/components/modal/test/test.utils.ts index ae62364b0b..c3b140653e 100644 --- a/core/src/components/modal/test/test.utils.ts +++ b/core/src/components/modal/test/test.utils.ts @@ -1,6 +1,6 @@ import { newE2EPage } from '@stencil/core/testing'; -import { generateE2EUrl } from '../../../utils/test/utils'; +import { generateE2EUrl } from '@utils/test'; export const testModal = async ( type: string, diff --git a/core/src/utils/overlays.ts b/core/src/utils/overlays.ts index 2bf66d6c6f..f52fb9a37d 100644 --- a/core/src/utils/overlays.ts +++ b/core/src/utils/overlays.ts @@ -128,6 +128,17 @@ const trapKeyboardFocus = (ev: Event, doc: Document) => { */ if (!lastOverlay || !target) { return; } + /** + * If the ion-disable-focus-trap class + * is present on an overlay, then this component + * instance has opted out of focus trapping. + * An example of this is when the sheet modal + * has a backdrop that is disabled. The content + * behind the sheet should be focusable until + * the backdrop is enabled. + */ + if (lastOverlay.classList.contains('ion-disable-focus-trap')) { return; } + const trapScopedFocus = () => { /** * If we are focusing the overlay, clear diff --git a/core/src/utils/test/utils.ts b/core/src/utils/test/utils.ts index 1c8754b02f..99f2c7dfe4 100644 --- a/core/src/utils/test/utils.ts +++ b/core/src/utils/test/utils.ts @@ -7,19 +7,28 @@ import { ElementHandle } from 'puppeteer'; * Instead, we return an object with some common * properties that you may want to access in a test. */ -export const getActiveElementParent = async (page) => { - const activeElement = await page.evaluateHandle(() => document.activeElement); +const getSerialElement = async (page, element) => { return await page.evaluate(el => { - const { parentElement } = el; - const { className, tagName, id } = parentElement; + const { className, tagName, id } = el; return { className, tagName, id } - }, activeElement); + }, element); } +export const getActiveElementParent = async (page) => { + const activeElement = await page.evaluateHandle(() => document.activeElement.parentElement); + return getSerialElement(page, activeElement); +} + +export const getActiveElement = async (page) => { + const activeElement = await page.evaluateHandle(() => document.activeElement); + return getSerialElement(page, activeElement); +} + + export const generateE2EUrl = (component: string, type: string, rtl = false): string => { let url = `/src/components/${component}/test/${type}?ionic:_testing=true`; if (rtl) { diff --git a/core/stencil.config.ts b/core/stencil.config.ts index fff6e16d9f..5b585b5e4c 100644 --- a/core/stencil.config.ts +++ b/core/stencil.config.ts @@ -261,6 +261,9 @@ export const config: Config = { allowableMismatchedPixels: 200, pixelmatchThreshold: 0.05, waitBeforeScreenshot: 20, + moduleNameMapper: { + "@utils/test": ["/src/utils/test/utils"] + }, emulate: [ { userAgent: 'iPhone', diff --git a/core/tsconfig.json b/core/tsconfig.json index 6570b8e42e..bf14a75f4c 100644 --- a/core/tsconfig.json +++ b/core/tsconfig.json @@ -23,7 +23,11 @@ "outDir": ".tmp", "pretty": true, "removeComments": false, - "target": "es2017" + "target": "es2017", + "baseUrl": ".", + "paths": { + "@utils/test": ["src/utils/test/utils"] + } }, "include": [ "src",