Skip to content

FIX: Contain mobile tour and standardize touch targets - #2294

Open
romanlutz wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-fix-mobile-audit-regressions
Open

FIX: Contain mobile tour and standardize touch targets#2294
romanlutz wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-fix-mobile-audit-regressions

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

The July 28 exploratory audit found that onboarding tour steps could overflow a 390px viewport and leave actions partially inaccessible. It also identified interactive controls below the project's recommended 44px mobile touch-target size across Home, Configuration, History, and Chat.

This change makes Joyride position against the viewport instead of a narrow scroll-parent boundary, so every tour step and action remains contained. It also adds a shared mobile-only Fluent UI touch-target primitive and applies it to the audited controls and state-dependent actions while preserving compact desktop density.

Tests and Documentation

  • Added Playwright coverage for all five tour steps at mobile and desktop viewports.
  • Added 390x844 and 600px breakpoint-edge bounding-box checks across Home, Configuration, History, and Chat, including empty and error states.
  • Added document-overflow and desktop-density assertions.
  • Extended TourTooltip Jest coverage for Back, Next, Finish, Skip, and Close behavior.
  • Ran 11 affected Jest suites: 274 tests passed.
  • Ran 6 focused Playwright geometry tests: all passed.
  • Ran frontend lint, type-check, and production build successfully.
  • JupyText: N/A; no notebooks or documentation code samples changed.

Keep onboarding tour steps within the viewport and standardize mobile-only 44px touch targets across audited frontend surfaces.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ac28dca5-03cd-4d4e-809f-408112dc84db
hideArrow: true,
shiftOptions: {
// The tooltip portal must use the viewport, not a target's narrower scroll parent.
boundary: [] as Element[],

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.

Does useTour.test.ts:53 need updating for these? It toEquals floatingOptions exactly, might go red for once boundary/rootBoundary are added. Looks like no CI has run on the branch yet, so it may not have shown up


export const useMobileTouchTargetStyles = makeStyles({
control: {
'@media (max-width: 600px)': {

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.

Should this reuse styles/touchTargets.ts? #2258 added it on main with the same 44px helpers, but keyed on pointer: coarse - and the style-guide line it added says not to use viewport width for hit-area sizing. Would it be simpler to drop the touch-target half on the rebase and keep just the tour fix? The SystemPromptSetup change looks new though, worth carrying that one over?

And if you do switch to pointer: coarse, would the new mobile tests need test.use({ hasTouch: true })? That's what main's do, not sure they'd match on plain Desktop Chrome otherwise.

@varunj-msft varunj-msft self-assigned this Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants