fix(modal): focus the dialog wrapper on present so screen readers can enter the modal#31260
Open
gnbm wants to merge 22 commits into
Open
fix(modal): focus the dialog wrapper on present so screen readers can enter the modal#31260gnbm wants to merge 22 commits into
gnbm wants to merge 22 commits into
Conversation
…resent
IONIC-91 / FW-7611: Android TalkBack users could not navigate into or
interact with modal content after opening it.
`present()` in overlays.ts moved DOM focus to `overlay.el` (the shadow
host) when no descendant was already focused. Every overlay built on
this shared utility (modal, alert, action-sheet, loading, popover)
declares `role="dialog"`/`aria-modal` on an inner `.ion-overlay-wrapper`
element inside its shadow root, never on the host itself. Focusing the
host therefore handed assistive tech a focus target with no accessible
role or name, so TalkBack's accessibility-focus never landed on the
actual dialog and its linear navigation cursor never entered the
overlay's content.
Focus the `.ion-overlay-wrapper` instead (falling back to the host if
none exists), and make modal's wrapper focusable via tabIndex={-1} so
the retargeted focus() call actually takes effect.
…ually focusable Alert declares role="alertdialog" and tabindex="0" on .alert-wrapper, so redirecting focus there (as done for modal) is correct and already works. Action-sheet, loading, and popover keep role/aria-modal on the host and never gave .ion-overlay-wrapper a tabindex, so it was never meant to be focused directly. The previous version of this fix called .focus() on that non-focusable wrapper unconditionally, which silently failed and left focus on <body> instead of the host -- a regression against their prior, correct behavior. Guard the redirect on the wrapper actually declaring a tabindex so only overlays authored to use it are affected; others keep focusing the host exactly as before.
Popover set aria-modal="true" on the host but declared no role at all. Per the ARIA spec, aria-modal is only defined on elements with role dialog or alertdialog, so assistive technologies were silently ignoring it -- popovers were never actually exposed as modal to screen readers. ion-select already declares aria-haspopup="dialog" on its trigger when using the popover interface, so this also fixes a pre-existing mismatch between what select promised and what the popover actually exposed. Default to role="dialog", placed before the htmlAttributes spread so consumers can still override it (e.g. role="menu") the same way modal and alert allow.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
gnbm
changed the title
…ocus for sheet/card
The initial fix made the modal's `.modal-wrapper` (which carries
role="dialog") the focus target on present so Android TalkBack can enter
the dialog. For sheet and iOS card modals that wrapper is also the
drag-gesture surface: leaving focus on it interferes with pointer-drag
recognition (observed as the sheet "drag events" e2e timing out on
Firefox). Real users are unaffected (the gesture works once focus
settles), but it is a genuine behavior change and broke a merge-gating
test.
Scope the wrapper `tabIndex={-1}` to default modals only. Sheet and card
modals keep focusing the host exactly as before, so their drag gestures
are untouched, while the reported IONIC-91 case (default modal) still
gets the accessible focus target. Also focus the wrapper with
`preventScroll` so the a11y focus move never scrolls the viewport.
Review + a new axe scan showed that defaulting role="dialog" on ion-popover makes every *unlabeled* popover fail axe's serious `aria-dialog-name` rule (an ARIA dialog must have an accessible name) -- a consumer-facing regression. Revert the popover role change (and its tests) so this PR stays scoped to the verified modal (IONIC-91) focus fix. Popover's missing role can be revisited with a proper accessible- name strategy. Also tighten the modal a11y test comment to match the surrounding concise style.
gnbm
changed the title
Match the focus-assertion style used across the modal suite (expect(locator).toBeFocused()) and the existing `.modal-wrapper` locator in this file, instead of a manual page.evaluate over shadowRoot.activeElement.
gnbm
changed the title
gnbm
commented
Jul 2, 2026
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
…dals too The wrapper now carries tabindex=-1 for all modal types, not just the default modal, so present() moves focus to the element that declares the dialog role and TalkBack users can enter sheet and card modals. The previous exclusion existed because making the wrapper focusable made the 'sheet modal: drag events' e2e hang on Firefox. Root cause: Gecko treats an element with tabindex as a selection root - a pointer press inside it places a text caret, and a later press over that caret starts a native drag and drop session instead of delivering pointer events. The gesture then never receives the final pointerup. The sheet and swipe-to-close gestures now cancel any native dragstart while a drag is active, which prevents the hijack without affecting drag and drop when the modal is not being dragged. Adds wrapper-focus e2e coverage for sheet and card modals.
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
- Move the focus-on-present e2e tests (default, sheet, card) to
overlays.e2e.ts, since overlays.ts owns the focus behavior. Keep the
modal-specific axe and keyboard focus-ring tests in modal a11y.
- Drop the native-drag workaround tests' issue annotations and remove
file-name references from their comments, per review.
- Clarify in modal.tsx why tabIndex={-1} is required (role="dialog"
alone is not focusable, so focus() would be a no-op).
gnbm
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves modal accessibility by ensuring focus is moved to the element that actually has role="dialog" when the modal is presented, enabling Android TalkBack (and similar AT) to properly enter and announce the modal. It also adds a gesture stability workaround for Firefox where native drag can swallow the sheet/card swipe gesture, plus associated Playwright coverage.
Changes:
- Update overlay
present()logic to focus the overlay’s dialog element (when applicable) instead of always focusing the overlay host. - Make the modal wrapper programmatically focusable (
tabIndex={-1}) and suppress its outline to avoid a visible focus ring on open. - Prevent native
dragstartduring active sheet/card swipe gestures; add e2e tests covering focus behavior and the drag workaround.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| core/src/utils/test/overlays/overlays.e2e.ts | Adds e2e assertions that focus lands on the modal dialog wrapper for default/sheet/card modals. |
| core/src/utils/overlays.ts | Changes present() to focus a dialog element rather than the overlay host. |
| core/src/components/modal/test/sheet/modal.e2e.ts | Adds test ensuring native drag is prevented only while the sheet gesture is active. |
| core/src/components/modal/test/card/modal-card.e2e.ts | Adds test ensuring native drag is prevented only while the card swipe gesture is active. |
| core/src/components/modal/test/a11y/modal.e2e.ts | Adds a11y regression coverage ensuring the wrapper does not show an outline when opened via keyboard. |
| core/src/components/modal/modal.tsx | Makes the modal wrapper focusable via tabIndex={-1} to support the new focus target. |
| core/src/components/modal/modal.scss | Suppresses the modal wrapper outline so programmatic focus does not render a focus ring. |
| core/src/components/modal/gestures/swipe-to-close.ts | Prevents native dragstart during the active card swipe-to-close gesture. |
| core/src/components/modal/gestures/sheet.ts | Prevents native dragstart during the active sheet swipe gesture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
present() queried the overlay for `[role="dialog"]`, but picker-legacy puts that role on a wrapper with no tabindex. Focusing a non-focusable element is a no-op, so focus would not move into the picker (a regression from focusing the host). Qualify the query with `[tabindex]` so only a focusable dialog wrapper (e.g. the modal wrapper) is targeted; otherwise fall back to the host, preserving prior behavior.
…le options
present() runs on a critical path, so wrap the focus({ preventScroll })
call in a try/catch that falls back to a plain focus(). This keeps the
no-scroll behavior on modern browsers while ensuring a focus call can
never reject present() on an engine that mishandles the options object.
joselrio
approved these changes
Jul 14, 2026
joselrio
left a comment
joselrio
left a comment
Contributor
There was a problem hiding this comment.
After looking into the suggested stuff and also run tests around this directly on the android device through the https://github.com/OutSystems/ui-comp-ionic-sandboxes repo and everything's running as expected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the current behavior?
When a modal is opened with Android TalkBack enabled, the screen reader cannot enter or navigate the modal's content. On present, focus is moved to the modal host, but the host is role-less —
role="dialog",aria-modal, and the label live on the inner.ion-overlay-wrapper— so assistive technologies have no dialog to land on.What is the new behavior?
present()now focuses the overlay's[role="dialog"]element (the wrapper) instead of the role-less host, falling back to the host when the role is on the host (action-sheet, loading).tabIndex="-1"so it can receive that programmatic focus (an element with onlyrole="dialog"is not focusable). Its focus ring is suppressed since the focus is programmatic, not keyboard-driven.Fixes the reported issue for default, sheet, and card modals.
Testing
The original issue reproduces on a physical Android device with TalkBack (reported on Pixel 9 / Android 16 / TalkBack 17.0.1) and on a Pixel 10 / Android 17 emulator launched outside of Android Studio. Virtual devices launched from inside Android Studio do not reproduce it.
Screen reader — reported issue (default, sheet, and card modals)
Fail scenario (before the fix):
mainand run it on the device/emulator above.breakpoints/initialBreakpoint) → same failure.Pass scenario (with this fix):
npm i @ionic/core@<dev-build>/ the framework packages).Browser-quirk workaround — Firefox native drag
This PR cancels a native drag that Firefox starts over the now-focusable wrapper. To verify:
Automated (Playwright e2e)
core/src/utils/test/overlays/overlays.e2e.ts— focus lands on the[role="dialog"]wrapper on present for default, sheet, and card modals.core/src/components/modal/test/a11y/modal.e2e.ts— no axe violations; the wrapper shows no focus ring when the modal is opened via keyboard.core/src/components/modal/test/{sheet,card}/*.e2e.ts—should emit ionDragStart, ionDragMove, and ionDragEnd events(fails on Firefox without the native-drag workaround) andshould cancel native drag and drop only while the gesture is active(assertsdragstartis prevented during the gesture and allowed after).