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
In a page that scrolls the document, Safari on iOS pans the visual viewport to the fixed dock when the keyboard opens, and keeps the pan. ChatLayout's lift added to that pan, so the composer went under the status bar. Where no layout guide publishes the keyboard's slide, ChatLayout now leaves the tap, the reveal and the dock's place to the browser in that mode. With the VirtualKeyboard API, as in Chrome on Android, and in self-scroll mode, nothing changes. AST-099 FR3 states it.
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
b58dab0 to
fa8cf7b
Compare
cixzhang
added
the
ci:run
|
/approve-spec fa8cf7b |
github-actions
Bot
removed
the
needs:spec-owner-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The spec:AST-099 FR3 exception is approved at this head, so the direction is settled, and four fixes remain:
leavesRevealToBrowserandisKeyboardSurfacetest!isSelfScrolling, so ascrollRefon an inner scrolling element also loses the dock lift, the focus takeover and the composer group, while the approved FR3 text, the changeset, the docs and the JSDoc limit the change to a page that scrolls the document; gate on the document's scroller or widen the record and the evidence.ChatLayout.doc.mjsstill says every way into the composer focuses it without scrolling the page (line 9) and that Done rests focus on the composer group (line 138), which is false withscrollRefin Safari on iOS and Firefox for Android, anddocsZhneeds the same update.- Firefox for Android takes the new path unmeasured, yet the docs now say the browser reveals the composer there, and AST-099 makes Firefox for Android emulator evidence and iOS home-screen app evidence a merge gate, so add those results or limit the claim to Safari.
- FR9's Docked containers clause and the FR3–FR4 verification row still require ChatLayout's dock to rise in Safari on iOS, so they need the same exception, followed by a new
/approve-specat the new head.
CI has not run on this head yet, because the pull request is a draft.
[Automated review]
The exception now applies only when scrollRef names the document's own scroller: its root or its body. A scrollRef on an inner scrolling element keeps the lift, the tap claim and the focus takeover. The docs name the exception for Safari on iOS, in both languages. AST-099 FR9's docked-container clause and the FR3-FR4 verification row name it too.
FR3 and FR9's docked-container clause now state the rule the code keys on: ChatLayout's scrollRef names the document's scroller.
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec b143b86 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested. The spec:AST-099 FR3 exception for a ChatLayout whose scrollRef names the document is approved at this head, and the gate now covers only the document's scroller, so the direction is settled. Three fixes remain:
- Firefox for Android and the iOS home-screen app also lose the dock lift, the tap claim and the pan put-back through
leavesRevealToBrowser, with no Firefox for Android emulator run or home-screen app run, which AST-099's Platform support makes part of the merge evidence, andChatLayout.doc.mjslines 9, 124 and 138 still promise Firefox for Android focus without page scroll and Done's park on the composer group. - The
composerprop description (ChatLayout.doc.mjs:110) still says the wrapper is a group where Done rests focus, with no exception for a documentscrollRefin Safari on iOS. - Miss: the browser-support guide (
packages/cli/assets/docs/browser-support.doc.mjs:151) still says ChatLayout's composer leaves the page where it was and puts a Firefox pan back, which no longer holds with a documentscrollRef.
The inner-scroller reach and the FR9 and verification-row wording from the last review are fixed. CI has not run on this head yet because the pull request is a draft.
[Automated review]
ChatLayout's docs, in English, Chinese and the dense line, and the composer prop's description name the exception's real scope: a scrollRef on the document in a browser without the VirtualKeyboard API, as Safari on iOS and Firefox for Android. There ChatLayout leaves focus and the reveal to the browser, with no composer group. The browser-support guide names it beside the Firefox pan put-back.
|
/approve-spec 37f7a9b |
There was a problem hiding this comment.
Changes requested: the spec:AST-099 FR3 exception for a ChatLayout whose scrollRef names the document is approved at this head, and the docs now name every browser it reaches, so one fix remains:
- Firefox for Android and the iOS home-screen app lose the dock lift, the tap claim and the pan put-back through
leavesRevealToBrowser(useChatKeyboard.ts:526-528) with no Firefox for Android emulator run or home-screen app run, which AST-099's Platform support makes part of the merge evidence, whileChatLayout.doc.mjsand the browser-support guide now say Firefox for Android reveals the composer there, so please add those runs for a document-scrolling ChatLayout (tap, previous and next,focus(), Done).
The composer prop description, the Firefox sentences in the ChatLayout docs and the browser-support guide from the last review are fixed. CI has not run its build and test workflows on this head yet because the pull request is a draft.
[Automated review]
|
/approve-spec 386ebb1 |
|
/approve-spec e1968f7 |
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the spec:AST-099 text at this head: this push rewrote FR3 (the dock now follows the cover the pan leaves after a touch or a wheel, and holds where the layout viewport follows the pan), FR9's docked clause and Gap 16, and added Gap 17 and a third Firefox for Android row, so an engineering owner records the decision by commenting /approve-spec <full head SHA>, which the author may do as an engineering owner.
Independent of that decision, these need fixing:
- The follow mode recommits the dock's
bottomand the transcript's padding, and readsscrollToparound those writes, on every visual-viewport report while the reader scrolls (useChatKeyboard.ts:364-371,:289,:425), whicharchitecture:react-component-runtimeINV18 and AST-099 FR3's "no per-frame layout work" do not allow, so follow the pan with the dock's compositor translate and commit the layout lift once the pan rests. - A composer show under a pinch zoom still teaches "covered" because the pan reads as null while the page is zoomed (
keyboardHeight.ts:752), so in Safari on iOS one zoomed show brings back the stacked lift for every later tap on the page; follow the cover without learning when the pan is null, and add a zoomed case. - AST-099 Platform support makes Firefox for Android emulator evidence at the exact head merge evidence, and the description gives the 731–755 ms cover without its head and no result for the third Firefox row (
spec.md:445), which checks the follow mode this push added.
The other-field learning, the watchWheels copy and the gate tests from the last review are fixed, and required CI has not run on this head because the pull request is a draft.
[Automated review]
While the dock follows the cover a pan leaves, each keyboard report now moves only the dock's translate, from the keyboard module's numbers, with no layout read or write. The layout takes the lift once, when the reader's scroll comes to rest: at a lifted finger or at the scroll's end, through the shared input stream. A composer show under a pinch zoom, whose pan is the reader's, now teaches nothing: its dock follows the cover instead.
A fling that pans only the visual viewport raises no document scroll end, so its followed lift waited for the next lifted finger. The keyboard module now passes on the visual viewport's own scroll end (subscribeViewportRest, one shared listener per window), and a report that finds the pan unchanged also counts as rest.
A report whose pan moved less than half a pixel counted as rest, so a slow drag laid out the lift every few reports. Now only a report that finds the pan exactly where it was, with no finger down, counts as rest: the exact repeat that ends a fling's momentum. The hook keeps the count of fingers down across its effect runs.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the spec:AST-099 text at this head: since the last approved revision, FR3 changed again (the dock now follows the pan on its translate, and the layout takes the lift when the scroll rests), Gap 16 now says a pinch-zoomed show teaches nothing, and FR9's docked clause, Gap 17 and a third Firefox for Android row remain unapproved, so an engineering owner records the decision, including whether a dock moved by script on each visual-viewport report is acceptable, by commenting /approve-spec <full head SHA>, which the author may do as an engineering owner.
Independent of that decision, these need fixing:
- The follow mode counts fingers down from its own
touchstart,touchendandtouchcancelsubscriptions (useChatKeyboard.ts:432-445) beside the input stream'sgetDownPresses, so add a touch-based down count to the input stream and read it here, or say in the comment whygetDownPresses, which drops a press at the scroll'spointercancel, cannot serve. - The Gap 17 hold is untested: switching off the layout-follows-pan check (
keyboardHeight.ts:842) leaves every keyboard case green while the dock's translate climbs to −323 px in the Safari slow-scroll case (useChatKeyboard.test.tsx:866-920), so assert there that the dock's translate stays empty. - AST-099 Platform support makes Firefox for Android emulator evidence at the exact head merge evidence, and the description gives no emulator result at this head for the three Firefox rows (
spec.md:444-446), including the translate follow this push changed.
The per-report layout commit and the pinch-zoom learning from the last review are fixed, and required CI has not run on this head because the pull request is a draft.
[Automated review]
The Safari slow-scroll case now asserts that the dock's translate stays empty, so removing the hold fails it. A keyboard hide resets the hook's finger count, so a lift the effect never heard cannot keep a later fling from resting. The comment says why the input stream's down presses cannot serve. FR3 says the layout takes the lift when the last finger lifts, and again when a fling rests.
FR3 now says plainly that the dock follows the pan on its compositor translate, with no layout work at each change of the visual viewport. It also says the layout takes the lift when the last finger lifts, and again when a fling comes to rest. The hook's comment matches.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the spec:AST-099 text at this head: since the last approved revision, FR3 changed again (the dock's follow is now stated as a compositor translate, and the layout takes the lift when the last finger lifts and again when a fling comes to rest), and FR9's docked clause, Gap 16, Gap 17 and the three Firefox for Android FR3–FR4 rows remain unapproved. An engineering owner records the decision, including whether a dock translate written by script on each visual-viewport report is acceptable, by commenting /approve-spec <full head SHA>; the author may do so as an engineering owner.
Independent of that decision, this needs fixing:
- AST-099 Platform support makes Firefox for Android emulator evidence at the exact head merge evidence, and the description gives no emulator result at this head for the three Firefox rows (
spec.md:446-448), including the translate follow.
The finger count's reason, the Gap 17 test and FR3's wording from the last review are fixed. Required CI has not run on this head because the pull request is a draft.
[Automated review]
|
/approve-spec a64d445 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: the spec:AST-099 text at this head carries an engineering owner's approval, so the direction is settled and one piece of merge evidence remains.
- AST-099's Platform support makes Firefox for Android emulator evidence at the exact head a merge gate, and the description gives no emulator result at this head for the three Firefox FR3–FR4 rows (
docs/specs/AST-099-mobile-keyboard/spec.md:446-448); please run them with animations at normal scale and removed and a slow-device profile, and state the head and each row's composer bottom against the visual viewport's bottom.
Non-blocking: consider amending architecture:react-component-runtime INV11 so it names the dock's pan follow, and naming Gap 16's 3 s cap in the changeset. The earlier findings on learning from another field's show, the pinch-zoomed show, the finger count's reason and the Gap 17 test are fixed. Required CI has not run on this head because the pull request is a draft.
[Automated review]
In the show that learned the browser leaves the dock covered, a pan to the dock before any input dropped the lift and forgot what the page learned, so the next show waited again. In Firefox for Android under Remove animations a reveal can land after the lift. Now that pan goes back, as at every later show, and the page keeps the cover. The dock watch puts it back itself: the module's own put-back measures from the opening report, before the lift moved the layout.
There was a problem hiding this comment.
Changes requested: the spec:AST-099 text at this head is unchanged since an engineering owner approved it, so the direction is settled, and one piece of merge evidence remains.
- AST-099's Platform support makes Firefox for Android emulator evidence at the exact head a merge gate, and the reported run is from the previous revision, before this push changed what Firefox's learning show does with animations removed (
keyboardHeight.ts:883-887); please rerun the three Firefox FR3–FR4 rows (docs/specs/AST-099-mobile-keyboard/spec.md:446-448) at the final head with animations at normal scale and removed and a slow-device profile, and state the head and each row's composer bottom against the visual viewport's bottom.
The late-reveal fix itself is covered by two new cases that fail on the previous revision, and the earlier findings on the gate tests, the focus owner, the finger count's reason and the Gap 17 test are fixed. Non-blocking: consider amending architecture:react-component-runtime INV11 to name the dock's pan follow, naming Gap 16's 3 s cap in the changeset, and the comment wording suggested inline. Required CI has not run on this head because the pull request is a draft.
[Automated review]
| | FR3–FR4 | ChatLayout's keyboard tests in jsdom and its interaction proofs in Chromium and WebKit | the fixed and the sticky dock with the keyboard shown and hidden; the message area's end padding; a transcript at its newest message; a page pinch-zoomed with the keyboard up; focus by tap, Tab, `focus()` and Done, under a finger and under a mouse | The dock left under the keyboard or lifted with none; the newest message unable to scroll above the composer; a transcript at its newest message left behind; the page panned to the composer's text under a finger; focus left in the text after Done, or the composer group focusable outside Done's park | | ||
| | FR3–FR4 | ChatLayout's keyboard tests in jsdom and its interaction proofs in Chromium and WebKit | the fixed and the sticky dock with the keyboard shown and hidden; the message area's end padding; a transcript at its newest message; a page pinch-zoomed with the keyboard up; focus by tap, Tab, `focus()` and Done, under a finger and under a mouse | The dock left under the keyboard or lifted with none; the newest message unable to scroll above the composer; a transcript at its newest message left behind; the page panned to the composer's text under a finger, outside FR3's document-scroll exception; focus left in the text after Done, or the composer group focusable outside Done's park | | ||
| | FR3–FR4 | Firefox for Android in the emulator, top-level document | ChatLayout whose `scrollRef` names the document, the page's first keyboard show: a tap on the composer, the keyboard shown | Once the wait ends (Gap 16), the composer does not rest on the keyboard: its bottom sits more than 0.5 CSS px below the visual viewport's bottom | | ||
| | FR3–FR4 | Firefox for Android in the emulator, top-level document | The same page's next keyboard show after one that left the composer covered: a tap on the composer, the keyboard shown | The composer does not rest on the keyboard at once: its bottom sits more than 0.5 CSS px below the visual viewport's bottom | |
There was a problem hiding this comment.
This push changes what this row measures with animations removed: the show that learns the cover now puts a late reveal pan back and keeps what it learned (keyboardHeight.ts:883-887). That is the case where the reported run, at the previous revision, left the second show covered 599 ms in one of three takes. Please rerun this row at the final head with animations at normal scale and removed and a slow-device profile, and state the composer bottom against the visual viewport's bottom.
| // Pans the visual viewport back to `top`, without scrolling the document. It | ||
| // scrolls `anchor` into view, or a probe fixed at `top` in the layout | ||
| // viewport, which the pan does not move. |
There was a problem hiding this comment.
Non-blocking (INV29): the second sentence narrates the two branches. The reason is the quirk. Suggested:
// A fixed probe sits in the layout viewport, which the pan does not move.
// Scrolling it into view pans the visual viewport back.
Review only this pull request's change: vjeux/astryx@0d75573...8e190c3
User impact
In Safari on iOS, tapping the composer of a ChatLayout whose
scrollRefis the document puts it under the status bar.Problem and solution fit
In a page that scrolls the document, Safari pans the visual viewport to the fixed dock when the keyboard opens, and keeps the pan. ChatLayout's lift adds to it. Firefox for Android pans at some shows only, so it needs that lift at the others.
Three narrower fixes failed on the Simulator. A lift less Safari's pan made Safari pan again at each change. Re-arming the put-back after ChatLayout's scroll left the pan in place. No lift with the tap claim kept left the composer under the keyboard: Safari then never panned.
Expected behavior and authority
spec:AST-099FR3 puts a docked composer on the keyboard; FR4 keeps focus from panning the page. FR3 and FR4 now name two exceptions: Safari on iOS, and the shows before one leaves the composer covered elsewhere without the VirtualKeyboard API. Three Firefox rows and Gaps 16 and 17 cover them. They need the owners' stamp at this head.Smallest restoration
Where
scrollRefnames the document's root or body, without the VirtualKeyboard API, shows leave focus to the browser until one finds the dock covered. The keyboard module's reveal-pan watch decides it, under a new opt-indockoption. The dock holds while the browser's reveal runs; a pan to the dock means no lift. A dock still covered after 0.7 s rises, and a clean show teaches the page to work as on main. After a touch or a wheel, the dock rises by the cover the pan leaves. Where the layout viewport follows the pan, as in Safari, it holds. Chrome and self-scroll mode are unchanged.Evidence
Reproduction before the change (iOS 26.2 Simulator, chore(sandbox): a Chat Layout page with a case for each chat check #10672's page): a tap puts the dock's top at −102 px in
long, −138 px incomposer.Result after the change: the dock's bottom meets the keyboard at 377 px.
Representative unchanged path:
docked, and every case with no keyboard (Testing).Regression test or other mutation-sensitive proof: new scripted-time cases fail on main, and removing any gate fails its case (Testing).
Captures, main first; the README maps each (https://github.com/vjeux/astryx/tree/f2732673f1409e230ae6ee02d6a3b4740adb0264):
Scope
Testing
useChatKeyboard.test.tsxandkeyboardHeight.test.tsscript time, touches and visual-viewport values:innerHeightgrows as the pan unwinds through a slow drag to the page's top: the lift stays 0, with no translate.focus()'s reveal and the put-back each have a case before and after a page learns the dock is covered. Removing any one gate fails its case.focus(). Each keyboard route rests the composer on the keyboard, and Done leaves no group. On main, a tap and Next put it off screen.check:knowledgeandcheck:changesetspass.long, 199 px incomposer.Release check
long, the dock's top reads 235 px on 33bc644 (before feat(ChatLayout): the composer rides the software keyboard #8176) and on main with feat(ChatLayout): the composer rides the software keyboard #8176 reverted. It reads −102 px on 695653f (feat(ChatLayout): the composer rides the software keyboard #8176) and on the release candidate c99ad4a.scrollRefis the document, in Safari on iOS.longcase predates this change (ChatLayout: Chrome on Android sometimes pans the visual viewport after the lift, floating the composer above the keyboard (longcase) #10713).Writing rubric results
spec:AST-099FR3, FR4, FR9's docked-container clause, the three Firefox rows, and Gaps 16 and 17: