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.
|
fbd7d10 to
26ae880
Compare
26ae880 to
638d774
Compare
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNew ComponentsSearchField (@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. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
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 needs an engineering owner's decision before it can be approved: the new draft spec:AST-094 (FR1-FR23, DEC-1, DEC-2) and the placement="bottom" contract it is cited for, including the prop's name next to Core's existing placement ('above' | 'below' | 'start' | 'end'), have no current spec authority yet. An engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise say what to amend. Whatever that decision, these changes are needed:
- Land the nine stacked pull requests and rebase, so this pull request carries only its own commit (about 1,600 lines) instead of 32,002 authored lines.
- A change of mind restarts the bottom field's frame tables at their first row, so the field snaps (43 px at the trailing edge for a cancel 153 ms into the activation, up to 60 px when the field is tapped again early in a cancel); start an interrupted move from the presented pose and add a proof for it.
- Resolve the default
CancelandClear textnames through the translator, and add the record's## Writing rubric resultsto the description.
[Automated review]
| settledRef.current = target; | ||
| // iOS changes the field's frame each frame with no animation object: | ||
| // play the measured frames, every part on one clock. | ||
| const motion = bottomMotion(target, width); |
There was a problem hiding this comment.
A change of mind lands here mid-move: bottomMotion(target, width) always starts the other frame table at its first row, so a cancel 153 ms into the activation moves the trailing edge from 352 px to 309 px in one frame on a 402 px screen. The stacked path starts from presented() with remainingFraction (lines 505-508) and has a mid-activation proof; entering the new table from the presented edges and height, with the same proof for the bottom placement, would remove the snap.
There was a problem hiding this comment.
Fixed in e4f2ea4: a change of mind no longer starts the other frame table at its first row. It adds an eased turn from the pose the field shows (read off its parts) to the curves on screen, on the activation's whole clock, as UIKit's animations add (the animator's additive). The interaction spec's "a cancel mid-activation turns around from where the field is, without a snap" and "focusing the field early in a cancel re-activates from where the field is" fail on the old head and pass here in Chromium and WebKit (continuous velocity, one turn, no step faster than the measured gel itself).
| cancelLabel = 'Cancel', | ||
| clearLabel = 'Clear text', |
There was a problem hiding this comment.
These defaults become the close and clear buttons' accessible names, so screen-reader users of a localized app hear English unless both props are passed. Resolving the defaults through the translator, as lab's Comments components do, and keeping the props as overrides would follow the catalog rule for Astryx-owned strings.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] `presentation="adaptive"` on DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector chooses the bottom sheet on a compact touch screen: a coarse primary pointer and a viewport narrower than 744px or shorter than 500px (it was 768px wide and below). An iPad mini in portrait keeps the anchored popover; a phone in landscape now gets the sheet. |
There was a problem hiding this comment.
At 744 px wide the old (max-width: 768px) and (pointer: coarse) query gave an iPad mini in portrait the bottom sheet, so it does not keep the anchored popover: it switches to it. Naming the 744 to 768 px change and the presentation value that keeps the sheet would tell upgraders what moves.
There was a problem hiding this comment.
This changeset is #8076's (the foundation under this stack). At this head it describes the theme's md point (768px by default): a coarse-pointer viewport narrower than it still gets the sheet, so an iPad mini in portrait (744px) keeps the bottom sheet, and only a screen exactly 768px wide switches to the popover. The wording is #8076's to settle.
638d774 to
7d040b2
Compare
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved: the new draft spec:AST-094 and the placement="bottom" contract it is cited for have no current spec authority yet, including the prop's name next to Core's existing placement ('above' | 'below' | 'start' | 'end') and whether FR8's Reduce Motion cross-fade applies to a field that keeps its travel. An engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise say what to amend. Whatever that decision, these changes are needed:
- Land the ten stacked pull requests and rebase, so this pull request carries only its own commits (about 1,650 lines) instead of 32,128 authored lines.
- A change of mind restarts the bottom field's frame tables at their first row, so the field snaps (43 px at the trailing edge for a cancel 153 ms into the activation, up to 60 px when the field is tapped again early in a cancel); start an interrupted move from the presented pose and add a proof for it.
- Resolve the default
CancelandClear textnames through the translator, add the record's## Writing rubric resultsto the description, and correct the adaptive-presentation Changeset's iPad mini statement.
[Automated review]
7d040b2 to
434f33d
Compare
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved: the new draft spec:AST-094 and the placement="bottom" contract it is cited for have no current spec authority yet, including the prop's name next to Core's existing placement ('above' | 'below' | 'start' | 'end'), whether FR8's Reduce Motion cross-fade applies to a field that keeps its travel, and whether FR4's rule that touch physics are not themable stands beside the theme motion roles this diff ships. An engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise say what to amend. Whatever that decision, these changes are needed:
- Land the thirteen stacked pull requests and rebase, so this pull request carries only its own commits (about 1,950 lines) instead of 36,900 authored lines.
- A change of mind restarts the bottom field's frame tables at their first row, so the field snaps (43 px at the trailing edge for a cancel 153 ms into the activation, up to 60 px when the field is tapped again early in a cancel); start an interrupted move from the presented pose and add a proof for it.
- Regenerate the
docs-topics.jsonCLI golden sotest-node (1/4)passes, resolve the defaultCancelandClear textnames through the translator, add the record's## Writing rubric resultsto the description, and correct the adaptive-presentation Changeset's iPad mini statement.
[Automated review]
|
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.
|
434f33d to
e4f2ea4
Compare
vjeux
changed the title
github-actions
Bot
added
the
needs:design-review
…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.
…task ended at once, however late the first read WebKit under load from other tasks: the call's task ends at once, another task holds the main thread, and the browser commits the animation after it, dated at the call. Nothing in the call's task delayed it, but the first read still moved its start to the read, about 95 ms late on a loaded box, so whatever chains on its end (the second half of a Reduce Motion cross-fade, a cancel fading on the previous fade's end) ran that much late and the cross-fade missed design:reduced-motion DR2's 200 ms. Fails now: the start moves to 1100.
e7b0676 moved a call's start from rest to its first read whenever WebKit had dated it more than two frames before that read. Under load from other tasks the first read comes late with nothing in the call's task delaying it, so the start moved about 95 ms on a loaded box, and whatever chained on the call's end ran that late: the second half of a Reduce Motion cross-fade, so SearchField's cancel missed DR2's 200 ms. The call's task now records when it ended, at its first microtask (or a read in those microtasks is that time), and only a call whose own task ran past the call by more than two frames, the block e7b0676 fixed, moves its start; a read other tasks delay keeps the browser's start.
…s its start past the block Core Animation dates an animation at the commit, when the calling task ends, however short the block before it. A 20 ms block in the call's task, with WebKit's start dated at the call, left the start at the call: the two-frame bound skipped it. Fails now.
… task, with no threshold Core Animation dates an animation at the commit, when the calling task ends, whatever the block's length. The call's task already records when it ended, at its first microtask, so a start the browser dated before that (WebKit's last frame time before the task) now moves to it: a task that ended at once leaves the start at the call, a blocked one moves it past the block, however short, and a read other tasks delay changes nothing. The two-frame bound, which told a read's own frame from a block before the task's end was known, goes, and the start no longer moves to the read.
…'s click guard has taken the click The claimed tap swallows its own late mouse events until their click. With meta#8360, the gesture host arms its click guard at the claimed lift, and the guard takes that click first, on the window's capture phase, so the swallow never saw it and stayed armed: the next press's mousedown, mouseup and click never reached the page. The swallow now notes the guard while it is armed and lets go once the guard has (its click, the next press, or its window); with no guard it ends at the click, as before. `internal/touch` re-exports `isClickGuardArmed` by name. Red-first: "a claimed tap swallows its own late mouse events, until the next touch" fails on the previous head (the second press's events are swallowed) and passes in Chromium and WebKit with this change.
…few ms, and one task's calls start together Browser start times are frame times, while a task's end is a live clock, so on every call they differ by up to a frame plus the task's own length. Moving every start from rest to its task's end shifted each one by that length, about 5 ms in a component's effect after a tap, and the shift added up along a chained sequence: SearchField's Reduce Motion cancel measured 204-207 ms against design:reduced-motion DR2's 200. The first case fails now (the start moves to 1010). The second keeps one motion's parts together when they are separate calls in one task: a block measured per call would split them by a frame.
…n on more than two frames, and all the task's calls move together fc42d64 moved every start from rest to the end of the call's task. Browser start times are frame times while that end is a live clock, so on every call they differ by up to a frame plus the task's own length: each start slid by that length, and a chained sequence added it up, about 5 ms per link in SearchField. The rule from e6876ca is back, with its bound now named for that quantization: only a task that ran on more than LATE_START_MS (two frames) past its first call blocked anything, and its calls' starts move to their first read. The block is now measured per task, not per call, and every call of a blocked task takes the same read, so one motion's parts that are separate calls in one task (a component's effect) start together instead of splitting by a frame. The 20 ms case keeps the browser's start, as no clock here tells it from quantization.
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: Lab SearchField's new placement prop and bottom layout have no governing spec, and the bottom placement's replayed iOS frames on a fixed 383.3 ms clock conflict with spec:AST-094 FR4, which requires named motion roles. 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; the owner can also say what to rename, amend or reject. Independently of that decision, these need changes:
- Take the keyboard height from
spec:AST-099FR2's visual-viewport measurement in Chrome too, and stop settingoverlaysContent(useKeyboardHeight.ts). - Let the gesture host's recognizers take input under every pointer, as
spec:AST-094FR19 and FR43 require (GestureHost.ts). - Gate the tap claim and its document
blurlistener on a coarse pointer, asspec:AST-094FR24 requires (SearchFieldTouch.ts). - Land or split the predecessor work so this pull request carries only the bottom placement, and add the FR2 pan and pinch-zoom and FR7 iframe tests.
[Automated review]
| const hold = () => { | ||
| if (appOverlay == null) { | ||
| appOverlay = keyboard.overlaysContent; | ||
| keyboard.overlaysContent = true; |
There was a problem hiding this comment.
spec:AST-099 FR6, FR8 and DEC-4 take the keyboard height from FR2's visual-viewport measurement in Chrome too, and FR8 says Astryx never overrides the app's own viewport setting. This branch sets overlaysContent and commits boundingRect.height instead; please measure by FR2 in every browser and leave overlaysContent alone.
| const [claimsTap, setClaimsTap] = useState(false); | ||
| useLayoutEffect(() => { | ||
| const win = rootAnimator.animator.element?.ownerDocument.defaultView; | ||
| setClaimsTap(!isStacked && win != null && keyboardCoversPage(win)); |
There was a problem hiding this comment.
This gate has no pointer condition, so desktop Safari and Firefox (a visual viewport, no navigator.virtualKeyboard) also get the per-field document blur listener at lines 592-605 under a mouse, against spec:AST-094 FR24. Gating it like isLifted (!isStacked && isTouch && keyboardCoversPage(win)) keeps the claim to a finger.
| * coarse, or with `perEventPointer` (FR19; `spec:AST-057`). | ||
| */ | ||
| private isActive(recognizer: GestureRecognizer): boolean { | ||
| return this.isCoarse || this.perEventRecognizers.has(recognizer); |
There was a problem hiding this comment.
spec:AST-094 FR19 and FR43 (DEC-21) now attach recognizers for every pointer, and INV13 says the host attaches with no touch-only query; this gate keeps every recognizer off under a fine pointer unless it opts into perEventPointer.
| useLayoutEffect(() => { | ||
| focusOptionsRef.current = focusOptions; | ||
| }); |
There was a problem hiding this comment.
This layout effect names no external system (architecture:react-component-runtime INV3). The options derive from placement alone, so the handle and the clear button can compute them from the prop, with placement in the handle's dependencies.
Depends on #8181 and predecessors. Review only this PR's change: vjeux/astryx@ee835dc...fc37107
User need
iOS 26 puts a phone's search field in the bottom toolbar. Built naively, Safari pans the page up when it focuses, and the field must follow the keyboard, centred 36 pt above it.
Problem and solution fit
What led to it: the UIKit-parity program puts #8181's field in a bottom bar (
placement="bottom"):spec:AST-099FR2, FR8).Current specification
spec:AST-094FR1, FR2, FR13 andspec:AST-099FR2, FR4, FR8. Pending owner review: theplacementprop, and replaying measured frames.Public delta and caller burden
New
placementprop; internalsearch.bottomentries; one added element, bottom only (INV7), carrying the magnifier and query.Size (gzip, base → head):
SearchFieldJS 41.70 → 61.20 KB, CSS 1.47 → 1.54 KB.Design and alternatives
Not here: automatic placement, the minimized button, adaptive widths.
Evidence
Scope
Testing
What I ran and what it showed (the "Bottom" story):
Local runs are the evidence; no CI run is expected.