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.
|
fe670fe to
1426e56
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved. PageControl has no governing spec, and the draft spec:AST-094 it builds on is unapproved. An engineering owner needs to approve or amend PageControl's API, its one-page-per-tap rule and its slider semantics, together with the draft, by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Whatever that decision, these changes are still needed:
- Land the stacked pull requests (#8045, #8052, #8062, #8074, #8076) on their own and rebase, so this one carries only the PageControl commit instead of about 27,000 changed lines.
- Attach the tap recognizer whenever the control's root mounts: after a first render with no pages, or after
hidesForSinglePagehides and shows it, taps currently do nothing. - Compose a caller's
onKeyDownwith the built-in handler instead of replacing it. - Raise the light-mode non-current dots to at least 3:1 against the page background (they measure 2.97:1).
[Automated review]
| if (pages === 0 || (hidesForSinglePage && pages === 1)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This null return unmounts the root that useGestureRecognizer attached to, and the hook attaches only once (useGestureRecognizer.ts:73-81, deps [ref, recognizer, manageStyle]). A control that first renders with count={0}, or is hidden by hidesForSinglePage and later shown, gets a root with no tap recognizer: taps do nothing, and only the keyboard works. Rendering an inner component only when there are pages lets the hook mount with its element.
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in cf0ea9d (see the reply there).
| current: current + 1, | ||
| total: pages, | ||
| })} | ||
| onKeyDown={onKeyDown}> |
There was a problem hiding this comment.
htmlProps (spread at line 235) can carry a caller's onKeyDown, which BaseProps accepts, and this line replaces it. Consider onKeyDown={composeEventHandlers(onKeyDownProp, onKeyDown)} from @astryxdesign/core/utils, so the caller's handler runs and can opt out through preventDefault().
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in cf0ea9d (see the reply there).
| opacity: | ||
| page === current | ||
| ? PC.currentIndicatorAlpha | ||
| : PC.otherIndicatorAlpha, |
There was a problem hiding this comment.
In light mode, --color-text-primary (#0A1317) at 0.45 measures 2.97:1 on --color-background-body (#F1F4F7), under the 3:1 WCAG 2.2 AA asks for these dots, which are the only cue for how many pages exist. 0.48 or more reaches 3.25:1.
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in cf0ea9d (see the reply there).
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 |
1426e56 to
8018ed2
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the direction this pull request proposes: draft AST-094, the new Lab PageControl (its API, one-page tap rule and slider semantics), and the experimental gesture and motion subpaths that no component owns. 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. Whatever that decision, these changes are still needed:
- Land or retarget onto the pull requests this one depends on, so it carries only the PageControl commit instead of 29,292 authored lines.
- Attach the tap recognizer when the control mounts after rendering nothing (
count={0}, or one page withhidesForSinglePage), because today taps never work after the count grows. - Compose a caller's
onKeyDownwith the built-in keys and acceptref. - Bring inactive dots to 3:1 on Core's default page background, where they measure 2.97:1.
- Move the
platformadaptation condition, the theme motion roles and the adaptive-threshold change into their own pull requests:platformneeds an amendment to AST-012's closed condition list, the motion roles contradict this draft's FR4, and the threshold move changes released menus and selectors on portrait tablets and landscape phones.
[Automated review]
| if (pages === 0 || (hidesForSinglePage && pages === 1)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
When the control first renders nothing here (count={0}, or one page with hidesForSinglePage), useGestureRecognizer has already run its attach effect against a null ref and never runs it again. Once pages arrive, taps do nothing (touch-action stays auto). Keep the root mounted and hidden, or make the hook attach when the node appears, and add a test that mounts with count={0} and then grows.
There was a problem hiding this comment.
Fixed in cf0ea9d: the indicator row is an inner component rendered only when there are pages, so its tap recognizer mounts with its element. Test: "taps work when the control first renders with no pages and the pages arrive later" (failed before).
| 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 effect is keyed on the ref object, which never changes, so a node that mounts after the first commit, or replaces the first one, is never attached. A callback ref, or node state the effect depends on, would cover attachment, null detachment and replacement. usePressTracker below has the same shape.
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; see the reply on PageControl.tsx).
| {...htmlProps} | ||
| {...mergeProps(stylex.props(styles.root, xstyle), className, { | ||
| ...style, | ||
| width: layout.width, | ||
| height: PC.height, | ||
| paddingInline: PC.horizontalPadding, | ||
| })} | ||
| role="slider" | ||
| tabIndex={0} | ||
| aria-label={label ?? t('@astryx.pagination.pageIndicators')} | ||
| aria-orientation="horizontal" | ||
| aria-valuemin={1} | ||
| aria-valuemax={pages} | ||
| aria-valuenow={current + 1} | ||
| aria-valuetext={t('@astryx.pagination.pageOfTotal', { | ||
| current: current + 1, | ||
| total: pages, | ||
| })} | ||
| onKeyDown={onKeyDown}> |
There was a problem hiding this comment.
{...htmlProps} forwards a caller's onKeyDown, then this line replaces it, so the caller never hears the key and cannot cancel paging. Call the caller's handler first and skip paging when it calls preventDefault(). Also accept ref and merge it with rootRef.
There was a problem hiding this comment.
Fixed in cf0ea9d: a caller's onKeyDown runs first (composeEventHandlers) and its preventDefault() keeps the page; ref is accepted and merged with the internal ref. Tests: "a caller's onKeyDown runs first and can keep the page with preventDefault" and "forwards ref to the slider, and taps still page with a ref passed" (both failed before).
| opacity: | ||
| page === current | ||
| ? PC.currentIndicatorAlpha | ||
| : PC.otherIndicatorAlpha, |
There was a problem hiding this comment.
Inactive dots are the primary text colour at 45 %. With Core's default tokens that is 2.97:1 on --color-background-body, under the 3:1 that WCAG 1.4.11 asks for. An alpha of 0.48 gives 3.25:1 there, or use a token pair that holds 3:1 on page and card surfaces.
There was a problem hiding this comment.
Fixed in cf0ea9d: inactive dots are at 48 % (UIKit's 45 % raised), 3.25:1 on Core's page background. Test: "inactive dots reach 3:1 on Core's default page background" (failed before at 2.97:1).
| * 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 adds a fifth field to the adaptation condition vocabulary, which AST-012 FR2 keeps closed: "Future environment fields require explicit additions to this closed vocabulary." It needs that amendment in its own pull request, or removal here.
There was a problem hiding this comment.
Not this PR's code: the platform adaptation condition belongs to the theme foundation below this commit, and is routed to its owner.
8018ed2 to
cf0ea9d
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 until an engineering owner approves or amends the governing spec for what this pull request adds: draft AST-094, the new Lab PageControl (its API, one-page tap rule and slider semantics), and experimental gesture and motion subpaths that no component owns. 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 latest PageControl fixes for taps after a hidden mount, a caller's onKeyDown, ref and inactive-dot contrast look right, and these changes are still needed whatever that decision:
- Land or retarget onto the pull requests this one depends on, so it carries only the
PageControlcommit instead of 29,487 authored lines. - Build the compact-touch query from
compactDefinition(), because current AST-012 FR9 puts compact at width belowmd(768 px) or height below 500, so 744 px sends coarse-pointer screens 744-767 px wide to a popover instead of the sheet, and namepresentationas the way back in its changeset. - Move the
platformadaptation condition, which needs an AST-012 FR2 amendment, and the theme motion roles, which contradict this draft's FR4, into their own pull requests. - Make
useGestureRecognizerandusePressTrackerattach elements that mount late or are replaced.
[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 (and public-component-api INV17) puts compact at width below the theme's effective md (768 by default) or height below 500, with no resolver keeping numbers of its own. main now exports compactDefinition() from @astryxdesign/core/theme; building these queries from its query and coarsePointerQuery would keep coarse-pointer screens 744-767 px wide, such as an iPad mini in portrait, on the bottom sheet FR9 asks for.
| '@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 restoring input as well as the old and new default: for a touch presentation default, the component's presentation value that keeps the previous surface (presentation="popover" or "bottom-sheet").
|
Agreed: one implementation.
|
cf0ea9d to
c53fc2a
Compare
github-actions
Bot
added
the
needs:design-review
c53fc2a to
f08fe67
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner approves or amends the governing spec for what this pull request adds: draft AST-094, the new Lab PageControl (its API, one-page tap rule and slider semantics), the experimental gesture and motion subpaths that no component owns, and the stable isRtlElement export from @astryxdesign/core/hooks. 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. Independent of that decision, these changes are needed:
- Land the lower pull requests first or retarget onto them, so this one carries only the
PageControlcommit; compared withmainit is about 30,200 authored lines. - Remove the
platformadaptation condition, which AST-012 FR2's closed condition vocabulary does not allow, or propose it with an FR2 amendment in its own pull request. - Make draft AST-094 FR8 follow design:reduced-motion (opacity only, 200 ms or less, no springs), and drop
timing.reducedMotionCrossFadeMsfrom the theme release note, since that role is excluded and a theme that sets it changes nothing. - Build the touch resolver's compact query from the active theme's
mdpoint, and makeuseGestureRecognizerandusePressTrackerattach elements that mount late or are replaced.
[Automated review]
| /** | ||
| * The platform's web engine: `ios` is iOS and iPadOS WebKit (every browser | ||
| * there), detected with `@supports (-webkit-touch-callout: none)`; `other` | ||
| * 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 adds a field to the adaptation condition vocabulary, which AST-012 FR2 keeps closed (width, height, pointer, contrast and motion): "Future environment fields require explicit additions to this closed vocabulary." It needs that FR2 amendment in its own pull request, or removal here; PageControl does not use it.
| - **FR8 — Reduce Motion substitutes, never jumps.** Under | ||
| `prefers-reduced-motion: reduce`, a touch presentation MUST behave as its | ||
| UIKit counterpart does with Reduce Motion and Prefer Cross-Fade Transitions | ||
| 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 | ||
| modal or an alert fades on its own presentation spring and drops its travel | ||
| and scale; menus open and close on UIKit's Reduce Motion spring. Motion that | ||
| follows a finger, such as a throw, MUST keep its motion. **Proposed; pending | ||
| owner review:** this is the minimally moving form the draft `design:motion` | ||
| DR5 asks for, for touch presentations (owners: ernestt, cixzhang); the | ||
| alternative is UIKit's Reduce Motion alone, which keeps the slides of | ||
| navigation, sheets and modals. |
There was a problem hiding this comment.
The current design:reduced-motion record requires an opacity-only change that finishes in 200 ms or less on a coarse pointer (DR2) and rules out springs and longer animations (DR4). A 0.5 s push cross-fade and menus on a Reduce Motion spring contradict both, and the animator in this stack already follows DR2. Consider having FR8 defer to that record.
| '@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 excluded from the theme roles (uikit/timing.ts:158, and .changeset/reduced-motion-record.md says it "is no longer a theme role"). Because expandMotionRoles accepts any well-formed id, a theme that follows this example writes a variable nothing reads, with no error. Use a registered role here, such as springs.navigationPush.
| @@ -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 times the normal push and pop; under Reduce Motion the fade is a fixed 200 ms that no theme can lengthen (design:reduced-motion DR4). "A slower reduced-motion fade" describes something this example cannot do.
| export const COMPACT_QUERY = DEFAULT_COMPACT_DEFINITION.query; | ||
|
|
||
| /** Both: presentations such as a sheet in place of a popover. */ | ||
| export const COMPACT_TOUCH_QUERY = | ||
| DEFAULT_COMPACT_DEFINITION.coarsePointerQuery; |
There was a problem hiding this comment.
These queries come from DEFAULT_COMPACT_DEFINITION, so a theme that overrides md still switches at 768, while AST-012 FR9 asks for "the effective md point" (its verification lists "overridden md") and the changeset promises "the theme's md point". AppShell already reads the effective width points from Theme context; compactDefinition() takes them.
| 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 effect is keyed on the ref object, which never changes, so an element that mounts after the first commit, or replaces the first one, is never attached (react-component-runtime INV6: a callback ref or other node owner handles attachment, null detachment and replacement). usePressTracker below has the same shape. A callback ref, or node state the effect depends on, would cover it; a test that mounts the element late would pin it.
meta#8076's new head carries main 70ea859 with meta#8073's writer of the press attribute on main's press controller, and its README names both writers. utils/pressFeedback.ts keeps that controller with this line's own changes on it: the pointer, drag, scroll and visibility listeners through the shared input stream (meta#8356) and touchcancel only while the primary pointer is coarse (meta#8270); the key path stays at the window's bubble phase, so a Space press the page cancels paints nothing. usePressTracker stays in its own module, one tracker for a component's life (meta#8315), so meta#8073's rebuilt-tracker hook and its test are not taken. The harness files are meta#8076's.
…s, and the action-group tracker writes its own The header and writePress called it the one writer of the press attribute, but main's action-group tracker writes its groups' presses itself. They now say what holds, as meta#8076's gestures README does.
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for what this pull request adds: a new Lab PageControl beside Pagination's variant="dots", its one-page tap rule for a finger and for a mouse click, and its slider semantics. 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 the three must change. Whatever the decision, these need changes:
- Land #8076 and #8562 first, or rebase once they land, so this diff carries only PageControl's 1,288 authored lines instead of 28,846.
- Make the gesture host attach for every pointer, as
spec:AST-094FR19 and FR43 now state, and let PageControl's tap recognizer page a mouse instead of a separate click path. - Give the control FR42's coarse-only 44 px hit area, and take the windowed row's shift from the row's own direction (
PageControl.tsx:144,design:bidirectionalityDR3). - Attach real-device iOS Safari and Android Chrome evidence for the phone and VoiceOver claims, and name Carousel's
hasSnapinstead ofhasPagingin the usage doc.
[Automated review]
| * - Recognizers attach, and the host registers listeners, only while the | ||
| * primary pointer is coarse (`spec:AST-094` FR19, FR24; coarsePointer.ts). | ||
| * Where it is fine, an attach is recorded but nothing is installed: no |
There was a problem hiding this comment.
This keeps recognizers and listeners coarse-only, but current spec:AST-094 FR19 says "Recognizers attach for every pointer, with FR43's exceptions", FR43 says "Gestures attach for every pointer, a mouse, a pen, and a finger, not only a coarse primary pointer", and architecture:react-component-runtime INV13 says "The gesture host attaches for every pointer, with no touch-only query". Attaching under a fine pointer, with FR43's exceptions and FR24's fine-pointer budget, and dropping the warning at :1354-1366 would follow them.
| * finger lifts, as UIPageControl reports it. Where the primary pointer is | ||
| * coarse the gesture system's tap recognizer reads the touch; where it is | ||
| * fine the gesture host listens to nothing (`spec:AST-094` FR19, FR24), so a | ||
| * click, a mouse's or a touchscreen finger's, pages by the same rule. The app |
There was a problem hiding this comment.
spec:AST-094 FR19 now reads "Recognizers attach for every pointer, with FR43's exceptions", so once the host follows it a mouse pages through the tap recognizer, and the separate click path at :239-266 (and this FR19 citation) would go. Until then every desktop development page logs the host's "attached where the primary pointer is fine" warning, since this control attaches under every pointer.
On #8360 (carrying #8076). 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@53eaf7d...5fa39ee (#8129 adds the same devDependency).
User need
A phone's paging view shows the page with UIPageControl's dots; a tap beside the current one moves one page. Pagination's dots are desktop-sized and jump to the clicked one.
Evidence
Recorded on the iOS 26.2 simulator; P8 on the Android 15 emulator. Red ring: the finger.
Chromium: 9.67 px boxes paint 9 or 10 px wide; scaled 10 px boxes stay round.
Problem and solution fit
pageControltable, new): 9.67 px indicators, 8 px apart, 25.67 px tall, at most 161.3 px wide; past that the row shifts to keep the current one visible. Current opaque, others 48 % (UIKit's 45 % raised to 3:1), in the text colour.slider, "Page indicators", reading "Page 3 of 5"; arrows, Home and End change the page.Current specification
spec:AST-094FR1–FR2: sourced geometry; themes keep colour. Proposed; pending owner review:slidersemantics.Public delta and caller burden
A new lab
PageControl(inastryx discover; one ShadCN route) and core's internalpageControltable (motion/uikit; Lab reaches it through@astryxdesign/core/internal/motion, and Core'sisRtlElementthrough/internal/touch). The app owns the page:Design and alternatives
Scope
Testing
pnpm exec vitest run packages/lab/src/PageControl packages/core/src/motion: all passed (24 PageControl cases: geometry against the measured 108.3 px and 161.3 px, the window, the tap rule, slider semantics, keyboard and RTL). Failing at the feat(core): the internal touch foundation: press tracking and the animator #8076 merge (its gesture host listens only under a coarse pointer), now passing: "with a fine primary pointer, a click pages by the same rule", "a press that travels more than 45 px is a drag and pages nothing", "a caller's onClick runs first", and "under a finger, a tap … asks for the next page, once" (the click after it does not page again).uikit.test.tschecks every new entry's source.pnpm test:interaction packages/lab/src/PageControl: Chromium 8 passed, WebKit 8 passed, with an asserted tap storyboard, "on an Android phone (no -webkit-touch-callout) the control pages the same" and "every indicator paints round, as wide as it is tall" (5 and 50 pages, from a decoded screenshot).pnpm a11y:audit -- --components lab/PageControl: 0 violations.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..github/scripts/lib/component-size.js): JavaScript 36,746 B, CSS 488 B.Stack: #8360 (carrying #8076) and #8562, then this PR.