Skip to content

Commit ee47660

Browse files
ShaneKIonitron
andauthored
fix(modal): add conditional tabIndex for handle cycling (#30510)
Issue number: resolves internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Currently, you cannot tab into a sheet modal from outside of it (the background), even with `handleBehavior` set to `cycle`. This destroys the accessibility of moving from the background behind a sheet modal to the contents of a sheet modal/the drag bar to be able to cycle the size. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> Now you can get into a sheet modal from outside of it and interact with its contents/drag handle when `handleBehavior` is set to `cycle`. This opens up the accessibility of the sheet modal and allows for interacting with background elements with sheet modals open using accessibility tools like VoiceOver and TalkBack. ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> [Relevant test screen](https://ionic-framework-git-fw-6523-ionic1.vercel.app/src/components/modal/test/sheet) Dev build: `8.6.3-dev.11750971489.140836b0` --------- Co-authored-by: ionitron <hi@ionicframework.com>
1 parent 72a5cdf commit ee47660

9 files changed

+93
-2
lines changed

core/src/components/modal/modal.tsx

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
7474
private currentBreakpoint?: number;
7575
private wrapperEl?: HTMLElement;
7676
private backdropEl?: HTMLIonBackdropElement;
77+
private dragHandleEl?: HTMLButtonElement;
7778
private sortedBreakpoints?: number[];
7879
private keyboardOpenCallback?: () => void;
7980
private moveSheetToBreakpoint?: (options: MoveSheetToBreakpointOptions) => Promise<void>;
@@ -950,6 +951,18 @@ export class Modal implements ComponentInterface, OverlayInterface {
950951
}
951952
};
952953

954+
/**
955+
* When the modal receives focus directly, pass focus to the handle
956+
* if it exists and is focusable, otherwise let the focus trap handle it.
957+
*/
958+
private onModalFocus = (ev: FocusEvent) => {
959+
const { dragHandleEl, el } = this;
960+
// Only handle focus if the modal itself was focused (not a child element)
961+
if (ev.target === el && dragHandleEl && dragHandleEl.tabIndex !== -1) {
962+
dragHandleEl.focus();
963+
}
964+
};
965+
953966
render() {
954967
const {
955968
handle,
@@ -965,11 +978,13 @@ export class Modal implements ComponentInterface, OverlayInterface {
965978
const mode = getIonMode(this);
966979
const isCardModal = presentingElement !== undefined && mode === 'ios';
967980
const isHandleCycle = handleBehavior === 'cycle';
981+
const isSheetModalWithHandle = isSheetModal && showHandle;
968982

969983
return (
970984
<Host
971985
no-router
972-
tabindex="-1"
986+
// Allow the modal to be navigable when the handle is focusable
987+
tabIndex={isHandleCycle && isSheetModalWithHandle ? 0 : -1}
973988
{...(htmlAttributes as any)}
974989
style={{
975990
zIndex: `${20000 + this.overlayIndex}`,
@@ -989,6 +1004,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
9891004
onIonModalWillPresent={this.onLifecycle}
9901005
onIonModalWillDismiss={this.onLifecycle}
9911006
onIonModalDidDismiss={this.onLifecycle}
1007+
onFocus={this.onModalFocus}
9921008
>
9931009
<ion-backdrop
9941010
ref={(el) => (this.backdropEl = el)}
@@ -1021,6 +1037,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
10211037
aria-label="Activate to adjust the size of the dialog overlaying the screen"
10221038
onClick={isHandleCycle ? this.onHandleClick : undefined}
10231039
part="handle"
1040+
ref={(el) => (this.dragHandleEl = el)}
10241041
></button>
10251042
)}
10261043
<slot></slot>

core/src/components/modal/test/sheet/index.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@
106106
>
107107
Present Sheet Modal (Scroll at any breakpoint)
108108
</button>
109+
<button
110+
id="cycle-scroll-no-backdrop"
111+
onclick="presentModal({ handleBehavior: 'cycle', backdropBreakpoint: 1, backdropDismiss: false, initialBreakpoint: 0.5, breakpoints: [0, 0.25, 0.5, 0.75, 1], expandToScroll: false })"
112+
>
113+
Present Sheet Modal (Cycle Handle, Scroll at any breakpoint)
114+
</button>
109115
<button
110116
id="custom-backdrop-modal"
111117
onclick="presentModal({ backdropBreakpoint: 0.5, initialBreakpoint: 0.5 })"

core/src/components/modal/test/sheet/modal.e2e.ts

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test';
2-
import { configs, test, dragElementBy } from '@utils/test/playwright';
2+
import { configs, dragElementBy, test } from '@utils/test/playwright';
33

44
configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
55
test.describe(title('sheet modal: rendering'), () => {
@@ -30,6 +30,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
3030
test.beforeEach(async ({ page }) => {
3131
await page.goto('/src/components/modal/test/sheet', config);
3232
});
33+
3334
test('should dismiss the sheet modal when clicking the active backdrop', async ({ page }) => {
3435
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
3536
const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
@@ -42,6 +43,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
4243

4344
await ionModalDidDismiss.next();
4445
});
46+
4547
test('should present another sheet modal when clicking an inactive backdrop', async ({ page }) => {
4648
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
4749
const modal = page.locator('.custom-height');
@@ -54,6 +56,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
5456

5557
await expect(modal).toBeVisible();
5658
});
59+
5760
test('input outside sheet modal should be focusable when backdrop is inactive', async ({ page }) => {
5861
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
5962

@@ -66,6 +69,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
6669
await expect(input).toBeFocused();
6770
});
6871
});
72+
6973
test.describe(title('sheet modal: setting the breakpoint'), () => {
7074
test.describe('sheet modal: invalid values', () => {
7175
let warnings: string[] = [];
@@ -88,18 +92,21 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
8892
const modal = page.locator('ion-modal');
8993
await modal.evaluate((el: HTMLIonModalElement) => el.setCurrentBreakpoint(0.01));
9094
});
95+
9196
test('it should not change the breakpoint when setting to an invalid value', async ({ page }) => {
9297
const modal = page.locator('ion-modal');
9398
const breakpoint = await modal.evaluate((el: HTMLIonModalElement) => el.getCurrentBreakpoint());
9499
expect(breakpoint).toBe(0.25);
95100
});
101+
96102
test('it should warn when setting an invalid breakpoint', async () => {
97103
expect(warnings.length).toBe(1);
98104
expect(warnings[0]).toBe(
99105
'[Ionic Warning]: [ion-modal] - Attempted to set invalid breakpoint value 0.01. Please double check that the breakpoint value is part of your defined breakpoints.'
100106
);
101107
});
102108
});
109+
103110
test.describe('sheet modal: valid values', () => {
104111
test.beforeEach(async ({ page }) => {
105112
await page.goto('/src/components/modal/test/sheet', config);
@@ -108,6 +115,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
108115
await page.click('#sheet-modal');
109116
await ionModalDidPresent.next();
110117
});
118+
111119
test('should update the current breakpoint', async ({ page }) => {
112120
const ionBreakpointDidChange = await page.spyOnEvent('ionBreakpointDidChange');
113121
const modal = page.locator('.modal-sheet');
@@ -118,6 +126,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
118126
const breakpoint = await modal.evaluate((el: HTMLIonModalElement) => el.getCurrentBreakpoint());
119127
expect(breakpoint).toBe(0.5);
120128
});
129+
121130
test('should emit ionBreakpointDidChange', async ({ page }) => {
122131
const ionBreakpointDidChange = await page.spyOnEvent('ionBreakpointDidChange');
123132
const modal = page.locator('.modal-sheet');
@@ -126,6 +135,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
126135
await ionBreakpointDidChange.next();
127136
expect(ionBreakpointDidChange.events.length).toBe(1);
128137
});
138+
129139
test('should emit ionBreakpointDidChange when breakpoint is set to 0', async ({ page }) => {
130140
const ionBreakpointDidChange = await page.spyOnEvent('ionBreakpointDidChange');
131141
const modal = page.locator('.modal-sheet');
@@ -134,6 +144,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
134144
await ionBreakpointDidChange.next();
135145
expect(ionBreakpointDidChange.events.length).toBe(1);
136146
});
147+
137148
test('should emit ionBreakpointDidChange when the sheet is swiped to breakpoint 0', async ({ page }) => {
138149
const ionBreakpointDidChange = await page.spyOnEvent('ionBreakpointDidChange');
139150
const header = page.locator('.modal-sheet ion-header');
@@ -211,6 +222,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
211222
expect(updatedBreakpoint).toBe(0.5);
212223
});
213224
});
225+
214226
test.describe(title('sheet modal: clicking the handle'), () => {
215227
test.beforeEach(async ({ page }) => {
216228
await page.goto('/src/components/modal/test/sheet', config);
@@ -285,4 +297,60 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
285297
await expect(await modal.evaluate((el: HTMLIonModalElement) => el.getCurrentBreakpoint())).toBe(0.75);
286298
});
287299
});
300+
301+
test.describe(title('sheet modal: accessibility'), () => {
302+
test('it should allow focus on the drag handle from outside of the modal', async ({ page }) => {
303+
// In this scenario, the modal is opened and has no backdrop, allowing
304+
// the background content to be focused. We need to ensure that we can
305+
// navigate to the drag handle using the keyboard and voiceover/talkback.
306+
await page.goto('/src/components/modal/test/sheet', config);
307+
308+
await page.setContent(
309+
`
310+
<ion-content>
311+
<button id="open-modal">Open</button>
312+
<ion-modal trigger="open-modal" initial-breakpoint="0.25">
313+
<ion-content>
314+
<ion-button id="dismiss" onclick="modal.dismiss();">Dismiss</ion-button>
315+
<ion-button id="set-breakpoint">Set breakpoint</ion-button>
316+
</ion-content>
317+
</ion-modal>
318+
</ion-content>
319+
<script>
320+
const modal = document.querySelector('ion-modal');
321+
const setBreakpointButton = document.querySelector('#set-breakpoint');
322+
323+
modal.breakpoints = [0.25, 0.5, 1];
324+
modal.handleBehavior = 'cycle';
325+
modal.backdropBreakpoint = 1;
326+
modal.backdropDismiss = false;
327+
modal.expandToScroll = false;
328+
329+
setBreakpointButton.addEventListener('click', () => {
330+
modal.setCurrentBreakpoint(0.5);
331+
});
332+
</script>
333+
`,
334+
config
335+
);
336+
337+
const openButton = page.locator('#open-modal');
338+
339+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
340+
341+
await openButton.click();
342+
await ionModalDidPresent.next();
343+
344+
const dragHandle = page.locator('ion-modal .modal-handle');
345+
await expect(dragHandle).toBeVisible();
346+
347+
openButton.focus();
348+
await expect(openButton).toBeFocused();
349+
350+
// Tab should now bring us to the drag handle
351+
await page.keyboard.press('Tab');
352+
353+
await expect(dragHandle).toBeFocused();
354+
});
355+
});
288356
});

0 commit comments

Comments
 (0)