Skip to content

fix(modal): focus the dialog wrapper on present so screen readers can enter the modal#31260

Merged
gnbm merged 26 commits into
mainfrom
FW-7611_investigation
Jul 17, 2026
Merged

fix(modal): focus the dialog wrapper on present so screen readers can enter the modal#31260
gnbm merged 26 commits into
mainfrom
FW-7611_investigation

Conversation

@gnbm

@gnbm gnbm commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Issue number: internal


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. The 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).
  • The modal wrapper gets tabIndex="-1" so it can receive that programmatic focus (an element with only role="dialog" is not focusable). Its focus ring is suppressed since the focus is programmatic, not keyboard-driven.
  • Clears text selection when gestures start to prevent Firefox from interrupting swipe-to-close and sheet gestures.
  • Adds an e2e test to modal/test/a11y/modal.e2e.ts to verify no visible outline added to the wrapper.
  • Adds e2e tests to utils/test/overlays/overlays.e2e.ts to verify the focus is on the wrapper instead of the host for all modals.
  • Fixes Playwright test helper to clear text selection immediately after mouse.down() to prevent the selection from blocking gesture detection in the test environment.

gnbm added 3 commits July 1, 2026 20:44
…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.
@vercel

vercel Bot commented Jul 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Jul 16, 2026 9:05pm

Request Review

@github-actions github-actions Bot added the package: core @ionic/core package label Jul 1, 2026
@gnbm gnbm changed the title fix(overlays): focus dialog wrapper on present so TalkBack can enter overlays (FW-7611) fix(overlays): focus dialog wrapper on present so TalkBack can enter overlays Jul 1, 2026
…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 gnbm changed the title fix(overlays): focus dialog wrapper on present so TalkBack can enter overlays fix(modal): focus the dialog wrapper on present so TalkBack can enter default modals (FW-7611) Jul 1, 2026
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 gnbm changed the title fix(modal): focus the dialog wrapper on present so TalkBack can enter default modals (FW-7611) fix(modal): focus the dialog wrapper on present so TalkBack can enter default modals Jul 1, 2026
Comment thread core/src/components/modal/modal.tsx Outdated
@gnbm
gnbm marked this pull request as ready for review July 2, 2026 11:14
@gnbm
gnbm requested a review from a team as a code owner July 2, 2026 11:14
@gnbm
gnbm requested review from BenOsodrac and removed request for BenOsodrac July 2, 2026 11:14
@gnbm
gnbm requested a review from brandyscarney July 7, 2026 17:51
Comment thread core/src/components/modal/test/a11y/modal.e2e.ts Outdated
Comment thread core/src/components/modal/modal.tsx Outdated
Comment thread core/src/components/modal/modal.tsx Outdated
gnbm and others added 2 commits July 7, 2026 22:00
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread core/src/utils/overlays.ts Outdated
…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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread core/src/components/modal/modal.scss

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@gnbm
gnbm requested a review from joselrio July 14, 2026 16:31

@joselrio joselrio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@codeCraft-Ritik codeCraft-Ritik left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a well-considered fix. Updating focus management to target the dialog element makes the modal much more accessible for screen reader users, while preserving compatibility with overlays that keep the role on the host. The cross-browser consideration is a nice touch.

@thetaPC thetaPC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gnbm
gnbm added this pull request to the merge queue Jul 17, 2026
Merged via the queue into main with commit 6251eb8 Jul 17, 2026
64 checks passed
@gnbm
gnbm deleted the FW-7611_investigation branch July 17, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants