Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request is on hold for an owner decision: the Android keyboard ride in ChatLayout, its focus-scoped navigator.virtualKeyboard.overlaysContent toggle, the theme-retunable touch motion roles (including androidKeyboard.*) and the draft spec:AST-094 requirements have no approved spec authority, and the roles also contradict the current theme-tokens record and the draft's own FR4. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner; the decision needs to settle whether a component may toggle overlaysContent and whether touch motion is theme-retunable. These changes are needed either way:
- Rebase once #8045, #8067, #8074, #8176 and #8258 land, so this pull request contains only the Android commit.
- Leave the
platformadaptation condition out of this pull request; it changes the theme authoring contract's fixed condition vocabulary. - State a removal condition for the
-webkit-touch-calloutkeyboard profile or key it on a capability (AST-013 FR6), and add Chrome evidence that the dock comes down when the composer blurs with the keyboard up. - Add
## Writing rubric resultsfor the new spec record.
[Automated review]
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
platform adds a condition axis to theme adaptations, while the current theme authoring contract (Change coupling) says "An adaptation change preserves the fixed condition vocabulary". The Android keyboard path never uses it; it arrives from #8258, where the contract needs an owner amendment first, so please leave it out of this pull request.
There was a problem hiding this comment.
Gone: this branch no longer touches the theme adaptations. After the restack onto #8176, its diff is Chrome's timing model only.
| const css = ( | ||
| win as Window & {CSS?: {supports?: (p: string, v: string) => boolean}} | ||
| ).CSS; | ||
| return css?.supports?.('-webkit-touch-callout', 'none') === true |
There was a problem hiding this comment.
This picks the keyboard model from -webkit-touch-callout support, a property the guide never uses, so every engine other than iOS WebKit, Firefox for Android included, gets Chrome's start-of-slide model. AST-013 FR6 allows engine inference only as a narrow workaround for a reproduced defect with a regression fixture and a review or removal condition. Please state that condition here and in the README, or key the Chrome path on the capability it relies on.
There was a problem hiding this comment.
Changed: the guide picks Chrome's schedule where the VirtualKeyboard API exists (keyboardProfileOf, keyboardLayoutGuide.ts:58), the API Chrome reports Gboard's slides through, and the WebKit schedule everywhere else, Firefox for Android included. No property the guide never uses decides it, and no user agent. Both schedules' values are sourced in motion/uikit/keyboard.ts and motion/android/keyboard.ts.
| }; | ||
| const onOverlayFocusOut = (event: FocusEvent) => { | ||
| if (!inOverlayField(event.relatedTarget)) { | ||
| release(); |
There was a problem hiding this comment.
When the composer blurs with the keyboard up (the send button, a tap on the transcript), this releases overlaysContent before Chrome reports the hide. onGeometry then ignores that geometrychange, and blurred() predicts nothing on Chrome, so the dock comes down only if Chrome resizes the visual viewport for a keyboard it laid out as an overlay. Could you add a Chrome recording or test of this path, or hold the overlay until the hide is reported?
There was a problem hiding this comment.
Gone: the guide no longer sets overlaysContent. Chrome is measured from its visual viewport (spec:AST-099 DEC-4), and a blur predicts Chrome's hide 16 ms later on Gboard's hide curve. ChatLayoutKeyboard.interaction.browser.spec.ts covers the ride down from the blur (its start at blur + 16 ms ± 8, the curve within 0.5 px), and the emulator take at this head rests the composer at the bottom 128 and 140 ms after the hide's report.
2c7ae0d to
4be76ef
Compare
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision: the Android keyboard ride in ChatLayout, its focus-scoped navigator.virtualKeyboard.overlaysContent toggle, the theme-retunable touch motion roles (including androidKeyboard.*) and the requirements of the new draft spec:AST-094 have no approved spec authority, and the roles also contradict the current theme-tokens record and the draft's own FR4. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner; the decision needs to settle whether a component may toggle overlaysContent, whether touch motion is theme-retunable, and whether AST-094's requirements stand. These changes are needed either way:
- Rebase once #8045, #8067, #8074, #8176 and #8258 land, so this pull request contains only the Android commit (the range is about 29,500 authored lines today).
- Leave the
platformadaptation condition out of this pull request; it changes the theme authoring contract's fixed condition vocabulary. - State a review or removal condition for the
-webkit-touch-calloutkeyboard model or key it on a capability (AST-013 FR6), and add Chrome evidence that the dock comes down when the composer blurs with the keyboard up. - Add
## Writing rubric resultsfor the new record, and correct its FR7 citation of AST-090, which does not exist in the repository.
[Automated review]
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/BottomSheet | +2.4KB |
+2B |
| core/ChatLayout | +14.1KB |
+39B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.2KB | 34.2KB | +25B |
Accessibility Audit
Status: 2 accessibility violation(s) found — 2 serious.
ChatComposer - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/19 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
ChatComposerInput - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/18 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
Visual Regression
55 of 502 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 440,175 |
| Chat | Full AI Chat | probe | dark | 438,105 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Chat | Empty State | probe | light | 122,185 |
| Chat | Empty State | probe | dark | 120,076 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
|
Posted by Cindy's agent on her behalf. Decision on the Android question in #8304: smooth keyboard riding on Chrome for Android is wanted, ahead of the one-step relayout. |
|
Agreed: this PR becomes the smooth Android ride on the shared keyboard module, rebuilt on #8176, and stays held until #8304 merges. The measured mechanism question is on #8304: #8304 (comment) |
4be76ef to
44fd946
Compare
vjeux
changed the title
github-actions
Bot
added
the
needs:design-review
It pointed a component with no interaction to infer from at useMotionPresentation(), which does not exist, and at theme tokens under a mouse, which no theme sets. Such a component reads useTouchPresentation() like any other behaviour: UIKit timings under a finger, its own motion under a mouse.
There was a problem hiding this comment.
Changes are needed before this can be approved: the Chrome-on-Android ride itself follows the current spec:AST-099 FR8 and DEC-4, but this pull request still contains the content of sixteen open predecessor pull requests (about 31,600 authored lines), which their own reviews need to settle first. Please rebase once #8176 and the pull requests beneath it have landed, so that this pull request carries only the 18 files of the Chrome work. While there, drop "under a finger" from the Chrome section of packages/core/src/keyboard/README.md, since the keyboard module measures Chrome under any pointer.
[Automated review]
| report and Gboard's top read from every recorded frame (`../motion/android/ | ||
| keyboard.ts` has the numbers and their sources). The guide reads these reports | ||
| in a browser with the VirtualKeyboard API (`keyboardProfileOf`), and the | ||
| keyboard module measures them from the visual viewport under a finger |
There was a problem hiding this comment.
This says the keyboard module measures Chrome "under a finger", but isKeyboardMeasurable has no pointer condition (useMobileKeyboard.ts:97-99), line 25 of this README says the decision is never from the pointer, and the updated test measures Chrome with (pointer: coarse) false (useMobileKeyboard.test.ts:238-252). Could you drop "under a finger" here?
There was a problem hiding this comment.
Fixed in 099d9de: the Chrome section says the keyboard module measures the visual viewport as in every browser, with no pointer condition. The Chrome interaction tests' setup comment said the same thing; it now says they run under a finger because Chrome on Android does, where the composer's focus routes are the touch ones.
|
|
||
| // A keyboard height the guide had no ride for (a resize, a rotation) | ||
| // reaches the layout directly. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
This Effect sets the lift store through commit(), which measures the root against the layout viewport (lines 257-288), but its comment names neither that measurement nor an external system (architecture:react-component-runtime INV3). Could the comment name the measurement, or could the commit run from the keyboard module's subscription? It arrives with #8176, so the fix may belong there.
There was a problem hiding this comment.
Fixed in #8176's bf7859a, merged here: the comment names what the Effect synchronizes, the keyboard module's height (the visual viewport, through useKeyboardHeight), and the measurement commit makes (the root against the layout viewport, into the lift store). It also says why a replay is safe: it acquires nothing, sets the lift only when that changes by half a pixel or more, and never during a ride.
architecture:react-component-runtime INV3 asks an Effect to name the external system it synchronizes. The Effect that commits a keyboard height the guide had no ride for said only that the height reaches the layout. Its comment now names the keyboard module's height (the visual viewport, through useKeyboardHeight) and the measurement `commit` makes (the root against the layout viewport, into the lift store), and why a replay is safe: it acquires nothing, sets the lift only when that changes by half a pixel or more, and never during a ride.
… Chrome section now says The keyboard README's Chrome section said the keyboard module measures Chrome's reports from the visual viewport "under a finger", but `isKeyboardMeasurable` has no pointer condition (meta#8176, spec:AST-099 DEC-4) and the README's own "Where it measures" says the decision is never from the pointer. The section says the visual viewport is read as in every browser. The Chrome interaction tests' setup comment said the same; it says they run under a finger because Chrome on Android does, where the composer's focus routes are the touch ones, and that the module measures under any pointer.
…to the motion-role animator # Conflicts: # packages/core/src/motion/README.md # packages/core/src/motion/roles.test.ts # packages/core/src/motion/uikit/timing.ts # packages/core/src/motion/uikit/uikit.test.ts
meta#8076's head folds main f69c31c (the scroll table's note), pins the press release curve by value, drops the `hooks/isRtlElement` resolver entry, and carries the maintainer's meta#8073 touch-action gate. No conflicts. `timing.ts` checked against meta#8484: the file is meta#8076's with exactly meta#8484's change (the `keyboardNotification` group, its binding and the `keyboardNotificationMs` shorthand), nothing else.
…tays internal (spec:AST-094 DEC-11) resolveMotion read each role's --motion-* variable from the page, which let a page's or theme's style retune touch motion that DEC-11 keeps internal until the public motion-token set is decided. It now returns the role's table value, reading no style; every motion still passes through it, so a public role can reach the animator later in one place. With no variable read, a role value that cannot settle can no longer arrive either. The unit tests and the browser proof now show a scope's variable changing nothing, and the README, useAnimator and coast docs no longer say a theme sets the roles.
…plays its table value) into the experimental touch exports Four files both sides changed: coast.ts takes the animator's wording (the edge spring plays its table value); useAnimator.ts takes its registration line and keeps this side's note that a component may play its own measured spring; roles.test.ts takes the menu spring's motionRoleOf check beside this side's keyboard checks; resolveMotion.test.ts takes the animator's tests and keeps this side's coast test (scroll.bounce is no role).
meta#8076's head is a fast-forward of 66edc03 that folds main 61480b5 with meta#8257 (a motion role plays its table value). No conflicts. `timing.ts` checked against meta#8484: the file is meta#8076's with exactly meta#8484's change (the `keyboardNotification` group, its binding and the `keyboardNotificationMs` shorthand), nothing else.
…reader architecture:react-component-runtime INV11 allows the keyboard module at most one layout read per keyboard change. `getMobileKeyboardHeight` measured again on every call, and the layout guide, each component's `useKeyboardHeight` and ChatLayout's commit all call it after the module has already measured the report: one show read the visual viewport's height 6 times in Full AI Chat in Chromium, 13 times in the module's test with the guide, a listener and two hooks. While the module listens, `getMobileKeyboardHeight` returns the height it measured at the last report. Every change of the visual viewport or the window is a report, so a reader between two reports still sees the current height; with nothing listening yet, it measures. Tests: the module's test counts one read for a report with the guide, a listener and two hooks reading the height (red before: 13), and an interaction test counts one read for a show in Full AI Chat (red on the previous build: 6).
…is focused without the reveal spec:AST-099 FR4 says every route into the composer's text, a tap included, focuses it without scrolling the page. The tap route claimed only contenteditable text: a `<textarea>` or `<input>` passed as ChatComposer's input (its documented plain textarea) was left to the browser, and with nothing focused no blur reaches the focus takeover, so Safari revealed the field by scrolling the document. The tap route now claims an unfocused text control in the composer that brings up a keyboard, and delivers it as it delivers contenteditable text: focused with `preventScroll`, with the caret where the standard `caretPositionFromPoint` maps the point inside the control, or after its text where the browser maps none. A focused control keeps its own taps (the browser moves the caret); a checkbox or an `inputmode="none"` field is never claimed. Tests (useChatKeyboard.test.tsx): ChatLayout with a textarea composer, under a coarse pointer, cancels the tap's default and focuses the textarea with `preventScroll` (red before: the browser's own focus); the claim takes an unfocused text control and leaves a focused one, a checkbox and an `inputmode="none"` field; the caret lands at the mapped offset, or after the text.
…d Chrome it moves once reported spec:AST-099 FR8 asks a ride within one frame of the keyboard on the measured keyboards, with Android emulator evidence against Gboard; Firefox for Android has FR2–FR5 only, and reports the keyboard once it is up. This branch plays UIKit's keyboard spring from the browser's report there, and times no ride to Gboard. The docs said the composer rides in step in Firefox for Android and rides the keyboard in Chrome on Android. ChatLayout's docs (English, Chinese, dense), the browser-support guide and the changeset now say: in Safari on iOS the composer rides on top of the keyboard in step with it; in Firefox for Android and Chrome on Android it moves above the keyboard once the browser reports it; under Chrome's `interactive-widget=resizes-content` it stays above it without moving.
Her af953bd merges meta#8076's bd9246d (main 2af4d30, with meta#8229's Popover presentation) into this branch, with her resolution of BottomSheetPanel.tsx against meta#8229. No conflicts with this branch's commits since 2d520a6: the result is her head plus those commits (the keyboard-height Effect's INV3 comment, one layout read per keyboard change, the text control tap route, and the Safari/Firefox/Chrome docs). `timing.ts` checked: meta#8076's with exactly meta#8484's change.
meta#8176's local head reads the layout once per keyboard change, focuses a text control composer tapped with nothing focused without the reveal, says the composer moves above the keyboard once reported in Firefox and Chrome for Android, and merges the maintainer's af953bd (meta#8076's bd9246d, main 2af4d30 with meta#8229). No conflicts; `timing.ts` is meta#8176's.
…ides in step meta#8176 says the composer moves above the keyboard once Chrome reports it, since it times no ride to Gboard (spec:AST-099 FR8 asks a ride within one frame of the keyboard, with Android emulator evidence). This branch times the ride to Gboard's measured slides, with that evidence: ChatLayout's docs (English, Chinese, dense) and the browser-support guide say the composer rides Gboard's slides in step in Chrome on Android, while a Tall BottomSheet makes its room once Chrome reports the keyboard. Firefox for Android keeps meta#8176's wording: it moves once Firefox reports the keyboard.
|
On the scope item: this branch lands after #8176 and the touch foundation under it; once those land, it carries only the Chrome work's 18 files. |
There was a problem hiding this comment.
Changes are needed before this can be approved: the Chrome-on-Android ride itself follows the current spec:AST-099 FR8 and DEC-4, but the pull request still contains the content of fifteen open predecessor pull requests (about 29,900 authored lines), which their own reviews need to settle. Please rebase onto main once #8176 and the pull requests beneath it have landed, so that this pull request carries only the 20 files of the Chrome work.
[Automated review]
… rides Gboard spec:AST-099 FR8 asks Core's keyboard-aware surfaces to ride the keyboard in Chrome on Android within one frame of it on the measured keyboards, and DEC-2 rules out a one-step relayout; on its own this branch played UIKit's keyboard spring from Chrome's report. meta#8287's Chrome work comes in here, so the change conforms on its own: Gboard's measured slides (`motion/android/keyboard.ts`), the keyboard model's Chrome profile, the guide's schedule picked by the VirtualKeyboard capability, `rideKeyboard`, and their tests. meta#8287 closes as folded in. Conflicts, resolved: - The browser-support guide takes meta#8287's Chrome sentence (the composer rides the keyboard as Gboard slides it), with this branch's correction that a Tall BottomSheet adds its scroll room once Chrome reports the keyboard, and that under `resizes-content` nothing changes. - The changesets: ChatLayout's says the dock rides in step in Safari on iOS and in Chrome on Android (Gboard's measured slides) and moves once reported in Firefox for Android; meta#8287's own changeset folds into it and is removed. The browser-support changeset says the same for Chrome.
meta#8076's head folds main through meta#8055's landing (the compositor animator) and is a fast-forward of the 0d1a829 meta#8287 brought in. It merges cleanly with main dbcfc30, which this branch's previous base no longer did. No conflicts; `timing.ts` is meta#8076's with exactly meta#8484's change.
|
Folded into #8176: spec:AST-099 FR8 and DEC-2 can't be met in Chrome without this Gboard timing, so the two land as one there. |
meta#8176 now carries meta#8287's Chrome-on-Android timing (Gboard's measured slides), the keyboard module's shared reveal-pan watch (a browser's own pan to the focused composer put back, which Chrome 124 animated by the keyboard's whole height), the scrollbar thickness measured once, and meta#8076's 2267ee5 (main through meta#8055's landing). Conflicts, resolved as the union of both sides: - keyboard/index.ts and useMobileKeyboard.ts headers and exports: rideKeyboard, keyboardProfileOf and watchRevealPan beside observeKeyboardDismiss and blurAsDismissal; the README's file table lists rideKeyboard.ts and keyboardDismiss.ts. - useChatKeyboard.ts: the header names both; the reveal-pan watch uses this branch's `isTouch` (one touch-presentation read). - ChatLayout.doc.mjs (EN, ZH, dense): meta#8176's ride sentence (in step in Safari and in Chrome on Gboard, once reported in Firefox) with this branch's drag sentence, and no drag dismissing under `resizes-content`. - ChatLayoutKeyboard spec: `openChat` takes both `android` and `args`. `timing.ts` is meta#8176's, with meta#8484's keyboard export.
Depends on #8176; review the commits after its merge.
User need
In Chrome on Android, Gboard slides over ChatLayout's composer, which jumps once Chrome reports the keyboard. spec:AST-099 FR8 asks core surfaces to ride the slide.
Evidence
Chromium 158 + Gboard, Android 15 emulator, 540×1200; backdrop blur off for capture (blur on). The emulator drops Chrome's page frames for 50–150 ms around each report (#8176 too), so the ride is judged on the page's clock: composer bottom per animation frame against Gboard's top per recorded frame.
interactive-widget=resizes-content: Chrome resizes the page; no ridePer frame (2 takes each): under reduced motion the dock held two positions, fading out then in; under
resizes-contentFR2 read 0 and the dock never translated, while Chrome laid it out above the keyboard.Problem and solution fit
resizes-contentFR2 reads 0 and nothing rides.cubic-bezier(0.1, 0, 0, 1)over 267 ms (20 shows, 2.4 px rms).cubic-bezier(0.3, 0.1, 0.2, 0.9)over 167 ms (21 hides, 7.7 px rms); Back from its report, dated back 33 ms.keyboard.spring(feat(ChatLayout): the composer rides the software keyboard #8176); a ride plays the curve its transition publishes (INV11).Current specification
spec:AST-099FR6, FR8 and DEC-4: the height comes from the visual viewport in every browser; underinteractive-widget=resizes-contentFR2 reads 0 and a field does not ride; Astryx never overrides the app's viewport setting.Public delta and caller burden
No public API change; iOS and desktop are unchanged. The browser-support guide covers Chrome.
Design and alternatives
Rejected: landing at the report (DEC-2's one step) and
resizes-contentwith a FLIP cover. No physical phone was used (DEC-3 gates release).Scope
Testing
ChatLayoutKeyboard,BottomSheet; WebKit withBottomSheetKeyboardInset): Chromium 22 passed, 5 skipped; WebKit 17 passed; BottomSheet and ChatLayout a11y 51 passed. Under a mouse (desktop Chromium, built storybook), the composer focuses on a click, Enter sends and the wheel scrolls. Chrome on Android inChatLayoutKeyboard.interaction.browser.spec.ts: the dock rides up on Gboard's show curve from the report, dated back 25 ms (start within 2 ms, curve within 0.5 px), and the layout takes the lift at rest; it rides down on the hide curve from the blur (start = blur + 16 ms ± 8, curve within 0.5 px); under Reduce Motion every frame is at the old or the new place.keyboardChromium.test.ts, where underresizes-contentthe guide rides nothing through a show, a hide and a second show (it fails if the layout height is cached);rideKeyboard.test.ts, the published spring (red before); the model's Chrome profile; the table's sources and that it registers no motion role; the module measuring Chrome under a finger but not under a mouse.pnpm buildand the rootpnpm typecheckpass.CI=true pnpm lint, prettier,check:sync,check:changesets, motion roles check,sync-exports --check,check:knowledge, the i18n catalog, the CLI goldens.resizes-content(2 takes), per frame, plus the ride re-taken at this head beside feat(ChatLayout): the composer rides the software keyboard #8176's: the composer rests on Gboard 218 and 244 ms after the show's report (feat(ChatLayout): the composer rides the software keyboard #8176's spring: 320 ms) and back at the bottom 128 and 140 ms after the hide's (feat(ChatLayout): the composer rides the software keyboard #8176: 418 ms).