Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 0dbf6f3 |
github-actions
Bot
removed
the
needs:spec-owner-review
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: the fix restores the keyboard height in the Safari focus pan it targets, but the new root-height floor makes a zoomed-out page report a keyboard that is not there.
layoutViewportHeight(packages/core/src/utils/layoutViewport.ts:49-52) compares the root's unscaled client height with the scaledinnerHeight * scalereading, so a page with nowidth=device-widthviewport or a fixedwidth=1024viewport now reads a 984–1,246 px keyboard with none shown (0 before), against AST-099 FR2; apply the floor only where the units match (for example at a visual-viewport scale of 1) and add a no-keyboard test for a zoomed-out page.- The ride formula in
packages/core/src/keyboard/README.md(line 96) still computes the keyboard's top asinnerHeight − height, which is now 40 px during Safari's focus pan instead of the 377 px the hook uses, so it should readlayoutViewportHeight(window) − height. - The Android claims (
resizes-contentstill reading 0, Chrome's toolbar-hidden height kept) and the scroll-clamp change rest on unit fixtures with assumed root heights, while AST-099 DEC-3 asks for emulator evidence for Android claims, so please add emulator readings (Chrome with Gboard and Firefox for Android) and a Simulator reading of the clamp state at the fixed head.
The four red-first unit cases and the Simulator capture show the focus-pan fix itself.
[Automated review]
0dbf6f3 to
00122d3
Compare
There was a problem hiding this comment.
Still requesting changes: the scale-1 gate restores the keyboard height during Safari's focus pan, but it lets a fixed-width page wider than the screen read a keyboard that is not there, and the Android readings the keyboard record requires are missing.
layoutViewportHeight(packages/core/src/utils/layoutViewport.ts:52-56) takes the root's client height whenever the scale is 1 andinnerHeightequals the visual viewport's height, so awidth=1024, initial-scale=1, user-scalable=no(orminimum-scale=1) page with no keyboard reads a 1,246 px keyboard in Chromium's Pixel 7 emulation where it read 0 before, against AST-099 FR2; also require the root to be as wide as the visual viewport, and add a no-keyboard test for a wider layout at scale 1.- The new branch also runs on Android, but the change has no emulator evidence, which AST-099's Platform support and DEC-3 require at the exact head before merge; please add keyboard-height readings in Chrome with Gboard (default and
resizes-content) and in Firefox for Android at the fixed head.
The red-first pan tests and the Simulator capture show the focus-pan fix itself.
[Automated review]
00122d3 to
280bac1
Compare
|
/approve-spec 280bac1 |
There was a problem hiding this comment.
Requesting changes: the scale-1 root floor now restores the keyboard height during Safari's focus pan and keeps a layout wider than the screen at no keyboard, but the Android emulator readings the keyboard record requires at this head are not on the pull request.
- The new branch in
layoutViewportHeight(packages/core/src/utils/layoutViewport.ts:54-61) also runs on Android, and AST-099's Platform support and DEC-3 make emulator evidence at the exact head a merge gate, so please post here the readings the description attributes to another run:innerHeight, the root's client height, the visual viewport and the module's keyboard height, keyboard up and down, in Chrome with Gboard (default andresizes-content) and in Firefox for Android.
The red-first pan tests, the new wide-layout tests and the Simulator capture cover the iOS fix itself.
[Automated review]
| expect(layoutViewportHeight(win)).toBe(377); | ||
| }); | ||
|
|
||
| it('keeps innerHeight where it is taller than the root, as in Chrome on Android with its toolbar hidden', () => { |
There was a problem hiding this comment.
This case and the resizes-content case above encode Android readings from assumed root heights. AST-099's Platform support makes emulator evidence at the exact head a merge gate, so please post the emulator's own readings at this head on the pull request: Chrome with Gboard (default and resizes-content) and Firefox for Android, keyboard up and down.
…pans to a focused field
While Safari on iOS 26.2 pans the page to a focused field near the bottom,
it reports innerHeight as the visual viewport's height (377 of 714 with
the keyboard up). layoutViewportHeight read innerHeight, so the keyboard
module measured 0, as if no keyboard were up, against AST-099 FR2 ("a page
pan is never a smaller keyboard"). The root's client height keeps the
layout viewport's height. At a visual-viewport scale of 1, where
innerHeight equals the visual viewport's height and the root is as wide
as the visual viewport, layoutViewportHeight now takes the larger of the
two. Everywhere else it is unchanged: a zoomed-out page's scaled
innerHeight and the root's unscaled height are in different units, a
layout wider than the screen gives a root taller than the screen, and the
scroll clamp reads as before.
The README's ride formula now takes the keyboard's top from
layoutViewportHeight(window), not innerHeight.
280bac1 to
86c74e8
Compare
There was a problem hiding this comment.
Requesting changes: the scale-1 root floor restores the keyboard height during Safari's pan to a focused field and keeps zoomed-out and wider-than-screen layouts at no keyboard, but the Android emulator readings the keyboard record requires at the exact head are still not on the pull request.
- The new branch in
layoutViewportHeight(packages/core/src/utils/layoutViewport.ts:54-61) also runs on Android, and AST-099's Platform support makes emulator evidence at the exact head a merge gate, so please post here the readings the description attributes to another run (innerHeight, the root's client height, the visual viewport and the module's keyboard height, keyboard up and down) in Chrome with Gboard, default andresizes-content, and in Firefox for Android.
The red-first pan tests and the Simulator capture cover the iOS fix itself.
[Automated review]
|
The readings at this head are now in the description's Testing section. They come from the Android 16 emulator: Chrome 133 with Gboard, default and
The root's client height equals |
|
I'll run a full review to re-check this with your note. |
There was a problem hiding this comment.
Approving: the scale-1 root floor in layoutViewportHeight restores the whole keyboard height during Safari's pan to a focused field and keeps zoomed-out and wider-than-screen layouts at no keyboard, and the Android emulator readings at this head now cover Chrome with Gboard (default and resizes-content) and Firefox for Android.
Approved pending CI: CI has not run on this head yet, because its branch is in a fork; a maintainer adds the ci:run label so CI runs.
[Automated review]
cixzhang
added
the
ci:run
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 76289f2 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Requesting changes: the scale-1 root floor in layoutViewportHeight still restores the whole keyboard height during Safari's pan to a focused field and keeps zoomed-out and wider-than-screen pages at no keyboard, and CI passes on this head, but the merge from main needs the Android emulator readings taken again.
- Regression: merging main moved the head after the Android emulator readings were taken, and AST-099's Platform support requires them at the exact head now that the merged docked ride, reveal and room checks read
layoutViewportHeight, so please re-take the Chrome-with-Gboard (default andresizes-content) and Firefox for Android readings, keyboard up and down, with the three animation and device profiles, on a build of this head, and refresh the Testing section, which still describes the earlier rebase.
[Automated review]
The pan check in layoutViewportHeight read the visual viewport's height a second time in each keyboard measurement, and every reader that laid out against the keyboard measured the layout viewport again. The keyboard module now passes the height it read, and keeps the layout viewport's height with each report, which getKeyboardViewportHeight returns. ChatLayout's keyboard hook, useMobileKeyboard, the Layer's keyboard clearance and the keyboard target read it there.
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 7952b5f |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Requesting changes: the scale-1 root floor in layoutViewportHeight restores the whole keyboard height during Safari's pan to a focused field, and the Chrome-with-Gboard readings at this head now cover normal animation scale, Remove animations and 4× CPU, but Firefox for Android still lacks two of the profiles the keyboard spec requires at the exact head.
- Please take the Firefox for Android emulator readings (default and
resizes-content, keyboard up and down) on a build of this head with Remove animations and under a slow-device profile, and add them to the Testing section, because AST-099's Platform support names Firefox as the Android proof of FR2–FR5 and requires both profiles at the exact head.
Nothing else in the change blocks.
[Automated review]
|
Re-taken at 7952b5f, in the description's Testing: Chrome with Gboard (default and |
|
Chrome with Remove animations (reduced motion active) and at 4× CPU are now re-taken at 7952b5f as well. All eight readings equal those at normal speed, to 0 px. With the normal-speed Chrome and Firefox 159 readings, they are in the description's Testing. |
useChatKeyboard.ts takes main's modality and focusReturn imports, and keyboardTarget.ts main's nearestScroller import. Neither imports layoutViewport: their readers use getKeyboardViewportHeight.
|
/approve-spec 348b522 |
There was a problem hiding this comment.
Requesting changes: the scale-1 root floor in layoutViewportHeight restores the whole keyboard height during Safari's pan to a focused field (the pan cases fail on main and pass here), and the code shows no defect; one evidence item remains.
- The Android emulator readings that
spec:AST-099Platform support requires at the exact head are missing: the newest are from earlier heads, andspec:AST-094DEC-31 cannot carry them because thismainmerge changeduseChatKeyboard.tsandkeyboardTarget.ts(both changed here) andmotion/animator.ts, which they import.
Next: take Chrome with Gboard and Firefox for Android in the emulator on a build of this head (default and resizes-content, keyboard up and down) at normal animation scale, with animations removed and under a slow-device profile, and add them to the Testing section. Required CI has not run on this head yet, as the pull request is a draft.
[Automated review]
Stack: #9983 → #8217
Merge group: #9983, #8217
User impact
While Safari on iOS pans the page to a focused field near the bottom, the keyboard module measured no keyboard. Readers such as
useMobileKeyboard's recovery check took it as down.Problem and solution fit
layoutViewportHeightreadinnerHeight, but the root's client height keeps the layout viewport's 714. At a visual-viewport scale of 1,innerHeightcan equal the visual viewport's height while the root is as wide as the visual viewport. In that state, taking the larger of the two restores the keyboard's height. Elsewhere nothing changes. A zoomed-out page's scaledinnerHeightand unscaled root differ in units, and a wider-than-screen layout has a taller root. The scroll clamp reads as on main.Expected behavior and authority
spec:AST-099FR2: "A page pan or a visual-viewport offset is a pan to put back (FR4), never a smaller keyboard". The FR2 verification row now names Safari's pan to a focused field and the Simulator.Smallest restoration
layoutViewportHeightreturns the larger of its current value and the root's client height (the body's in quirks mode), in that one state. The keyboard module keeps the layout viewport's height it measured with each report (getKeyboardViewportHeight), for the keyboard's readers and the README's formula. No API change.Evidence
The module's own reading on a page Safari pans (a local probe story, not in the diff), iOS 26.2 Simulator, hostname masked:
innerHeight377, root 714, visual viewport 377 panned 337: the module reads 0 (2 takes).innerHeightin every reading, so the floor returns main's value.layoutViewport.test.tsandkeyboardHeight.test.tspan cases, red on main.Open PRs on these files: #9777 adds
layoutViewport.tsfrom an older base and takes this fix when it rebases.Scope
Writing rubric results
R1 narration: none (the row states what verifies FR2). R2 research: none. R3 evidence: the row names its tests and the Simulator, no change. R4 rejected alternatives: none. R5 change references: none. R6 open questions: none.
Testing
useChatKeyboard.tstakes main's interaction-modality and focus-return imports, andkeyboardTarget.tsmain'snearestScrollerimport. Neither importslayoutViewportnow, as their readers usegetKeyboardViewportHeight. The change's own lines are unchanged. At the merge, ChatLayout's INV11 browser test passes 3 of 3 in Chromium. The Chat, keyboard and Layer interaction specs pass: Chromium 60, WebKit 45 with 15 skipped. Keyboard, Chat, Layer andlayoutViewportunits: 1,262 of 1,262.pnpm test:ui: 965 files, 23,490 tests pass. Typecheck 0,CI=true pnpm lint0 errors,pnpm check:knowledgeclean.layoutViewportHeight, and keeps the layout viewport's height with each report (getKeyboardViewportHeight). ChatLayout's keyboard hook,useMobileKeyboard, the Layer's keyboard clearance and the keyboard target read it there. INV11 browser test, Chromium ×3: 3 of 3 fail on the plain merge, 3 of 3 pass here. Two new units count one read per report under the pan, the second with a reader laying out; each fails without the fix (2 reads). Chat, keyboard and Layer interaction specs: Chromium 53 passed; WebKit 41 passed, 12 skipped. Core UI suite (pnpm test:ui): 920 files, 22,546 tests pass;CI=true pnpm lint0 errors.layoutViewport.ts, the 2 pan cases fail (one per test file); with the change all pass. The wide-layout, zoomed-out and scroll-clamp cases pass on main and here. The wide-layout cases fail on the previous head (a 1,105 px keyboard with none shown).vitest --project uikeyboard module andlayoutViewport: 74/74. Core units: 21,147 of 21,151 pass. The other 4, inSwitch.focusRing.source-build.test.mjsandRadioListItem.press.source-build.test.mjs, fail on main too (a fresh install has no@babel/preset-reactat the root).CI=true) 0 errors; Prettier;pnpm check:knowledge.Android emulator readings at 280bac1, the pan floor's patch before the one-read report, in CSS px. A local probe page, not in the diff, reads the module built from that head. Android 16
BE2A.250530.026.D1, Chrome 133.0.6943.137 with Gboard 15.1.08, Firefox 159.0.1. Each reading taps a field at the page's bottom, then presses Back:innerHeightresizes-contentresizes-contentresizes-contentresizes-contentresizes-content,innerHeightequals the visual viewport's height at scale 1 with the keyboard up, so the new branch runs. The root shrinks with them, so it returns the same value as main.resizes-content: no pan, and 193 px. BottomSheet's field sits 48.2 px above the keyboard (default) and on its top (resizes-content). The same numbers at 4× CPU slowdown and with animations removed, and Back returns the viewport to 783.2 px at offsetTop 0. In one take with animations removed, Back left the keyboard up; it hid in every other take.resizes-content. Chrome: AI chat 64.2 px clear with no pan (default) and 193 px (resizes-content); BottomSheet 48.2 px (default) and flush (resizes-content). Firefox matches within 1 px, with no pan: 64.1, 193, 48.1 px and flush. Keyboard down returns the viewport to full at offsetTop 0 in every reading (Chrome 783.2 px, Firefox 779.3 px). With Remove animations (reduced motion active) and at 4× CPU, Chrome's eight readings equal those at normal speed, to 0 px. In Firefox a real tap on "Add a comment" opens the sheet, 3 of 3 on main and on 7952b5f.spec:AST-099FR3, with FR4: focus never pans the page). It came in with the main merge. Chromium's phone emulation with a 312 px virtual keyboard shows it, as a proxy with no real keyboard and no browser pan. In 3 takes each, 76289f2 leaves the composer under the keyboard. The merge without the one-read commit, and 7952b5f, both put the card on the keyboard's top.resizes-contentat 4× CPU, whose composer did not render in time in 7 tries. Firefox was not read either: no keyboard report reached the page in 2 tries. The probe-page rows above read Firefox at 280bac1.