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
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNew ComponentsRefreshControl (@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 |
f696241 to
2adb6f3
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval of this pull request waits on an engineering owner's spec decision: the draft spec:AST-094 requirements (FR1–FR23, DEC-1, DEC-2), RefreshControl's UIKit-parity behaviour in Lab, and the two new experimental core subpaths (@astryxdesign/core/motion/experimental, @astryxdesign/core/gestures/experimental, which spec:AST-017 FR21 does not cover) have no current spec authority. An engineering owner records that decision by commenting /approve-spec 2adb6f3b1f5994902204dfdcb8727820dffea32c on this pull request, and the author may do so if they are an engineering owner. Whatever the decision, please land the stacked dependencies first so this diff carries only the refresh-control commits, move the adaptive-presentation breakpoint change (744/500 px instead of 768 px) into its own pull request, and fix the three refresh-control defects:
- A controlled
isRefreshingthat staysfalsenever ends a pulled refresh (useRefreshControl.ts:455-460,:724-731). - Every pull leaves an inline
translateon the content, soposition: fixedchildren stop being fixed (useRefreshControl.ts:276-278,:530). - The pulled list gets a blocking
touchmovelistener although the pull never claims a touch (GestureHost.ts:148-153).
[Automated review]
| const callHandler = useCallback(() => { | ||
| const token = ++refreshToken.current; | ||
| const result = latest.current.onRefresh?.(); | ||
| if (latest.current.isControlled) { |
There was a problem hiding this comment.
In controlled mode a triggered pull returns here and can end only when isRefreshing changes (the Effect at line 724). A parent that keeps false, or sets true then false in one handler, leaves the list refreshing behind the 60 px inset with aria-busy="true". Render controlled state from the prop, or end on the first render after onRefresh in which the prop is still false.
| } | ||
| }, []); | ||
| const demote = useCallback(() => { | ||
| contentElement.current?.style.removeProperty('will-change'); |
There was a problem hiding this comment.
Only will-change is removed at rest; the content keeps the translate: 0px written by animator.set({y: 0}), which makes it a containing block for position: fixed children for the rest of the session (a fixed bar inside the page story moves from 0 to −284 px after one pull). Remove the inline translate here as well.
| }; | ||
| }, [applyEdge, isPullEnabled, releaseEdge, resolveOwner]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
This Effect sets state with no comment naming what it synchronizes (architecture:react-component-runtime INV1/INV3), and a lock already held at mount first renders the pull enabled. useSyncExternalStore(subscribeScrollLock, isScrollLockActive, () => false) reads it during render.
| function claimsWhileMoving(recognizer: GestureRecognizer): boolean { | ||
| return ( | ||
| recognizer.acceptsTouchInput && | ||
| !(recognizer instanceof TapGestureRecognizer) | ||
| ); | ||
| } |
There was a problem hiding this comment.
This ignores claimsTouches, so RefreshPullRecognizer (which never claims) still gives the whole refreshable list a {passive: false} touchmove listener; the README's Known limits say scrolls that start on such an element wait for the current task in Mobile Safari. Requiring recognizer.claimsTouches here removes it.
2adb6f3 to
eaffb19
Compare
There was a problem hiding this comment.
Approval of this pull request waits on an engineering owner's spec decision: the iOS pull-to-refresh direction for the Lab RefreshControl, the draft spec:AST-094 requirements (whose FR4 and DEC-1 keep touch motion off the theme while this branch makes it themable), and the two new experimental core subpaths, none of which has current spec authority. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever the decision, these still need changes:
- Land the stacked dependencies first so this diff carries only the refresh-control commit; it is over 33,000 authored lines today.
- Move the theme motion roles and the
platformadaptation condition (which contradictspec:AST-012FR2 andarchitecture:theme-tokensINV1) and the 744/500 px adaptive breakpoint into their own pull requests. - Make a controlled
isRefreshingthat staysfalseend a pulled refresh, apply the pull'soverscroll-behavior-y: noneonly while the scroll owner overflows (spec:AST-025FR14), give non-claiming recognizers no blockingtouchmovelistener, and replace the two state-setting Effects inuseRefreshControl.ts. - Add the
## Writing rubric resultssection that spec approval reads.
[Automated review]
| const callHandler = useCallback(() => { | ||
| const token = ++refreshToken.current; | ||
| const result = latest.current.onRefresh?.(); | ||
| if (latest.current.isControlled) { |
There was a problem hiding this comment.
In controlled mode a triggered pull returns here, and only the Effect at line 750 can end it, which runs only when isRefreshing changes. A parent that keeps false, or sets true then false in one handler, leaves the list refreshing behind the 60 px inset with aria-busy="true". Render controlled state from the prop, or end on the first render after onRefresh in which the prop is still false.
| }; | ||
| }, [applyEdge, isPullEnabled, releaseEdge, resolveOwner]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
This Effect sets state with no comment naming what it synchronizes (architecture:react-component-runtime INV1/INV3), and a lock already held at mount first renders the pull enabled. useSyncExternalStore(subscribeScrollLock, isScrollLockActive, () => false) reads it during render.
| const near = | ||
| isPullEnabledRef.current && | ||
| scrollTopOf(current) < Math.max(heightOf(current), REFRESH_HEIGHT); |
There was a problem hiding this comment.
near never checks that the owner overflows, so a short list keeps overscroll-behavior-y: none on an area that does not scroll; spec:AST-025 FR14 allows a containment policy only on axes that currently overflow.
There was a problem hiding this comment.
Kept on purpose. This is the refresh pull's own containment, not ScrollableArea's, and it follows UIKit: a UIScrollView with a UIRefreshControl always bounces vertically (alwaysBounceVertical), so a list shorter than its scroll view still pulls and refreshes. That is one of this stack's interaction proofs ("a list shorter than the screen still pulls and refreshes").
Without the claim, a pull on a scroll owner that doesn't overflow chains to the page: the page rubber-bands under the pull, and on iOS Safari the page's own reload can fire. The claim is held only near the top, only while a control is mounted and the pull is enabled (iOS only), and it is dropped while a scroll lock holds the page.
If AST-025 FR14 should govern the refresh pull too, that needs an owner decision on the spec, and pulls on short lists would stop working.
| recognizer.acceptsTouchInput && | ||
| !(recognizer instanceof TapGestureRecognizer) | ||
| ); | ||
| } |
There was a problem hiding this comment.
This ignores claimsTouches, so RefreshPullRecognizer (which never claims) still gives the whole refreshable list a {passive: false} touchmove listener; the note at the top of this file says a scroll that begins on such an element waits for a busy main thread in iOS Safari. Requiring recognizer.claimsTouches here removes it.
| * 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 is a fifth condition field; spec:AST-012 FR2 keeps the when vocabulary closed to width, pointer, contrast and motion until that record adds one, so this needs a pull request that amends FR2.
| } | ||
| const variable = motionRoleVariable(id); | ||
| try { | ||
| tokens[variable] = formatMotionRoleValue(kind, value); |
There was a problem hiding this comment.
These --motion-* names come from table paths and are never declared in tokens.stylex.ts, which architecture:theme-tokens INV1 names as the origin of every core token name; the draft spec:AST-094 FR4 in this same pull request also says touch motion is not themable.
eaffb19 to
4e4c16b
Compare
There was a problem hiding this comment.
Approval of this pull request waits on an engineering owner's spec decision: the iOS pull-to-refresh direction for the Lab RefreshControl, the draft spec:AST-094 requirements (whose FR4 and DEC-1 keep touch motion off the theme while this branch makes it themable), and the two new experimental core subpaths have no current spec authority. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever the decision, these still need changes:
- Land the stacked dependencies first so this diff carries only the refresh-control commit; it is over 33,000 authored lines today.
- Make the adaptive compact test follow
spec:AST-012FR9 (width below the effectivemd, 768 px by default, or height below 500) instead of 744 px, and move the theme motion roles and theplatformcondition, which contradictspec:AST-012FR2 andarchitecture:theme-tokensINV1, into their own pull request. - Make a controlled
isRefreshingthat staysfalseend a pulled refresh, start the end slide fromend()so a refresh that begins and ends in one render still slides back, apply the pull'soverscroll-behavior-y: noneonly while the scroll owner overflows (spec:AST-025FR14), give non-claiming recognizers no blockingtouchmovelistener, and replace the two state-setting Effects inuseRefreshControl.ts. - Add the
## Writing rubric resultssection that spec approval reads.
[Automated review]
| if (latest.current.isControlled) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
In controlled mode a triggered pull returns here, and only the Effect at line 848 can end it, which runs only when isRefreshing changes. A parent that keeps false, or sets true then false in one handler, leaves the list refreshing behind the 60 px inset with aria-busy="true". End a triggered refresh on the first commit after onRefresh returns in which the prop is still false, or render controlled state from the prop.
| useEffect(() => { | ||
| if (isRefreshing || pendingEnd.current == null) { | ||
| return; | ||
| } | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = null; | ||
| const play = pendingEnd.current; | ||
| pendingEnd.current = null; | ||
| play?.(); | ||
| }); | ||
| }); | ||
| }, [isRefreshing]); |
There was a problem hiding this comment.
The end slide now waits for this Effect, which runs only when isRefreshing changes. When the begin and the end land in one render (a refresh() from a timer or socket handler with an onRefresh that settles at once), it never runs: aria-busy clears but the content stays at translate: 0px 60px with the spinner spinning in the gap. Scheduling the two frames from end() itself avoids depending on a state change rendering.
| const near = | ||
| isPullEnabledRef.current && | ||
| scrollTopOf(current) < Math.max(heightOf(current), REFRESH_HEIGHT); |
There was a problem hiding this comment.
near never checks that the owner overflows, so an overflow: auto container whose content fits still gets overscroll-behavior-y: none; spec:AST-025 FR14 allows a containment policy only on axes that currently overflow.
| useEffect(() => { | ||
| setIsLocked(isScrollLockActive()); | ||
| return subscribeScrollLock(setIsLocked); | ||
| }, []); |
There was a problem hiding this comment.
This Effect sets state with no comment naming what it synchronizes (architecture:react-component-runtime INV1/INV3), and a lock already held at mount first renders the pull enabled. useSyncExternalStore(subscribeScrollLock, isScrollLockActive, () => false) reads it during render.
| function claimsWhileMoving(recognizer: GestureRecognizer): boolean { | ||
| return ( | ||
| recognizer.acceptsTouchInput && | ||
| !(recognizer instanceof TapGestureRecognizer) | ||
| ); | ||
| } |
There was a problem hiding this comment.
This ignores claimsTouches, so RefreshPullRecognizer (which never claims) still gives the whole refreshable list a {passive: false} touchmove listener; the note at the top of this file says a scroll that begins on such an element waits for a busy main thread in iOS Safari. Requiring recognizer.claimsTouches here removes it.
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; | ||
|
|
||
| /** Both: presentations such as a sheet in place of a popover. */ | ||
| export const COMPACT_TOUCH_QUERY = | ||
| '(width < 744px) and (pointer: coarse), (height < 500px) and (pointer: coarse)'; |
There was a problem hiding this comment.
Current spec:AST-012 FR9 defines compact as width below the effective md point (768 px by default) or height below 500, exported once from the theme, and says no resolver keeps size numbers of its own. The 744 px edge sends a 744–767 px touch screen to the popover where FR9 promises the sheet.
| * 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 is a new condition field; spec:AST-012 FR2 keeps the when vocabulary closed to width, height, pointer, contrast and motion until that record adds one, so this needs a pull request that amends FR2.
| const variable = motionRoleVariable(id); | ||
| try { | ||
| tokens[variable] = formatMotionRoleValue(kind, value); |
There was a problem hiding this comment.
These --motion-* names come from table paths and are never declared in tokens.stylex.ts, which architecture:theme-tokens INV1 names as the origin of every core token name; the draft spec:AST-094 FR4 in this same pull request also says touch motion is not themable.
4e4c16b to
8bfbe7c
Compare
github-actions
Bot
added
the
needs:design-review
…, on the split touch layer Restacked on meta#8175's split hook: the drawing's layer animators, showRevealLayer, attachLayer and the two layer refs live in refreshTouchController.ts, the touch module that useRefreshControl imports once, and the indicator names come from refreshIndicator.ts. The gate is a finger on any touch device (useTouchPresentation), not iOS. The trigger's cross-fade from the revealed spokes to the stepping ones passes no reduced-motion option, so under Reduce Motion the animator fades it, opacity only, in 200 ms or less on a touch screen (design:reduced-motion DR2). The Chromium test that pinned the drawing unchanged under reduced motion now records the animations the trigger starts and checks that (it fails on an animator that plays the opacity spring as is). The drawing's browser tests find the glyph inside the indicator band, after the content. The drawing, its unit tests and its interaction tests are as before (the measured spokes, the 0.8 s stepping gradient, plus-lighter's sum at the trigger, the stepping kept through the end's exit).
8bfbe7c to
435e3f7
Compare
…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.
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for the Lab RefreshControl (the four choices the description proposes, including the UIKit-table motion it plays, which current spec:AST-094 leaves to the component's own record) and to settle three conflicts with current spec: the refresh starting with the finger still down (spec:AST-094 FR36/FR37), a pull that works under a coarse pointer only (spec:AST-094 FR19/FR43), and overscroll-behavior-y: none on a scroll container that does not overflow, which needs spec:AST-025 FR14 updated if it stays. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever the decision, these still need changes:
- Land the stacked predecessors first: this pull request carries 35,029 authored lines, about 5,300 of them for RefreshControl itself.
- Make
scripts/touch-support-bundle.test.mjspass again and let the gesture host attach for every pointer (architecture:react-component-runtimeINV13,spec:AST-094FR43). - Remove the per-move scroll-offset read (
spec:AST-094FR9), keep the title's status element mounted (spec:AST-086FR3), route the animator throughinternal/touch(INV13), and stop the page jump when the refresh button runs on short content. - Add fine-pointer before-and-after evidence for the scroll-lock change (
spec:AST-017FR59) and real-device evidence for the iOS Safari and Android Chrome claims.
[Automated review]
| {...stylex.props(styles.spokes)}> | ||
| {spokes} | ||
| </span> | ||
| {title != null ? ( |
There was a problem hiding this comment.
The role="status" span mounts only once title is set, so a title that first appears after mount lands in a newly mounted live region already holding its text, which spec:AST-086 FR3 forbids. Render the status span always and leave it empty without a title.
| const scrolledBack = | ||
| scrollTopOf(owner.current) - geometry.scrollTop0 > 0.5; |
There was a problem hiding this comment.
This reads scrollTop (or window.scrollY) on every move after the pull starts; spec:AST-094 FR9 allows no layout read after the gesture begins. Flag the scroll-back from the owner's scroll event, which the controller already listens for (:490-505), instead of reading the offset per move.
| place(REFRESH_HEIGHT); | ||
| scrollOwnerBy(current, REFRESH_HEIGHT); |
There was a problem hiding this comment.
When the owner cannot scroll by the whole inset, the 60 px translate shows: in the Controlled story with three rows, pressing Refresh scrolled the page 32 px and dropped the list 28 px in one frame. Settle the inset in, or skip it, when the owner cannot absorb it, and do not scroll the page for a scroller that does not overflow.
| * | ||
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the |
There was a problem hiding this comment.
Current architecture:react-component-runtime INV13 says the host "attaches for every pointer, with no touch-only query", and spec:AST-094 FR19 and FR43 require recognizers to attach for every pointer; this gate keeps the host coarse-only.
|
|
||
| import {useEffect} from 'react'; | ||
| import {claimRootScrollStyle, reportScrollLock} from './rootScrollStyles'; | ||
| import {holdPageScroll} from '../gestures/pageHold'; |
There was a problem hiding this comment.
Importing the page hold here brings gestures/pageHold.ts and gestures/hostRegistry.ts into Core's root and hooks entries, and scripts/touch-support-bundle.test.mjs fails its two entry tests at this head (they pass on the base).
…he page presented Two proofs failed on a loaded machine with no defect in the control: - the busy-main-thread settle counted the screencast's delivered frames against its cadence. It now hands the compositor's own frame record to busyWindowProof, whose verdict alone judges the settle; a settle paused before the block fails it. - the catch read the settle's position before the touch, which lands later under load. A capture listener now records where the settle stood as the finger landed; a catch that restarts from zero travel fails it.
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for the Lab RefreshControl (the four choices the description proposes, including the UIKit-table motion it plays, which current spec:AST-094 leaves to the component's own record) and to settle three intended departures from current spec: the UIKit release (the refresh starts with the finger still down, and any later lift settles onto the inset from rest, whatever its velocity, against spec:AST-094 FR36–FR38), a pull that works under a coarse pointer only (spec:AST-094 FR19/FR43), and overscroll-behavior-y: none on a scroll container that does not overflow, which needs spec:AST-025 FR14 updated if it stays. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever the decision, these still need changes:
- Land the stacked predecessors first: this pull request carries 35,051 authored lines, about 5,300 of them for RefreshControl itself.
- Make
scripts/touch-support-bundle.test.mjspass again and let the gesture host attach for every pointer (architecture:react-component-runtimeINV13,spec:AST-094FR43). - Remove the per-move scroll-offset reads (
spec:AST-094FR9), keep the title's status element mounted (spec:AST-086FR3), route the animator throughinternal/touch(INV13), stop the page jump when the refresh button runs on short content, attach the pull's recognizer only under a touch presentation, and move the@astryxdesign/builddefault change to its own pull request. - Add fine-pointer before-and-after evidence for the scroll-lock change (
spec:AST-017FR59) and real-device evidence for the iOS Safari and Android Chrome claims.
[Automated review]
…dy part The proof failed on a loaded machine with one long task: it judged long tasks over the whole trace, the pull's trigger and the release's render included, while Layout and Paint were judged only in the steady part. Under a 4x CPU throttle the release's last commit frame also landed past the fixed 50 ms start. The steady part now starts once the page has rendered three frames after the settle began, counted in frames, and long tasks are judged there too. It passes 4 of 4 under the throttle; a paint on every frame of the settle, or an 80 ms block inside it, still fails it.
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for the Lab RefreshControl (the four choices the description proposes, including the UIKit-table motion it plays, which current spec:AST-094 leaves to the component's own record) and to settle three intended departures from current spec: the UIKit release (the refresh starts with the finger still down, and any later lift settles onto the inset from rest, whatever its velocity, against spec:AST-094 FR36–FR38), a pull that works under a coarse pointer only (spec:AST-094 FR19/FR43), and overscroll-behavior-y: none on a scroll container that does not overflow, which needs spec:AST-025 FR14 updated if it stays. An engineering owner records that 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 latest commit's narrower settle trace is sound, but whatever the decision, these still need changes:
- Land the stacked predecessors first: this pull request carries 35,091 authored lines, about 5,400 of them for RefreshControl itself.
- Make
scripts/touch-support-bundle.test.mjspass again and let the gesture host attach for every pointer (architecture:react-component-runtimeINV13,spec:AST-094FR43). - Remove the per-move scroll-offset reads (
spec:AST-094FR9), keep the title's status element mounted (spec:AST-086FR3), route the animator throughinternal/touch(INV13), stop the page jump when the refresh button runs on short content, attach the pull's recognizer only under a touch presentation, and move the@astryxdesign/builddefault change to its own pull request. - Add fine-pointer before-and-after evidence for the scroll-lock change (
spec:AST-017FR59) and real-device evidence for the iOS Safari and Android Chrome claims.
[Automated review]
…oined (#9070) ## Exact decision needed Whether `spec:AST-086` FR9's shared channel joins a component's superseded message about its current state with its next one, and the optional public `useAnnounce` option that asks for replacement. Today every message written to a region before the previous one is exposed is joined, so a component that states its current state on every input, such as a search's result count, has all its stale states spoken together when input outpaces a frame (a barcode scanner, dictation, automation): `1 result Create "Urg" Create "Urge" Create "Urgen" Create "Urgent"`. ## Existing authority searched - `spec:AST-086` FR9 (the join), FR6 (an identical message repeated in a frame is one message), FR8 (a busy start its outcome overtakes is dropped), FR13 and FR14 (own-content messages and modal containers). - `architecture:interaction-modality` INV8: superseded work must not change the current status or announcement, with supersession left to component contracts. - `spec:AST-075` DEC-2 and FR7: Astryx's comboboxes announce their result count with `useAnnounce`, the same hook products use. Tokenizer's record: its sheet's count and empty-state text go through `useAnnounce`. MultiSelector's record (draft): its result count and create row label go through the polite channel. - `spec:AST-002` FR1, FR2, FR5, FR7, FR16 and FR20 for the new public option; `has*` booleans such as `hasClear` and `hasPaging` for its name. - Open pull requests that mention AST-086 (#7871, #7902, #7910, #8175, #8211, #8583, #8001): none edits FR9. #8640 implements the replacement for MultiSelector and waits for this decision. ## Decision A `useAnnounce` instance created with `hasReplace` states one current state of its owner, such as a search's result count: each message it writes withdraws that instance's previous message while it is not yet exposed, whatever their urgencies, and is spoken in the region of its own urgency; a message already exposed has been heard and stays. Messages from other instances, and from instances without `hasReplace`, still join. `useAnnounce` takes an optional options object with one field, `hasReplace` (default `false`), and the `announce(message, politeness)` it returns is unchanged. FR14's modal containers withdraw the same way, and a picker's own empty-panel message comes from its search's `hasReplace` instance (FR13). Astryx's search counts and MultiSelector's create row use a separate `useAnnounce({hasReplace: true})` instance (`spec:AST-075` FR7, Tokenizer's and MultiSelector's records); selection messages keep their own plain instance. DEC-4 records it; its decider is the approving engineering owner, with INV8 and FR8 as its reasons. Public option (`spec:AST-002` FR5, FR20): two otherwise identical messages, `"7 results"` written twice in one frame, need different outcomes. As a search's count it replaces the earlier one; as two distinct notices both are spoken. Only the caller knows which it sends, and the channel cannot derive it. Before: `const announce = useAnnounce(); announce(t('resultCount', {count}));`. After: `const announceCount = useAnnounce({hasReplace: true}); announceCount(t('resultCount', {count}));`. Existing calls are unchanged. ## Rejected alternatives - Each owner debouncing its own messages: every announcement waits, and every owner repeats the rule. - An exception for every `useAnnounce` message: Astryx's comboboxes and Tokenizer announce through `useAnnounce`, so their counts would keep joining. - A string key on each message (`announce(message, politeness, {currentState})`): a third argument whose strings every call must keep consistent, where one instance already states one state. ## Claim scope and non-goals FR9's replacement within one `hasReplace` instance, the option that asks for it, and the AST-075, Tokenizer and MultiSelector sentences that describe their counts' announcement. Which messages a component sends through a `hasReplace` instance stays with its own record. ## Evidence and implementation relationship - On main, the MultiSelector CreateRow accessibility spec, which types into the create row at about four keystrokes per frame, fails 9 of 10 runs alone and 26 of 48 under load, its polite region reading the joined statuses. - #8640 implements the replacement for MultiSelector. With it, the spec passes 10 of 10 alone and 48 of 48 under load. It lands after this record changes, adopting `useAnnounce({hasReplace: true})`. ## Writing rubric results - R1 narration: none, in AST-086, AST-075, Tokenizer's or MultiSelector's record. - R2 research in the record: none. - R3 evidence naming a change: none. - R4 rejected alternatives over one line or outside a decision: one, inside DEC-4, one line; nothing added to the other records. - R5 pull request or issue references remaining: none. - R6 ruled open questions still listing options: none. ## Scope - [x] This PR contains specification records only. - [x] It records observable behavior and preserves equivalent internal implementations. - [x] Any named internal mechanism is intentionally public, names its dependent caller/system, and explains why an equivalent implementation would not satisfy the contract. - [x] Verification states the evidence layer and failure signal without prescribing CI job/workflow topology. - [x] It records one intentional durable decision, not a rescue for a separable tagalong. - [x] The canonical knowledge owner, affected current claims, and overlapping open work are named; any ownership collision is resolved or called out for human decision. - [x] Adjacent API, visual, accessibility, composition, and implementation facts remain outside scope unless this exact decision depends on them. - [x] Public text and artifacts contain no internal Meta context. ## Validation Local gates at 2b84901: `pnpm check:knowledge` (records and templates aligned), Prettier, `git diff --check`, `pnpm check:repo`.
Depends on #8171. Carries #8562 (Core's internal entries) as a dependency; if the entries land differently, this follows her decision. Review only this PR's change: vjeux/astryx@338dc33...16f8df7
Why
Astryx has no pull to refresh: on a phone, a pull at the top of a list reloads the whole page instead of refreshing the list.
What led to it
Part of the touch-presentations program (
spec:AST-094). #8086 measured UIRefreshControl, and #8170 and #8171 built the pull; this is the component that renders it.Options and tradeoffs
onRefresh(a promise keeps it refreshing) and a controlledisRefreshing.Why this choice
These keep the pull opt-in per list and leave everything else to the browser. It works under a finger, on iOS and Android alike (#8688 and #8697 add the mouse, pen, trackpad and wheel). The docs require a visible refresh button, since a pull has no keyboard or screen-reader equivalent. Under Reduce Motion a release and the end only fade, within 200 ms. These four choices are proposed for owner review. The new lab exports are canary only:
RefreshControlanduseRefreshControl.How to check it
On a phone or the iOS simulator:
Evidence
With #8251's spinner, iOS 26.2 simulator at 60 fps: pull · cancel · end · short list · page, no reload · push back · pull again · catch · Reduce Motion. Android 15 emulator: scroll area · page, no reload.
touch-support-bundlefails because this branch carries fix(useScrollLock): keep the page where it is while an overlay is open #8098's scroll lock; test(core): a touch component's import carries its gesture code, and nothing else does #8671's per-component test replaces it and passes here.Stack (merge in this order): #8360 gesture foundation (carries #8076) → #8086 refresh values → #8170 pull and state machine → #8171 useRefreshControl → this PR. It also needs #8098 (useScrollLock v2, on #8360).