Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ce1dc72
fix(overlays): focus the dialog wrapper, not the role-less host, on p…
gnbm Jul 1, 2026
deeca02
fix(overlays): only redirect present() focus to a wrapper that is act…
gnbm Jul 1, 2026
58782ef
fix(popover): add default role="dialog" so aria-modal is not a no-op
gnbm Jul 1, 2026
5b25d95
fix(modal): scope dialog-wrapper focus to default modals; keep host f…
gnbm Jul 1, 2026
7acf4eb
revert(popover): drop default role="dialog"; keep PR scoped to modal fix
gnbm Jul 1, 2026
b98cbb3
test(modal): use idiomatic toBeFocused for the a11y focus assertion
gnbm Jul 1, 2026
cbca643
Update comment in modal
gnbm Jul 2, 2026
fe89761
Update core/src/components/modal/test/a11y/modal.e2e.ts
gnbm Jul 7, 2026
ca26b3b
Update core/src/components/modal/modal.tsx
gnbm Jul 7, 2026
b1ca7aa
fix(modal): focus the dialog wrapper on present for sheet and card mo…
gnbm Jul 7, 2026
f6363c2
fix(modal): suppress native focus ring on the focused wrapper
gnbm Jul 8, 2026
4d16cd7
refactor(modal): simplify a11y fix comments and extend gesture guard …
gnbm Jul 8, 2026
f28d893
docs(modal): trim verbose comments on the a11y focus fix
gnbm Jul 8, 2026
481e513
Update core/src/components/modal/modal.scss
gnbm Jul 13, 2026
03397f2
Update core/src/components/modal/test/a11y/modal.e2e.ts
gnbm Jul 13, 2026
870c497
Update core/src/utils/overlays.ts
gnbm Jul 13, 2026
0a1b6f3
Update core/src/components/modal/test/a11y/modal.e2e.ts
gnbm Jul 13, 2026
2d3cb96
Update core/src/components/modal/test/a11y/modal.e2e.ts
gnbm Jul 13, 2026
16d0939
Update core/src/components/modal/test/a11y/modal.e2e.ts
gnbm Jul 13, 2026
b0b5256
test(modal): address review feedback on a11y focus tests
gnbm Jul 13, 2026
5ddc39c
fix(overlays): only retarget focus to a focusable dialog wrapper
gnbm Jul 13, 2026
7449639
fix(overlays): guard preventScroll focus against engines that mishand…
gnbm Jul 13, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions core/src/components/modal/gestures/sheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,16 @@ export const createSheetGesture = (
return true;
};

/**
* In Firefox, pressing over a text caret in the focusable wrapper can
* start a native drag that swallows the gesture's pointer events.
* Cancel native drag while the gesture is active.
*/
const preventNativeDragStart = (ev: DragEvent) => ev.preventDefault();
Comment on lines +301 to +306

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I test this?

@gnbm gnbm Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual repro (Firefox): present a sheet modal and drag the header immediately. Because the wrapper is now focusable and contains text, Firefox starts a native text drag that swallows the gesture's pointer events, so ionDragStart never fires and the sheet cannot be dragged. Cancelling dragstart while the gesture is active fixes it.

Automated: the existing should emit ionDragStart, ionDragMove, and ionDragEnd events e2e is the fail/pass gate (I confirmed it fails 3/3 on Firefox with this removed), and should cancel native drag and drop only while the gesture is active asserts dragstart is defaultPrevented during the gesture and not after.


const onStart = (detail: GestureDetail) => {
baseEl.addEventListener('dragstart', preventNativeDragStart);

/**
* If canDismiss is anything other than `true`
* then users should be able to swipe down
Expand Down Expand Up @@ -443,6 +452,8 @@ export const createSheetGesture = (
};

const onEnd = (detail: GestureDetail) => {
baseEl.removeEventListener('dragstart', preventNativeDragStart);

const snapBreakpoint = calculateSnapBreakpoint(detail.deltaY);

const eventDetail: ModalDragEventDetail = {
Expand Down
11 changes: 11 additions & 0 deletions core/src/components/modal/gestures/swipe-to-close.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,18 @@ export const createSwipeToCloseGesture = (
return false;
};

/**
* In Firefox, pressing over a text caret in the focusable wrapper can
* start a native drag that swallows the gesture's pointer events.
* Cancel native drag while the gesture is active.
*/
const preventNativeDragStart = (ev: DragEvent) => ev.preventDefault();
Comment on lines +116 to +121

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I test this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual repro (Firefox): present a sheet modal and drag the header immediately. Because the wrapper is now focusable and contains text, Firefox starts a native text drag that swallows the gesture's pointer events, so ionDragStart never fires and the sheet cannot be dragged. Cancelling dragstart while the gesture is active fixes it.

Automated: the existing should emit ionDragStart, ionDragMove, and ionDragEnd events e2e is the fail/pass gate (I confirmed it fails 3/3 on Firefox with this removed), and should cancel native drag and drop only while the gesture is active asserts dragstart is defaultPrevented during the gesture and not after.

🤖 Addressed by Claude Code


const onStart = (detail: GestureDetail) => {
const { deltaY } = detail;

el.addEventListener('dragstart', preventNativeDragStart);

/**
* Get the initial scrollY value so
* that we can correctly reset the scrollY
Expand Down Expand Up @@ -237,6 +246,8 @@ export const createSwipeToCloseGesture = (
};

const onEnd = (detail: GestureDetail) => {
el.removeEventListener('dragstart', preventNativeDragStart);

const velocity = detail.velocityY;
const step = detail.deltaY / height;

Expand Down
8 changes: 8 additions & 0 deletions core/src/components/modal/modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ ion-backdrop {
z-index: 10;
}

/**
* The wrapper receives programmatic focus for screen readers but should not
* show a visible focus ring, which is meant only for keyboard navigation.
*/
.modal-wrapper {
outline: none;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is no longer required with my suggested change to query for role="dialog".

@gnbm gnbm Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still required. Because tabIndex={-1} stays on the wrapper (needed for focusability), the wrapper is the element that receives programmatic focus on present, so it can render a focus ring — most visibly when the modal is opened via the keyboard. This rule suppresses that ring to match the host. The should not render a focus ring on the wrapper when presented via keyboard e2e covers it.

}
Comment thread
gnbm marked this conversation as resolved.

.modal-shadow {
position: absolute;

Expand Down
6 changes: 6 additions & 0 deletions core/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1644,10 +1644,16 @@ export class Modal implements ComponentInterface, OverlayInterface {
same element. They must also be set inside the
shadow DOM otherwise ion-button will not be highlighted
when using VoiceOver: https://bugs.webkit.org/show_bug.cgi?id=247134

tabIndex={-1} is required so present() can move focus to this
element (which carries the dialog role) instead of the role-less
host. role="dialog" alone does not make an element focusable, so
without the tabindex focus() would be a no-op.
*/
role="dialog"
{...inheritedAttributes}
aria-modal="true"
tabIndex={-1}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment on core/src/utils/overlays.ts

Suggested change
tabIndex={-1}

@gnbm gnbm Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping tabIndex={-1} here. Without it, focus() on the role="dialog" wrapper is a no-op (an ARIA role does not make an element focusable), so the fix would silently stop working — focus would stay on the role-less host and screen readers would have nothing to land on. Verified in light + shadow DOM. See the overlays.ts thread for detail.

class="modal-wrapper ion-overlay-wrapper"
part="content"
ref={(el) => (this.wrapperEl = el)}
Expand Down
17 changes: 17 additions & 0 deletions core/src/components/modal/test/a11y/modal.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,22 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
const results = await new AxeBuilder({ page }).analyze();
expect(results.violations).toEqual([]);
});

// The focused wrapper must not show a focus ring when opened via keyboard.
test('should not render a focus ring on the wrapper when presented via keyboard', async ({ page }) => {
await page.goto(`/src/components/modal/test/a11y`, config);

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
const button = page.locator('#open-modal');
const wrapper = page.locator('ion-modal .modal-wrapper');

// Open with the keyboard so :focus-visible applies to the wrapper.
await button.focus();
await page.keyboard.press('Enter');
await ionModalDidPresent.next();

await expect(wrapper).toBeFocused();
await expect(wrapper).toHaveCSS('outline-style', 'none');
});
});
});
36 changes: 36 additions & 0 deletions core/src/components/modal/test/card/modal-card.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,41 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c
expect(ionDragEnd.length).toBe(1);
expect(Object.keys(dragEndEvent.detail).length).toBe(4);
});

// Native drag can interfere with the swipe gesture when initiated over
// text in the focusable wrapper. Suppress it during the gesture only.
test('should cancel native drag and drop only while the gesture is active', async ({ page }) => {
await page.goto('/src/components/modal/test/card', config);

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');

await page.click('#drag-events');
await ionModalDidPresent.next();

const ionDragStart = await page.spyOnEvent('ionDragStart');
const ionDragEnd = await page.spyOnEvent('ionDragEnd');

const dispatchNativeDragStart = () => {
return page.evaluate(() => {
const content = document.querySelector('.modal-card ion-content')!;
const ev = new DragEvent('dragstart', { bubbles: true, cancelable: true, composed: true });
content.dispatchEvent(ev);
return ev.defaultPrevented;
});
};

const header = page.locator('.modal-card ion-header');

// Hold the drag mid-gesture without releasing
await dragElementBy(header, page, 0, 50, undefined, undefined, false);
await ionDragStart.next();

expect(await dispatchNativeDragStart()).toBe(true);

await page.mouse.up();
await ionDragEnd.next();

expect(await dispatchNativeDragStart()).toBe(false);
});
});
});
36 changes: 36 additions & 0 deletions core/src/components/modal/test/sheet/modal.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,42 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
expect(ionDragEnd.length).toBe(1);
expect(Object.keys(dragEndEvent.detail).length).toBe(5);
});

// Native drag can interfere with the swipe gesture when initiated over
// text in the focusable wrapper. Suppress it during the gesture only.
test('should cancel native drag and drop only while the gesture is active', async ({ page }) => {
await page.goto('/src/components/modal/test/sheet', config);

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');

await page.click('#drag-events');
await ionModalDidPresent.next();

const ionDragStart = await page.spyOnEvent('ionDragStart');
const ionDragEnd = await page.spyOnEvent('ionDragEnd');

const dispatchNativeDragStart = () => {
return page.evaluate(() => {
const content = document.querySelector('.modal-sheet ion-content')!;
const ev = new DragEvent('dragstart', { bubbles: true, cancelable: true, composed: true });
content.dispatchEvent(ev);
return ev.defaultPrevented;
});
};

const header = page.locator('.modal-sheet ion-header');

// Hold the drag mid-gesture without releasing
await dragElementBy(header, page, 0, 50, undefined, undefined, false);
await ionDragStart.next();

expect(await dispatchNativeDragStart()).toBe(true);

await page.mouse.up();
await ionDragEnd.next();

expect(await dispatchNativeDragStart()).toBe(false);
});
});

test.describe(title('sheet modal: late breakpoints binding'), () => {
Expand Down
23 changes: 22 additions & 1 deletion core/src/utils/overlays.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,28 @@ export const present = async <OverlayPresentOptions>(
* to the overlay container.
*/
if (overlay.keyboardClose && (document.activeElement === null || !overlay.el.contains(document.activeElement))) {
overlay.el.focus();
/**
* Some overlays (e.g. modal) put the dialog role and label on the
* `.ion-overlay-wrapper` rather than the host. Screen readers need
* focus on the element with role="dialog" to properly announce and
* navigate the dialog. The `[tabindex]` qualifier ensures we only
* target such a wrapper when it is actually focusable; otherwise
* (e.g. picker-legacy, whose role="dialog" wrapper has no tabindex)
* `focus()` would be a no-op, so we fall back to the host.
*/
const overlayWrapper = getElementRoot(overlay.el).querySelector<HTMLElement>('[role="dialog"][tabindex]');
const focusTarget = overlayWrapper ?? overlay.el;
/**
* `preventScroll` keeps this a pure focus move so the viewport does not
* jump when the wrapper is partially off-screen (e.g. a sheet modal).
* Guard the options call so an older engine that mishandles it can never
* reject present(); we fall back to a plain focus() in that case.
*/
try {
focusTarget.focus({ preventScroll: true });
} catch {
focusTarget.focus();
}
}

/**
Expand Down
82 changes: 82 additions & 0 deletions core/src/utils/test/overlays/overlays.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,5 +421,87 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
// verify focus is in correct location
await expect(input).toBeFocused();
});

// Focus the role="dialog" wrapper on present so screen readers can enter.
test('should focus the modal wrapper on present', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'FW-7611',
});
await page.setContent(
`
<ion-modal>
<ion-content>Modal Content</ion-content>
</ion-modal>
`,
config
);

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
const modal = page.locator('ion-modal');
const wrapper = page.locator('ion-modal .modal-wrapper');

await modal.evaluate((el: HTMLIonModalElement) => el.present());
await ionModalDidPresent.next();

await expect(wrapper).toHaveAttribute('role', 'dialog');
await expect(wrapper).toBeFocused();
});

test('should focus the sheet modal wrapper on present', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'FW-7611',
});
await page.setContent(
`
<ion-modal initial-breakpoint="0.5" breakpoints="[0, 0.5, 1]">
<ion-content>Sheet Modal Content</ion-content>
</ion-modal>
`,
config
);

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
const modal = page.locator('ion-modal');
const wrapper = page.locator('ion-modal .modal-wrapper');

await modal.evaluate((el: HTMLIonModalElement) => el.present());
await ionModalDidPresent.next();

await expect(wrapper).toHaveAttribute('role', 'dialog');
await expect(wrapper).toBeFocused();
});

test('should focus the card modal wrapper on present', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'FW-7611',
});
await page.setContent(
`
<div class="ion-page">
<ion-content>Root Content</ion-content>
</div>
<ion-modal>
<ion-content>Card Modal Content</ion-content>
</ion-modal>
`,
config
);

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
const modal = page.locator('ion-modal');
const wrapper = page.locator('ion-modal .modal-wrapper');

await modal.evaluate((el: HTMLIonModalElement) => {
el.presentingElement = document.querySelector<HTMLElement>('.ion-page')!;
return el.present();
});
await ionModalDidPresent.next();

await expect(wrapper).toHaveAttribute('role', 'dialog');
await expect(wrapper).toBeFocused();
});
});
});
Loading