-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5350
#5350 · imdreamrunner · internal (ENGOWNERS) Merged by the author mid-review; the draft is recorded for the findings. A/B vibe test — B's radius table produced both findings.
On touch, clearing a TouchDateField scrolls the page to the top: the clear
handler focuses the field synchronously inside the tap, and the browser scrolls
the focused element into view.
Defer the focus out of the gesture task, with an Effect that cleans up the pending call on unmount.
No API change, no theme targets, no breaking on any axis, no perf cost. Low-risk by R19 on all four conditions — no surface, no regression, no perf cost, nothing grew.
-
The same jump is one component away. The fix lives in
TouchDateField, but the mechanism — focus inside the tap — belongs to the shared clear button.InputClearButtondoes the same thing for every input that has one. - The new test pins
setTimeoutrather than the behavior. AssertingactiveElementafter timers survives moving the fix.
Both from the radius pass, neither reachable from the rubric — which points at DateInput and has nothing to say about the component next door. "Where does the fix not reach" produced the headline; "what would an unevadable fix look like" exposed the missing variant.
Arm A drafted request-changes with a different finding first: focus is lost
when changeAction disables the field mid-clear. jsdom said the PR introduced
it. Real WebKit said main does the same — WebKit blurs a newly-disabled
element, jsdom does not. The verdict flipped to approve on that one check.
→ R14i: jsdom cannot tell you something is a regression; a delta claim is a
two-sided measurement in a real engine.
Whether the iOS jump is real or whether deferral fixes it — WebKit will not run on this Mac and Chromium shows nothing in any variant. That is why finding 2 was written as a question, not a recommendation.
The class finding has no home now the PR is merged: an issue against
InputClearButton is owed.