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.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNew ComponentsPageControl (@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.
Approval needs an engineering owner's decision first: the PageControl behaviour and the new @astryxdesign/core/motion/experimental and @astryxdesign/core/gestures/experimental subpaths rest only on draft spec AST-094, which this PR adds unapproved, including the two choices it marks pending (the window slide kept under Reduce Motion and the --color-background-muted platter). 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 that decision, these need changes:
- Against
mainthe PR is about 28,000 authored lines across 28 commits; please land the predecessor PRs first and rebase so this PR is the PageControl change. - The new
adaptivethresholds for DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector are separable from PageControl, and their Changeset names no way to keep the old 768 px behaviour; please split them out. - PageControl never attaches its touch recognizer when it first renders with no pages, because the early
return nullmeetsuseGestureRecognizer's attach-once layout effect. - A page the app declines is never requested again, and a server-rendered control with more than ten pages paints its dots outside the control until hydration.
[Automated review]
| const requestRef = useRef<{page: number; kind: ChangeKind} | null>(null); | ||
| const request = (page: number, kind: ChangeKind) => { | ||
| const latest = latestRef.current; | ||
| if (page === latest.current || page === requestRef.current?.page) { |
There was a problem hiding this comment.
If the app declines this page (leaves value unchanged), requestRef keeps it until value changes (:499-501), so every later tap, arrow key or End for the same page returns here without calling onChange. Dropping the pending request once the app has answered (the next commit) would keep a declined request repeatable.
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in 4c6d228 (see the reply there).
| before.pages !== pages || | ||
| before.windowStart === windowStart | ||
| ) { | ||
| animatorFor(strip).set({x: offsetFor(windowStart)}); |
There was a problem hiding this comment.
The resting translate and dot scales exist only from this layout effect, so server-rendered HTML for more than ten pages places the dots at page × 17.67 px with no offset or scale until hydration (the Many Pages story's dots land about 261–536 px from the control's start edge). Rendering the resting translate and scale from windowStart, and keeping the animator for the slide, would make the first paint right.
There was a problem hiding this comment.
Fixed in 4c6d228: the markup carries the resting window: the strip's offset and each dot's scale as custom properties a class turns into translate/scale (the animator's inline values override them once it runs). Test: "server-rendered, a long control already carries its resting offset and sizes" (failed before).
| } | ||
| }, [current]); | ||
|
|
||
| if (pages === 0 || (hidesForSinglePage && pages === 1)) { |
There was a problem hiding this comment.
With null here the root does not exist when useGestureRecognizer's layout effect runs at mount, and that effect only re-runs when [ref, recognizer, manageStyle] change (useGestureRecognizer.ts:73-81). A control first rendered with count={0} (or one page with hidesForSinglePage) never gets its recognizer or touch-action once pages arrive. Keeping the root mounted and hidden, or attaching through a callback ref, avoids it.
There was a problem hiding this comment.
Fixed in 4c6d228: the indicator row is an inner component rendered only when there are pages, so its recognizers mount with its element. Test: "taps work when the control first renders with no pages and the pages arrive later" (failed before).
| '@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.
AST-017 FR3 asks a Changeset for a changed default to name the prop or option that restores the old default; nothing restores the 768 px rule. This change is also separable from PageControl and could ship on its own.
There was a problem hiding this comment.
Superseded by the later review of the same file; routed (see the reply there).
4c1c60c to
f8153e0
Compare
There was a problem hiding this comment.
Approval is on hold for an engineering owner's decision: draft spec AST-094, the two new experimental core subpaths, the role-keyed theme motion with its platform adaptation condition, and the new adaptive presentation default for five menus and selectors have no current spec authority. 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 says which of these needs a different direction. Independently of that decision, the pull request carries about 30,700 authored lines from the whole stack and should land or split so it holds only the PageControl change, and it needs three fixes: tap and scrub input never attaches when PageControl first renders with count={0}, a page the caller declines is never requested again, and the adaptive changeset should name the option that restores the old default.
[Automated review]
| return getGestureHost(element.ownerDocument).attach(element, recognizer, { | ||
| manageStyle, | ||
| }); | ||
| }, [ref, recognizer, manageStyle]); |
There was a problem hiding this comment.
This effect reads ref.current once and never reruns when the element mounts later: PageControl returns null at count={0} (PageControl.tsx:608), so its recognizers never attach once pages arrive and touch input does nothing. Attach through a callback ref, or keep the node in state so the effect reruns (react-component-runtime INV6).
There was a problem hiding this comment.
Not this PR's code: useGestureRecognizer belongs to the gesture foundation below this commit, and attach-on-mount is routed to its owner. PageControl no longer depends on it (its row mounts with its element).
| const requestRef = useRef<{page: number; kind: ChangeKind} | null>(null); | ||
| const request = (page: number, kind: ChangeKind) => { | ||
| const latest = latestRef.current; | ||
| if (page === latest.current || page === requestRef.current?.page) { |
There was a problem hiding this comment.
A page the caller declines stays in requestRef, which is cleared only when current changes (line 586), so every later tap or key toward that page is dropped without calling onChange. Clear the pending request when the next render keeps the old value.
There was a problem hiding this comment.
Fixed in 4c6d228: a pending request lapses on the next task when the app keeps its value, so asking for the same page again calls onChange again. Test: "a page the app declines can be asked for again" (failed before).
| '@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.
AST-017 FR3 asks this changeset to name the prop or option that restores the old default. Name presentation="bottom-sheet" or "popover" as the migration for callers who need the old surface at a given size, or say that no option restores the 768px rule.
There was a problem hiding this comment.
Not this PR's code: the touch-presentation changeset belongs to the change below this commit in the stack, and is routed to its owner.
f8153e0 to
4c6d228
Compare
|
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. |
There was a problem hiding this comment.
Approval is on hold for an engineering owner's decision: draft spec AST-094, the PageControl touch behaviour it governs, the two new experimental core subpaths, the role-keyed theme motion and the platform adaptation condition have no current spec authority. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or says which of these needs a different direction. Independently of that decision, these need changes:
- The pull request carries about 31,000 authored lines from the whole open stack; land the predecessor pull requests or split it so it holds only the PageControl change.
- The adaptive menus' compact rule (below 744 px wide) should come from the theme's compact definition in current AST-012 FR9 (below the
mdpoint, 768 px by default, or 500 px tall), and its changeset should namepresentation="bottom-sheet"or"popover"as the way to keep a surface. useGestureRecognizershould attach when its element mounts late or is replaced.- Ten
[chore] Internalchangesets call code internal that this release exports through the experimental subpaths, and several say no component uses code PageControl now uses.
[Automated review]
| 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 AST-012 FR9 makes a screen compact below the theme's effective md point (768 px by default) or 500 px tall, and says every resolver uses the one definition the theme exports, with no size numbers of its own; DEC-6 rejects mapping compact to platform size classes. With 744 px, an iPad mini in portrait (744 by 1133) gets the popover where FR9 requires the sheet. main exports compactDefinition() from @astryxdesign/core/theme; building these queries from it (and testing 767/768 and 499/500) would conform.
| '@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.
AST-017 FR3 asks this changeset to name the option that restores the old behaviour, and AST-012's current-state impact says this release note names presentation as the way to keep a surface: callers who need the sheet or the popover at a given size pass presentation="bottom-sheet" or "popover". The numbers here also need to follow FR9 (below 768 px or 500 px tall).
| useIsomorphicLayoutEffect(() => { | ||
| const element = ref.current; | ||
| if (element == null) { | ||
| return undefined; | ||
| } | ||
| return getGestureHost(element.ownerDocument).attach(element, recognizer, { | ||
| manageStyle, | ||
| }); | ||
| }, [ref, recognizer, manageStyle]); |
There was a problem hiding this comment.
This reads ref.current once; with deps [ref, recognizer, manageStyle] it never reruns when the element mounts later or is replaced, so a consumer with a conditionally rendered target gets no recognizer, and a replaced node stays attached (react-component-runtime INV6). PageControl now sidesteps it, but the hook ships in @astryxdesign/core/gestures/experimental. A callback ref, or the node kept in state, would fix it.
|
Agreed: one implementation.
|
4c6d228 to
c4c8717
Compare
|
Pushed in c4c8717:
The touch layer is one module, |
github-actions
Bot
added
the
needs:design-review
c4c8717 to
c72f4af
Compare
There was a problem hiding this comment.
Approval is on hold for an engineering owner's decision: draft spec AST-094, the PageControl touch behaviour it governs, the two new experimental core subpaths, the role-keyed theme motion and the isRtlElement export from @astryxdesign/core/hooks have no current spec authority. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or says which of these needs a different direction. Independently of that decision, these need changes:
- The pull request carries about 32,000 authored lines from the whole open stack; land the predecessor pull requests or split it so it holds only the PageControl change.
- The adaptive menus' compact rule should follow the active theme's
mdpoint, as current AST-012 FR9 says, not only the default 768 px. - The
platformadaptation condition needs an AST-012 FR2 amendment or removal, and theastryxStylexoption belongs in its own pull request. useGestureRecognizershould attach when its element mounts late or is replaced, and the[chore] Internalchangesets,PageControl.doc.mjsand the touch-motion docs should describe what this release actually ships.
[Automated review]
| export const TOUCH_QUERY = '(pointer: coarse)'; | ||
|
|
||
| /** The viewport is compact: layout behaviours (`spec:AST-012` FR9). */ | ||
| export const COMPACT_QUERY = DEFAULT_COMPACT_DEFINITION.query; |
There was a problem hiding this comment.
These queries are fixed at the default md (768 px). Current AST-012 FR9 and public-component-api INV17 put compact below the effective md point, and the changeset promises the theme's md, so a theme with md: 1024 still gets popovers on a 900 px coarse tablet. Building them from compactDefinition() with the active theme's widthBreakpoints (through useThemeDefinition(), as AppShell reads them) and testing an overridden md would conform.
| * 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.
AST-012 FR2 keeps the when vocabulary closed: "Future environment fields require explicit additions to this closed vocabulary." platform needs that amendment first, or should go; PageControl no longer reads a platform.
| 'paging', | ||
| ], | ||
| description: | ||
| 'A row of page indicators under a paging view, as on iOS: one dot per page, the current one highlighted. A tap to either side of the current dot moves one page that way. Under a finger on iOS, sliding the finger across the dots scrubs straight to the page under it, and with more than ten pages the row is a window whose edge dots shrink to show there is more; elsewhere (Android, a mouse) a long row is cropped to the control. Experimental: lands in lab first.', |
There was a problem hiding this comment.
This still says the scrub and window are iOS-only and that Android gets the cropped row, but the head gives every coarse pointer the touch presentation (PageControl.tsx:53-56, and the test "an Android finger ... gets the window too").
There was a problem hiding this comment.
Fixed in 2bdd822: the description now says every touch device gets the scrub and the window (iOS and Android alike), and a mouse gets the cropped row.
| '@astryxdesign/cli': patch | ||
| --- | ||
|
|
||
| [feat] Themes can retune touch motion. `defineTheme({motion})` takes `springs`, `durations` and `curves` keyed by motion role, the table path of a value touch presentations play (`springs.sheetPresent`, `timing.reducedMotionCrossFadeMs`), at the root or in an adaptation rule. A role the theme leaves out keeps its measured default, and the duration scale (`fast`, `medium`, `ratio`) is optional when a theme sets only roles. Adaptation rules gain a `platform` condition, `ios` or `other`. `astryx docs motion` lists every motion role with its CSS variable and default, and the theme authoring guide and template describe the new fields. |
There was a problem hiding this comment.
timing.reducedMotionCrossFadeMs is no longer a role (reduced-motion-record.md says so and roles.test.ts:80 checks it), so a theme that follows this note writes a variable nothing reads. A real role, such as springs.sheetPresent, would make the example work.
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default.', |
There was a problem hiding this comment.
No core sheet, alert, menu, navigation or scroll view reads a motion role at this head; only the lab PageControl does, so the springs.sheetPresent example below changes nothing for a theme author today.
| @@ -247,9 +255,15 @@ export interface DefineThemeInput { | |||
| * // Snappy: { fast: 100, medium: 250, ratio: 0.75 } | |||
| * // Default: { fast: 175, medium: 410, slow: 975, ratio: 0.75 } | |||
| * // Cinematic: { fast: 200, medium: 500, slow: 1200, ratio: 0.7 } | |||
| * | |||
| * // A softer sheet and a slower reduced-motion fade: | |||
There was a problem hiding this comment.
navigation.transitionMs paces the navigation transition's completion callbacks (uikit/navigation.ts:55-61), and 300 is faster than its 350 ms default; it is not a reduced-motion fade, which is no longer themable.
| on: a transition that travels becomes a cross-fade carrying the same change | ||
| of state. An entrance lands in place and fades in, an exit fades out in | ||
| place, and any other move fades through. A navigation push or pop | ||
| cross-fades over 0.5 s on the ease-in-out curve; a sheet, a full-screen |
There was a problem hiding this comment.
Current design:reduced-motion DR1 and DR2 require opacity only, 200 ms or less, with no spring for core touch transitions; this 0.5 s cross-fade and "menus open and close on UIKit's Reduce Motion spring" contradict them, while the head's animator already follows the current record.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
spec:AST-090 is not in the repository on main or at this head, so this amendment (and the one in Current-state impact) points at nothing an owner can rule on.
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.
There was a problem hiding this comment.
Not ready to approve yet: PageControl's held look (the --color-overlay-pressed platter and 25 % dots after 155 ms, with taps read by its own recognizers) has no row in design:user-states and departs from spec:AST-094 FR11, and its window spring, slide delay and fades play table constants where FR4 asks for motion roles. An engineering owner needs to provide or identify the governing spec for both, recorded by commenting /approve-spec <full head SHA> on this pull request; the author may do so if they are an engineering owner. Whatever that decision, please also:
- Let the scrub (with its hold and tap) attach for every pointer, as
spec:AST-094FR43 requires: today a mouse or trackpad drag across the dots pages nothing. - Re-export
animatorForandreducedMotionFadethrough@astryxdesign/core/internal/touch, the routearchitecture:react-component-runtimeINV13 names, or have INV13 amended. - Land #8164 and its stack first so this diff is only PageControl's own change, and split or justify that change if it stays over 2,000 lines.
- Add real-device iOS Safari and Android Chrome recordings, which
spec:AST-094's Platform support requires for the doc's iOS and Android claim.
[Automated review]
| break; | ||
| } | ||
| }, | ||
| {isEnabled: isTouch}, |
There was a problem hiding this comment.
spec:AST-094 FR43 says gestures attach for every pointer (a mouse, a pen and a finger), and its verification fails "a gesture missing under a mouse or pen". With isEnabled: isTouch on the pan (and on the hold and tap at lines 350 and 361), a mouse or trackpad drag across the dots pages nothing: the same 60 px sweep that scrubs a finger from Page 3 to Page 5 leaves a mouse on Page 3. Attach these for every pointer and drop the 45 px mouse rule (PageControl.tsx:339-348), or have an engineering owner amend FR43 for PageControl. The comment at PageControl.tsx:324-329 still cites FR19 and FR24 for the fine-pointer path, but FR19 now attaches recognizers for every pointer and FR24 budgets a fine pointer's gesture listeners rather than forbidding them.
| MotionValues, | ||
| SpringTransition, | ||
| } from '../motion/animator'; | ||
| export {animatorFor} from '../motion/animator'; |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 lists the animator among Core's touch modules and says "Core and Lab components reach them through @astryxdesign/core/internal/touch". Re-exporting animatorFor here (and reducedMotionFade at line 39) gives Lab a second route to a touch module; re-export both from internal/touch instead, or have INV13 amended.
| 'paging', | ||
| ], | ||
| description: | ||
| 'A row of page indicators under a paging view, as on iOS: one dot per page, the current one highlighted. A tap to either side of the current dot (a click with a mouse) moves one page that way. Under a finger (any touch device, iOS and Android alike), sliding the finger across the dots scrubs straight to the page under it, and with more than ten pages the row is a window whose edge dots shrink to show there is more; with a mouse a long row is cropped to the control. Experimental: lands in lab first.', |
There was a problem hiding this comment.
This tells consumers the scrub and window work on "any touch device, iOS and Android alike". spec:AST-094's Platform support requires real-device evidence for every claim about iOS Safari and Android Chrome behaviour; the recordings so far are simulator and emulator only. Please add real-device recordings of the tap, scrub and jog, window slide and fades.
|
On #8164. 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@5fa39ee...7e6276b
User need
UIPageControl does more than step: a sliding finger scrubs, a long control shows a window with shrinking edge dots, and the current dot fades. #8164's PageControl only steps.
Evidence
Recorded on the iOS 26.2 simulator (P11: the Android 15 emulator), cropped. Red ring: the finger; the "JS thread" bar stops while JS blocks.
Problem and solution fit
Measured on iOS 26.2 (the
pageControltable), under a finger; with a mouse, #8164's clicks, keys and cropped row stay:onChangeeach time it changes. Past a windowed control's edge it keeps stepping at UIKit's measured rates, slower toward the first page. A swipe that starts vertical stays the page's scroll (FR19).::beforeplatter shows the pressed overlay and the other dots dim to 25 %.Current specification
spec:AST-094FR1–FR8, FR13; the platter followsdesign:user-statesDR1. Reduce Motion followsdesign:reduced-motion.Public delta and caller burden
No API change. During a scrub
onChangefires each time the page under the finger changes.Design and alternatives
Scope
Testing
pnpm exec vitest run packages/lab/src/PageControl packages/core/src/motion: all passed (40 PageControl cases, including, failing before, "under a finger, a vertical swipe that starts on the dots pages nothing and leaves every move to the page's scroll" (its moves were cancelled), "a run toward the first page steps at the measured 10 pages a second 77 pt past the edge" (13.3 before), "a page the app declines can be asked for again", "server-rendered, a long control already carries its resting offset and sizes", "under a finger and Reduce Motion, every opacity change fades in 200 ms on easeInOut" and "an Android finger gets the window too"). New with feat(core): the internal touch foundation: press tracking and the animator #8076 07aa9c0: "with a mouse, a click … asks for the next page, on the leading side for the previous", "a press that travels more than 45 px is a drag and pages nothing", "under a finger, the click the browser sends after a tap does not page again", and "a--motion-page-control-…variable changes nothing".pnpm test:interaction packages/lab/src/PageControl: Chromium 22 passed (twice); WebKit 21 passed, 1 skipped (screencast clock), three runs. New: "a vertical swipe that starts on the dots pages nothing and leaves every move to the page's scroll" (no move cancelled in either engine; the page scrolls in Chromium, where injected touches scroll natively; before, WebKit's page stayed at scrollY 0). The held-finger test checks the::beforeplatter resolves to--color-overlay-pressed. Also "under Reduce Motion nothing slides or scales, and no fade runs over 200 ms on a coarse pointer", "on an Android phone (no -webkit-touch-callout) a finger scrubs", and a mouse block (hasTouch: false, asserting the pointer is fine): the cropped row and round dots.CI=true pnpm lint, Prettier, changeset checks and coverage,check:repo, Storybook and sandbox builds: clean.registry-contract (preview)locally (generate-data.mjs, thenverify-full-catalog.mjs): 1,036 items and 1,040 routes verified in TypeScript and JavaScript.scripts/lab-core-imports.test.mjs,scripts/core-internal-entries.test.mjs,scripts/touch-support-bundle.test.mjsandmotion/uikit/tableImports.test.tspass.PageControlTouch.ts(512 lines), imported once by PageControl.tsx. PageControl's import (built lab, gzip,.github/scripts/lib/component-size.js): JavaScript 36,746 → 51,794 B, CSS 488 → 709 B.