fix(modal): focus the dialog wrapper on present so TalkBack can enter default modals#31260
Open
gnbm wants to merge 7 commits into
Open
fix(modal): focus the dialog wrapper on present so TalkBack can enter default modals#31260gnbm wants to merge 7 commits into
gnbm wants to merge 7 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
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
2 participants
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 an
ion-modalis opened with Android TalkBack enabled, the screen-reader user cannot navigate into or interact with any of the modal's content. Reported against a stock Ionic Angular Quickstart + the docs modal example (Pixel 9 / Android 16 / TalkBack 17.0.1).Root cause:
present()incore/src/utils/overlays.tsmoves DOM focus tooverlay.el(the shadow host) when no descendant is already focused.ion-modaldeclaresrole="dialog",aria-modal, and its accessible label on the inner.ion-overlay-wrapperelement inside the shadow root — not on the host (the role must live in the shadow DOM for VoiceOver; see the existing note inmodal.tsx). Focusing the role-less host hands assistive technologies a focus target with no accessible role or name. Screen readers that rely on the focus / accessibility-focus event to know a dialog opened — TalkBack in particular, which does not treataria-modalalone as a navigation boundary — get no usable landing point, so their linear-navigation cursor never enters the modal's content.What is the new behavior?
overlays.tspresent()now focuses the.ion-overlay-wrapper(which carries the dialog role/label) instead of the role-less host — but only when that wrapper actually declares atabindex(i.e. the component authored it to be focusable). Overlays that keep the role on the host and leave the wrapper non-focusable (action-sheet,loading,popover) continue to focus the host exactly as before. Focus is moved withpreventScrollso it never scrolls the viewport.ion-modalwrapper gainstabIndex={-1}so the retargetedfocus()lands on therole="dialog"element — scoped to default modals. For sheet and iOS card modals the wrapper doubles as the drag-gesture surface, and leaving focus on it interferes with pointer-drag recognition (it made the existingsheet modal: drag eventse2e time out on Firefox). Those modals keep focusing the host as before, so their gestures are unaffected. Improving programmatic focus for sheet/card modals is left as follow-up.Scope notes (verified during review, intentionally not included here)
ion-alertalready manages its own focus inpresent().then()(it focuses the single action button, or the.alert-wrapperotherwise) and is unaffected by this change — it was already accessible, so no alert code/tests are touched.ion-popoverhas a pre-existingaria-modalwith norole. Adding a defaultrole="dialog"was explored but reverted: an axe scan showed it makes every unlabeled popover fail the seriousaria-dialog-namerule (a dialog must have an accessible name), a consumer-facing regression. It needs a dedicated accessible-name strategy and is out of scope here.Tests
Does this introduce a breaking change?
No public API changes. Focus targeting is an internal a11y detail.