Repository navigation
Conversation
…r along the flat tree, and a claimed touch finds its pointer finger wherever fingers come from pointer events) into meta#8293
…starts; useLongPress on each axis; meta#8336: the host compares a finger with a scroller along the flat tree, and a claimed touch finds its pointer finger wherever fingers come from pointer events) into the every-pointer host
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
needs:design-review
…nger's (spec:AST-094 FR21, FR43) A pen on an iPad sends its pointer events and then its stylus touches; the host follows the pen through its pointer events and leaves the stylus touches alone. So when a recognizer claimed a pen's tap, its stylus touchend went uncancelled, and Safari synthesized the mousedown that focuses natively and the click. FR21 cancels the touchend of a touch a gesture takes, and FR43 makes a pen's gestures as a finger's. Fails until the host cancels it; a second case keeps an unclaimed pen's lift the browser's.
…ger's (spec:AST-094 FR21, FR43) The host remembers whether a gesture took a pen's lift (its pointerup's dispositions, where the host reads Touch Events) until the pen's stylus touchend, which an iPad sends after the pointer events, and cancels that touchend then, so Safari synthesizes no compatibility mousedown, with its native focus, and no click. A stylus touchend whose pen is still down and claimed is cancelled too; a new pen press forgets a lift that never got its touchend, as on a pen that sends no Touch Events.
…inger's) into the wheel input
… the presented start A zoom stage already drives its photo with an animator (pinch, pan, settle), and the zoom's grow is a rect morph on the same element: two animators would overwrite each other's channels. createRectMorph takes that animator in `animator` and plays on it; dispose leaves it to its owner. The morph's presentedRect then reads the stage's own transform, so a dismissal from a zoomed photo starts from the shown rect. A present or a dismissal also takes the animator's `start`: with 'presented', the element holds its first values until two frames have presented it, so a surface that appears late (a dialog's first frame in WebKit) plays the whole curve. Both new cases fail before this change and pass after.
…cel) into the FR43 harness helper
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsMultiSelector (@astryxdesign/core) · View in Storybook
Popover (@astryxdesign/core) · View in Storybook
MobilePowerSearch (@astryxdesign/lab) · View in Storybook
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 | +37.1KB |
+98B |
| core/DateInput | +37.0KB |
+85B |
| core/DateTimeInput | +36.8KB |
+89B |
| core/PowerSearch | +36.8KB |
+101B |
| core/TimeInput | +37.1KB |
+92B |
| lab/MobilePowerSearch | +36.9KB |
0B |
| lab/MobileTokenizer | +37.0KB |
0B |
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 |
| @astryxdesign/lab | 4.6KB | 1.9KB | 1.9KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.6KB | 34.7KB | +94B |
| @astryxdesign/lab | 11.1KB | 11.1KB | 0B |
Accessibility Audit
Status: No accessibility violations detected.
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 | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| 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 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| 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
![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 a spec decision: the BottomSheet wheel and trackpad pull relies on spec:AST-094 FR43, which is only proposed in #8646, while FR19 on main still keeps gesture recognizers off a fine primary pointer and spec:AST-017 FR59 asks for an owner decision before touch work adds new mouse or trackpad behaviour. An engineering owner needs to land FR43 or an equivalent amendment on main, or identify the existing claim that governs this addition, and records that decision by commenting /approve-spec <full head SHA> on this pull request; the author may do so if they are an engineering owner. Separately, the diff against main is 234 files and 47,300 changed lines because it carries the unmerged #8386 and #8676 stack, so please merge those through their own reviews first, or rebase this branch, so that it contains only its own four files.
[Automated review]
meta-cla
Bot
added
the
CLA Signed
…only moves Under Reduce Motion on a touch device, the morph treats a path with no opacity of its own as an entrance on a present and an exit on a dismissal, so the cross-fade lands it at rest (design:reduced-motion). An element that stays visible and only moves, such as a sub-menu's parent card, then cut out and faded in on present, and faded out and popped back on dismissal, with no way for its caller to say so: the animator's crossfadeAs names only an entrance or an exit. A present or a dismissal now takes crossfadeAs: 'move', which passes no cross-fade direction, so the animator plays its own dip: out, the travel lands, back in. The new case does not compile before this change (the option had no value for a move; an untyped 'move' reached the animator only by accident) and passes after.
…come from pointer events; a press's selection lock gets what it landed on Where the host reads fingers from pointer events, onTouchStart returned before its second-finger check and onPointerDown never ran it, so an unclaimed one-finger pan could still claim after a second finger landed, and with claimed touches' moves now cancelled in that mode, it would hold the fingers of a pinch (spec:AST-094 FR10). And onPointerDown handed the selection lock the event's retargeted target, the shadow host, so a pen hold on a control inside an open shadow root left that control out of the lock, selectable in iOS WebKit. Both fail until the host fixes them.
…s come from; a press's selection lock gets what it landed on The second-finger check (an unclaimed pan limited to one finger fails, so the two fingers stay the page's pinch, spec:AST-094 FR10) now runs before onTouchStart's pointer-mode return, and in onPointerDown as a touch pointer lands while another finger is down, wherever it lands. And onPointerDown hands lockSelection the element the press landed on, so a control inside an open shadow root joins the lock.
…handlers type UseLongPressHandlers.start takes a point only again, as released: the per-press tolerance useMenuPress's trigger chooses as its press starts lives on the internal useLongPressDetector and LongPressDetectorHandlers, which the hooks entry does not export. useLongPress returns the same detector, typed as before.
… animations wrap instead of turning
astracat found that the control self-test's back-and-forth animation turns at 1,600 ms. When a loaded runner starts the cast late enough that the block holds the turnaround, two cast frames straddling it evenly read one position, and the control verdict fails the correct compositor animation: "moved on 6 of the 7 frames the control moved". Both self-tests' compositor animations and their timed rAF loops now wrap from 560 px back to 0, a change on every frame. The pooled case's counted mutants still bounce: they are negatives, and a turn changes their position by a full step.
BusyProof.stepPx's note now describes the step it reads from the judged part's changes, and the file header names pooledBusyProof.
Red first: with the setup stretched so the block holds the turnaround, the previous test fails 7 of 12 Chromium runs ("moved on 5 of the 6" and "6 of the 7 frames the control moved"), and this one passes 12 of 12.
…rol inside an open dialog, leaving its backdrop on the compositor
C17 found that in WebKit the control's top-layer popover, shown above an open modal dialog, takes the dialog's `::backdrop` animation off the compositor. BottomSheet's dim held its value for 155 to 225 ms into the block, then jumped to rest, while the control moved on every frame. `startBusyControl(page, {within})` now mounts the box inside the given element as a plain fixed box, with no popover and no top-layer entry. Its frames still count presented frames, and `raise()` leaves such a box where it is.
The self-test opens a modal dialog whose backdrop dims through a block, and judges the dim's brightness at a point against the control.
Red first: in WebKit at 8 workers, with the top-layer control the dim stalled in 5 of 24 runs, from 43 to 78 ms at the block's start, while the control moved ("17/20 moved, longest stall 76.5 ms at +20 ms"); with `within` it passed 24 of 24. Chromium passed either way.
…hold asks the layer, not the gestures) into meta#8203
There was a problem hiding this comment.
This is on hold for a spec decision: the branch carries #8665's amendment to spec:AST-094 FR41 (a mouse or pen drag that a gesture claims after it moves clears the page's text selection, while a press claimed at its own pointerdown keeps it), which main does not state yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise the amendment leaves this diff when #8665 lands on its own. The wheel and trackpad pull itself follows FR43 and behaves as described in Chromium, with three quick wheel notches resting the sheet on its half detent. Independently, please land #8386 and #8676 through their own reviews and merge main back in (the diff is 243 files and about 47,900 authored lines around a 350-line change), drop or move the pseudoElement option on the harness's sampleCurve that only its own proofs use, and in the stack replace class PressTracker with a factory, as react-component-runtime INV13 requires.
[Automated review]
| export async function sampleCurve( | ||
| locator: Locator, | ||
| times: readonly number[], | ||
| {pseudoElement}: {pseudoElement?: '::before' | '::after'} = {}, |
There was a problem hiding this comment.
Nothing in the BottomSheet wheel proofs calls sampleCurve with pseudoElement; its only callers are the two new harness proofs (harness.interaction.browser.spec.ts:1436-1505). Please drop the option, its README row and those proofs, or move them to the pull request whose proof samples a ::before or ::after.
| return tap.numberOfTapsRequired === 1 && tap.numberOfTouchesRequired === 1; | ||
| } | ||
|
|
||
| export class PressTracker implements TouchConsumer { |
There was a problem hiding this comment.
Still a class at this head. INV13 of architecture:react-component-runtime on main keeps classes to the recognizer family (its base, the arena and the host), and its verification fails "a class outside the recognizer family". #8763's createPressTracker would conform once it reaches this branch; this belongs to the stacked pull request that owns press tracking.
| (`spec:AST-017` FR59; DEC-9). A mouse or pen drag that a recognizer claims | ||
| takes no new text selection for that drag only. A claim made after the | ||
| drag has moved clears the page's selection; a press claimed at its own | ||
| pointerdown, before it moves, keeps it. An unclaimed mouse drag keeps its | ||
| released behavior. |
There was a problem hiding this comment.
This FR41 amendment, carried from #8665, adds a rule main does not state: a mouse or pen drag that a recognizer claims after it moves clears the page's selection, while FR41 on main says a mouse otherwise keeps its released behavior. It needs an engineering owner's decision, recorded with /approve-spec <full head SHA>, or it can leave this diff with #8665.
|
Thanks. On the two other points:
|
There was a problem hiding this comment.
This is on hold for a spec decision: the branch carries #8665's amendment to spec:AST-094 FR41 (a mouse or pen drag that a gesture claims after it moves clears the page's text selection, while a press claimed at its own pointerdown keeps it), which main does not state yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise the amendment leaves this branch and is decided in #8665. The wheel and trackpad pull itself follows FR43 on main, and independently of that decision these changes remain:
- Land #8386 and #8676 through their own reviews and merge
main, so the diff (50,548 authored lines today) carries only this pull request's 7 files. - Drop or move
sampleCurve'spseudoElementoption and its two harness proofs, which #8360's current head does not carry. - In the stack, replace
class PressTrackerwith thecreatePressTrackerfactory (#8763), as react-component-runtime INV13 requires.
[Automated review]
| (`spec:AST-017` FR59; DEC-9). A mouse or pen drag that a recognizer claims | ||
| takes no new text selection for that drag only. A claim made after the | ||
| drag has moved clears the page's selection; a press claimed at its own | ||
| pointerdown, before it moves, keeps it. An unclaimed mouse drag keeps its | ||
| released behavior. |
There was a problem hiding this comment.
This FR41 amendment, carried from #8665, adds a rule main does not state: a mouse or pen drag that a recognizer claims after it moves clears the page's selection, while a press claimed at its own pointerdown keeps it. It needs an engineering owner's decision (/approve-spec <full head SHA>), or it can leave this branch and be decided in #8665.
| export async function sampleCurve( | ||
| locator: Locator, | ||
| times: readonly number[], | ||
| {pseudoElement}: {pseudoElement?: '::before' | '::after'} = {}, |
There was a problem hiding this comment.
Nothing in the BottomSheet wheel proofs calls sampleCurve with pseudoElement; its only callers are the two new harness proofs (harness.interaction.browser.spec.ts:1833-1906). #8360's current head does not carry this option, so it is this pull request's own change; please drop it or move it to the pull request whose proof uses it.
| return tap.numberOfTapsRequired === 1 && tap.numberOfTouchesRequired === 1; | ||
| } | ||
|
|
||
| export class PressTracker implements TouchConsumer { |
There was a problem hiding this comment.
Still a class at this head. INV13 of architecture:react-component-runtime keeps classes to the recognizer family (its base, the arena and the host) and fails "a class outside the recognizer family". #8763's createPressTracker factory would fix it once merged into the stack.
Depends on #8386 and #8676. Review only this PR's change: https://github.com/vjeux/astryx/compare/0a3bb823ed09f93ffeec5f08a501a5376ad97753..9f6bc54496032ffd96a7bb0f6d55fd9a7805c9a9
Primary intent
On the web, a trackpad or a mouse wheel pulls a BottomSheet the way a drag does, proven by tests.
User or maintainer impact
On a desktop, scrolling over a sheet did nothing. Now:
Problem and solution fit
spec:AST-094FR43 has a pan take wheel and trackpad scrolling. #8676 gives every pan both scroll types, and #8386 puts every pointer on the sheet's pans, so with both merged the pull works with no sheet code of its own. Options:Authority
spec:AST-094FR43, FR36 (the release), FR14 (the content's edge);spec:AST-107FR7 with INV13's gaps (#8751).Change and evidence
Desktop Chrome, a trackpad scrolling up at the content's top, then down over the handle (without #8676, then this PR):
Scope
Testing
How to check, in the Snap points story on a laptop: scroll up with two fingers on the trackpad while the content is at its top (the sheet pulls down, then settles); scroll down over the handle (it rises); click a mouse wheel three times quickly (it rests on a detent).
What ran: the new hook tests fail on #8386 without #8676 (4 of 5) and pass here (the notch test caught an earlier #8676 regression, since fixed). The Chromium wheel specs pass; core, lab and the sheet's WebKit tests pass.
touch-support-bundlefails here, not on main; #8671 replaces that check.