Skip to content

fix(NGTE-204): Tour hideOverlay should not trap interactions when set true#668

Draft
chris-connelly-cb wants to merge 1 commit into
coinbase:masterfrom
chris-connelly-cb:cconnelly/ngte-204-tour-hidden-overlay-rfc
Draft

fix(NGTE-204): Tour hideOverlay should not trap interactions when set true#668
chris-connelly-cb wants to merge 1 commit into
coinbase:masterfrom
chris-connelly-cb:cconnelly/ngte-204-tour-hidden-overlay-rfc

Conversation

@chris-connelly-cb
Copy link
Copy Markdown

When hideOverlay is true the Tour now behaves as a true non-modal coachmark: the root forwards pointer-events so the underlying page stays interactive, aria-modal is omitted, scroll is no longer locked, the FocusTrap is skipped, and the step container auto-focuses so keyboard users can still reach it. Also trims stale deps on the scroll-lock effect.

Adds tests covering aria-modal/pointer-events routing, mask absence, per-step override of the prop, and the default modal behavior.

What changed? Why?

Root cause (required for bugfixes)

UI changes

iOS Old iOS New
old screenshot new screenshot
Android Old Android New
old screenshot new screenshot
Web Old Web New
old screenshot new screenshot

Testing

How has it been tested?

  • Unit tests
  • Interaction tests
  • Pseudo State tests
  • Manual - Web
  • Manual - Android (Emulator / Device)
  • Manual - iOS (Emulator / Device)

Testing instructions

Illustrations/Icons Checklist

Required if this PR changes files under packages/illustrations/** or packages/icons/**

  • verified visreg changes with Terran (include link to visreg run/approval)
  • all illustration/icons names have been reviewed by Dom and/or Terran

Change management

type=routine
risk=low
impact=sev5

automerge=false

When `hideOverlay` is true the Tour now behaves as a true non-modal
coachmark: the root forwards `pointer-events` so the underlying page
stays interactive, `aria-modal` is omitted, scroll is no longer locked,
the FocusTrap is skipped, and the step container auto-focuses so
keyboard users can still reach it. Also trims stale deps on the
scroll-lock effect.

Adds tests covering aria-modal/pointer-events routing, mask absence,
per-step override of the prop, and the default modal behavior.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 1
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2
CODEOWNERS 🟡 See below

🟡 CODEOWNERS

Code Owner Status Calculation
ui-systems-eng-team 🟡 0/1
Denominator calculation
Additional CODEOWNERS Requirement
Show calculation
Sum 0
0
From CODEOWNERS 1
Sum 1

@@ -345,17 +349,54 @@ const TourComponent = <TourStepId extends string = string>(_props: TourProps<Tou
],
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.

First of all, thanks for making this pr!

Maybe it would be better to introduce a separate prop that enables/disables interaction with the background, because hideOverlay was purely a visual preference until this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants