Skip to content

fix(DateInput): clearing on touch no longer jumps the page to the top - #5350

Open
imdreamrunner wants to merge 1 commit into
mainfrom
fix/date-input-clear-scroll-jump
Open

fix(DateInput): clearing on touch no longer jumps the page to the top#5350
imdreamrunner wants to merge 1 commit into
mainfrom
fix/date-input-clear-scroll-jump

Conversation

@imdreamrunner

@imdreamrunner imdreamrunner commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

The bug

On touch, tapping DateInput's clear (✕) throws the user to the top of the page.

Repro (real, on the live docsite): open
https://astryx.atmeta.com/components/DateInput on a phone, scroll to the second
date input — the DateInput — Clearable example, "Event date / April 6,
2026" — and tap its ✕. The page jumps to the top. React state survives, so it
is an in-page scroll jump, not a reload.

Cause

TouchDateField.handleClear clears the value and focuses the field in the same
task. Clearing unmounts the clear button — it only renders while there is a
value — and on iOS Safari, focusing an element in the same task as the focused
button's removal scrolls the whole document to 0.

Measured on the iOS 26 simulator driving real taps against the live docsite,
field at scrollY 2055, with every scroll API instrumented:

variant result
synchronous focus() (today) 2055 → 0
synchronous focus({preventScroll: true}) 2055 → 0
focus suppressed entirely 2055 (no move, but focus lost to <body>)
deferred focus({preventScroll: true}) 2055 (no move, focus kept)

What is not happening, all instrumented and silent through the jump: no
scrollTo / scroll / scrollBy / scrollIntoView, no scrollTop write, no
history.pushState/replaceState/hashchange, no scroll-lock body pinning,
and document.scrollHeight never changes (5156 throughout, so it is not a
height collapse clamping the scroll). Focus on the same field while the page is
quiet does not move it either — the unmount is required.

The fix

Defer the focus handoff past the unmount. The page stays put and focus still
returns to the field, so it is not lost to <body>.

preventScroll is kept: the ordinary scroll-into-view nudge is real too (+12px
on a plain page) and unwanted for the same reason — the field the user just
tapped is already on screen. It is not sufficient on its own, per the table.

Only the touch surface is affected; the pointer surface is a different
component and does not do this.

Test plan

  • New test in DateInputTouch.test.tsx: focus is not called synchronously,
    and after timers run it is called with {preventScroll: true}. Negative
    control: restoring the synchronous call fails it.
  • packages/core/src/DateInput — 229 tests pass.
  • pnpm lint:strict — 0 errors (55 pre-existing warnings).
  • pnpm test — 11376 pass; the 11 failures are all packages/cli, the known
    devvm full-run load flake, green under vitest run --project node packages/cli (200 files / 2780 tests).
  • pnpm build plus the build-storybook typechecks
    (core|lab|charts typecheck:docs, cli typecheck:{strict,template-docs},
    storybook typecheck, core typecheck, lab:readiness:check,
    storybook build) — all exit 0.

Note on earlier revisions of this PR

The first version of this PR changed DateRangeInput, Selector and
MultiSelector — none of which are DateInput and none of which are the touch
surface. That was wrong and has been dropped. Those three do have a separate,
real defect (their clear leaves focus on <body>); worth its own PR, not this
one.

@vercel

vercel Bot commented Aug 23, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview Aug 23, 2026 7:13am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 23, 2026
@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

DateInput (@astryxdesign/core) · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A N/A N/A
Lines of Code N/A 2713 -
Complexity N/A Very High (274) -

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.8KB 1.2KB

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Aug 23, 2026
@imdreamrunner imdreamrunner changed the title fix: clearing a field no longer throws keyboard focus to the top of the page fix(DateInput): clearing on touch no longer scrolls the page Aug 23, 2026
@imdreamrunner
imdreamrunner force-pushed the fix/date-input-clear-scroll-jump branch from 41d6cb4 to 102fee1 Compare August 23, 2026 06:26
github-actions Bot added a commit that referenced this pull request Aug 23, 2026
On the touch surface, tapping the clear (X) threw the user to the top of the
page. Clearing unmounts the clear button, and handleClear focused the field in
that same task; on iOS Safari, focusing an element as the focused button is
removed scrolls the whole document to 0.

The focus handoff is deferred past the unmount. The page stays put and focus
still lands on the field, so it is not lost to <body>.

Measured on the iOS 26 simulator driving real taps against the live docsite
(DateInput - Clearable, page at scrollY 2055), with every scroll API and the
scroll lock instrumented:

  synchronous focus()                    -> 2055 -> 0
  synchronous focus({preventScroll})     -> 2055 -> 0
  focus suppressed entirely              -> 2055 (no move, focus lost)
  deferred focus({preventScroll})        -> 2055 (no move, focus kept)

No script scrolls the page: no scrollTo/scroll/scrollBy/scrollIntoView, no
scrollTop write, no history navigation, no scroll-lock body pinning, and the
document height never changes. preventScroll is kept because the ordinary
scroll-into-view nudge is real too (+12px on a plain page) and unwanted for the
same reason.
@imdreamrunner imdreamrunner changed the title fix(DateInput): clearing on touch no longer scrolls the page fix(DateInput): clearing on touch no longer jumps the page to the top Aug 23, 2026
@imdreamrunner
imdreamrunner force-pushed the fix/date-input-clear-scroll-jump branch from 102fee1 to 12617af Compare August 23, 2026 07:10
github-actions Bot added a commit that referenced this pull request Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant