Repository navigation
Conversation
vjeux
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
October 24, 2026 11:37
|
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
Contributor
PR Analysis Report
Modified ComponentsMultiSelector (@astryxdesign/core)
Component 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 | +15.2KB |
+2B |
| core/ChatLayout | +17.4KB |
+39B |
| core/DateInput | +14.0KB |
+4B |
| core/DateTimeInput | +13.8KB |
+3B |
| core/PowerSearch | +12.1KB |
+4B |
| core/TimeInput | +14.3KB |
+4B |
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 | 35.4KB | 35.4KB | +26B |
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 504 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 | View full report
vjeux
force-pushed
the
uikit/c08-keyboard-dismiss
branch
from
October 24, 2026 12:06
9bcc941 to
79f61b0
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an owner decision: no current spec covers ChatLayout's new keyboardDismissMode prop or its default-on 'interactive' dismissal. An engineering owner needs to provide or identify the governing spec, meaning whether ChatLayout ships the prop and which default it uses, and record it by commenting /approve-spec <full head SHA> on this pull request; the author can do this if they are an engineering owner. Independently of that decision, these changes are needed:
- The diff includes four open predecessor pull requests (#8045, #8067, #8074, #8176), about 27,000 authored lines, so please land them through their own reviews and rebase this branch down to its ChatLayout commit.
'onDrag'blurs the composer even when no software keyboard is shown, unlike'interactive', so please gate it on the keyboard being up and add a test.- The new draft
spec:AST-094needs its own owner ruling in the pull request that introduces it, and its FR7 citesspec:AST-090, which is not in the repository.
[Automated review]
| }; | ||
|
|
||
| // The browser took the touch to scroll: a drag began. | ||
| const onPointerCancel = (event: PointerEvent) => { |
There was a problem hiding this comment.
'onDrag' dismisses here without checking that a software keyboard is up, while 'interactive' returns early when guide.inset <= 0 (line 127). On a touch screen used with a hardware keyboard, a ChatLayout set to onDrag loses the composer's focus on every touch scroll of the transcript. Consider requiring guide.inset > 0 for onDrag too (at the touch start or here), and adding the "no keyboard" case to the onDrag tests.
| name: 'ChatLayout', | ||
| displayName: 'Chat Layout', | ||
| description: '完整聊天界面的布局外壳。消息在页面中自然流动，编写器固定在底部，带有毛玻璃效果。通过 density 属性控制密度。', | ||
| description: '完整聊天界面的布局外壳。消息在页面中自然流动，编写器固定在底部，带有毛玻璃效果。通过 density 属性控制密度。在手机上，编写器随软键盘同步上移并停在键盘上方，停留在最新消息的对话保持在最新消息；轻点编写器会聚焦它而不滚动页面。', |
There was a problem hiding this comment.
docsZh was not updated for this change: its description omits the drag-to-dismiss behavior that is now the default, and propDescriptions has no keyboardDismissMode entry, so Chinese readers see the English text for the new prop.
vjeux
force-pushed
the
uikit/c08-keyboard-dismiss
branch
from
October 24, 2026 16:35
79f61b0 to
187dadb
Compare
There was a problem hiding this comment.
Approval waits on an owner decision: no current spec covers ChatLayout's new keyboardDismissMode prop or its default-on 'interactive' dismissal. An engineering owner needs to provide or identify the governing spec, meaning whether ChatLayout ships the prop and which default it uses, and record it by commenting /approve-spec <full head SHA> on this pull request; the author may do so if they are an engineering owner. Independently of that decision, these changes are needed:
- The diff includes four open predecessor pull requests (#8045, #8067, #8074, #8176), about 27,000 authored lines, so please land them through their own reviews and rebase this branch down to its ChatLayout commit.
'onDrag'blurs the composer even when no software keyboard is shown, unlike'interactive', so please gate it on the keyboard being up and add a test for that case.- The new draft
spec:AST-094needs its own owner ruling in the pull request that introduces it, and its FR7 citesspec:AST-090, which is not in the repository. docsZhinChatLayout.doc.mjshas nokeyboardDismissModeentry and does not mention the new default.
[Automated review]
vjeux
force-pushed
the
uikit/c08-keyboard-dismiss
branch
from
October 24, 2026 19:02
187dadb to
edc7843
Compare
There was a problem hiding this comment.
Approval waits on an owner decision: no current spec covers ChatLayout's new keyboardDismissMode prop or its default-on 'interactive' dismissal. An engineering owner needs to provide or identify the governing spec, meaning whether ChatLayout ships the prop and which default it uses, and record it by commenting /approve-spec <full head SHA> on this pull request; the author may do so if they are an engineering owner. Independently of that decision, these changes are needed:
- The diff includes six open predecessor pull requests (#8045, #8067, #8074, #8176, #8258, #8287), about 29,800 authored lines, so please land them through their own reviews and rebase this branch down to its ChatLayout commit.
'onDrag'blurs the composer even when no software keyboard is shown, unlike'interactive', so please gate it on the keyboard being up and add a test for that case.- The draft
spec:AST-094needs its own owner ruling in the pull request that introduces it: its FR7 citesspec:AST-090, which is not in the repository, and its FR4 rules out the theme-tunable touch motion that #8258 adds. docsZhinChatLayout.doc.mjshas nokeyboardDismissModeentry and does not mention the new default.
[Automated review]
Merged
8 tasks done
Contributor
|
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. |
Contributor
Author
|
Agreed: one implementation.
|
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 25, 2026
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 25, 2026
vjeux
force-pushed
the
uikit/c08-keyboard-dismiss
branch
from
October 25, 2026 01:51
edc7843 to
cd49b1b
Compare
Contributor
Author
github-actions
Bot
added
the
needs:design-review
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
October 25, 2026 01:52
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
A press on the transcript decided at its release whether it lifted past the keyboard's line, however little it moved, so a tap that landed just above the composer and drifted a few pixels onto it dismissed the keyboard. In UIKit a scroll view's drag begins only once its pan travels its 10 px hysteresis, so that tap never drags. A press is now followed as a drag only once it travels UIKit's pan hysteresis on either axis; until then its lift decides nothing. New cases for a finger and a mouse drifting 8 px across the line fail before this change, and a 10 px drag still dismisses.
BottomSheet.test.tsx's imports from both sides are kept; the search goldens are regenerated after the remaining merges.
… guide The keyboard layout guide (this pull request) reads where focus moves to tell a keyboard that comes up from one that stays: meta#8559 removed the store's focus reports because nothing there read them, so they return here with their first reader. One pair of capture listeners on the document, attached while anything subscribes, reports each focusin and focusout with the current height; BottomSheet's reveal still acts on viewport reports only.
Conflicts were in this stack's own keyboard files: the browser-support sentence keeps ChatLayout's composer and takes meta#8559's page wording, BottomSheet's test imports follow the sheetKeyboard split, and the keyboard notes keep both the sheet's and ChatLayout's Chrome 124 findings. The search goldens are regenerated after the merge.
…op-level page The composer's tap route, its scroll-free focus and Done's park attach only where the keyboard is measurable, which excludes a page inside an iframe (spec:AST-099 FR7); there the browser's own focus applies. The docs and changeset now say so.
ChatLayout.doc.mjs conflicted where both pull requests edit the same description lines: meta#8176's touch-screen and top-level-page focus wording is kept and this pull request's drag-to-dismiss sentences are added to it in English, Chinese and the dense line. The search goldens pass unchanged.
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: component:ChatLayout FR10 and DEC-1 add drag-to-dismiss to a draft record as built-in behavior with no caller control, and FR10 now attaches for every pointer and no longer relies on the INV16 carve-out for touch-only behavior that the earlier owner decision asked for. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or names the FR10 text they want. Independently of that decision:
- Land #8176, the work under it and #8671 through their own reviews, then rebase so this pull request carries only its dismissal change; today it changes about 39,900 lines around a change of roughly 1,560.
- Re-record the device checks at the final head on a real iPhone in Safari and real Android devices in Chrome and Firefox, since the changeset names those browsers and the current recordings come from an earlier revision.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] ChatLayout: dragging the conversation down to the composer and lifting there now dismisses the software keyboard, as in Messages, wherever ChatLayout keeps its composer on the keyboard (Safari on iOS, Firefox and Chrome for Android); a release moving back up keeps it. A finger drags, and so does a pen the browser reports as touches; in a desktop browser a mouse press, or a press of a pen reported as a pointer, already takes focus off the composer, so the keyboard leaves at the press. Where a page sets `interactive-widget=resizes-content`, Chrome resizes the page for the keyboard, ChatLayout measures no keyboard, and no drag dismisses it. A web page cannot move the system keyboard, so the keyboard leaves on its own at the lift instead of following the finger. Drags that start in the composer never dismiss it, and dismissing moves focus nowhere. Drag-to-dismiss has no caller control. Typing with a keyboard is unchanged. |
There was a problem hiding this comment.
This names Safari on iOS, Firefox and Chrome for Android. spec:AST-013 FR10 asks that evidence for a named browser come from that browser, and spec:AST-094's Platform support asks for real-device evidence for iOS Safari and Android Chrome claims, while the description's recordings come from an earlier revision. Please re-record the checks at the final head on a real iPhone in Safari and real Android devices in Chrome and Firefox, and link them in the description.
| | FR7 | `density` MUST reflect on the root as the `chat-layout` target's `data-density`, and MUST select the dock, message-area, dock-inner, and blur-layer geometry together. | shipped implementation; pinned tests; family doc `visualProps` | verify | | ||
| | FR8 | The layout MUST publish its scroll container and message content refs through `ChatLayoutContext` so the message list can register the observed content box. | shipped implementation | verify | | ||
| | FR9 | Accepted `ref`, DOM/data/ARIA props, `className`, `style`, and `xstyle` MUST reach and compose on the layout root. | `architecture:public-component-api/INV5–INV6, INV8` | settled | | ||
| | FR10 | While the keyboard module measures a software keyboard (`spec:AST-099` FR2; none where a page sets `interactive-widget=resizes-content`, FR8), a drag down the transcript by a finger, or by a pen the browser reports as touches, MUST dismiss the keyboard, as in Messages, when it lifts at or past the composer's top, with no projection (`spec:AST-094` FR36; UIKit's interactive dismissal, measured on the iOS 26.2 simulator, in `keyboard/keyboardDismiss.ts`), and a release moving back up MUST keep it (FR37). The dismissal attaches for every pointer (`spec:AST-094` FR19, FR43); in a desktop browser a mouse press, or a press of a pen the browser reports as a pointer, takes focus off the composer itself, so the keyboard leaves at the press. Dismissing only blurs the focused field; focus moves nowhere else. Drag-to-dismiss is part of ChatLayout's behavior, with no caller control: DEC-1 rejects an off-switch prop. Under a fine pointer it listens on the layout's own element, and on the document only while a button is down (`spec:AST-094` FR24); the keyboard is unchanged. | `spec:AST-094` DEC-1, FR19, FR24, FR36–FR37, FR43; `spec:AST-099`; DEC-1 | settled | |
There was a problem hiding this comment.
The code now follows a press as a drag only after 10 px of travel on either axis (keyboardDismiss.ts:134-142), so a tap that drifts onto the composer keeps the keyboard. FR10 does not say so; consider stating it here, with spec:AST-094 FR17 as its basis, so the row the owners approve protects that case.
vjeux
marked this pull request as draft
October 29, 2026 22:34
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review only this PR's change: vjeux/astryx@7373545...04727e8 (plus #8671's test file, which this stacks on), and the tap fix vjeux@927c9bb1fc.
Stacked on #8176 and #8671.
Why
In Messages on iOS, dragging the conversation down to the text field puts the keyboard away. ChatLayout kept it up, covering about 40% of an iPhone's screen, so the only way to read more of the conversation was to tap outside the field.
What led to it
Part of bringing Astryx's touch behavior to parity with iOS's UIKit, text entry first. #8176 puts ChatLayout's composer on top of the keyboard, which made the missing gesture stand out. The engineering owner decided it is built-in behavior with no prop (
component:ChatLayoutDEC-1), andspec:AST-094FR36–FR37 say a drag decides once, when it lifts.Options and tradeoffs
Why this choice
Decide once, when the drag lifts: at or past the top of the composer the keyboard goes, moving back up first keeps it, a drag that starts in the text field (moving the caret) never dismisses, and a press counts as a drag only after it travels UIKit's 10 px pan threshold, so a tap that drifts onto the composer keeps the keyboard. The keyboard stays up during the drag. On desktop a mouse press already moves focus off the field, and wheel scrolling doesn't dismiss, which stays an open question.
How to check it
Evidence
The recordings come from an earlier version with the same finger handling; they'll be redone after the rebase. Unit and browser tests (Chromium, WebKit) replay finger, mouse and pen drags, including a touch-screen laptop. Known failures not from this change: slow first loads from main (#8353, #8471), and pr-rtl's new touch-target check, whose test file reached main after this branch's last merge.
Size (INV6, gzip, main → head): ChatLayout JS 50,010 → 67,822 B, CSS 10,173 → 10,212 B.
Writing rubric results
component:ChatLayout(FR10 and its FR43 scope, intro, summary rows, verification): R1, R2, R3, R5 and R6 none; R4: DEC-1's one rejected line.