Repository navigation
feat(SegmentedControl): the selection slides on every change and drags with any pointer - #9632
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:design-review
github-actions
Bot
added
the
needs:spec-owner-review
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| SegmentedControl | 🔴 81.6 kB (+44.8 kB) · 7.8 kB CSS (+1.3 kB) | — | ✓ · ✓ · ✓ |
| DateTimeInput | 🔴 158.4 kB (+43.3 kB) · 17.0 kB CSS (+1.3 kB) | — | — · — · — |
| lab/MobilePowerSearch | 🔴 213.4 kB (+44.6 kB) · 1.2 kB CSS (0) | — | — · — · — |
58 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| SegmentedControlItem | 10.3 kB (+54 B) · 4.0 kB CSS (+10 B) | — | ✓ · ✓ · ✓ |
| Switch · Storybook | 86.7 kB (+46 B) · 10.5 kB CSS (0) | 1,719 (−61) | ✓ · ✓ · ✓ |
| CheckboxListItem | 53.4 kB (+38 B) · 11.0 kB CSS (0) | — | — · — · — |
| ListItem | 52.8 kB (+35 B) · 11.0 kB CSS (0) | — | — · — · — |
| RadioListItem | 51.3 kB (+35 B) · 10.1 kB CSS (0) | — | — · — · — |
| ContextMenu | 75.0 kB (+33 B) · 15.3 kB CSS (0) | — | — · — · — |
| Table | 84.3 kB (+33 B) · 16.0 kB CSS (0) | — | — · — · — |
| TableCell | 76.2 kB (+31 B) · 15.8 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 75.8 kB (+31 B) · 15.5 kB CSS (0) | — | — · — · — |
| Slider | 88.5 kB (+29 B) · 9.4 kB CSS (0) | — | — · — · — |
| lab/TransferList | 74.4 kB (+28 B) · 1.7 kB CSS (0) | — | — · — · — |
| Markdown | 145.1 kB (+25 B) · 18.3 kB CSS (0) | — | — · — · — |
| Overlay | 19.8 kB (+25 B) · 4.0 kB CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 65.1 kB (+25 B) · 773 B CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 65.6 kB (+25 B) · 1.1 kB CSS (0) | — | — · — · — |
| charts/Chart | 62.9 kB (+24 B) · 20 B CSS (0) | — | — · — · — |
| MoreMenu | 73.3 kB (+24 B) · 15.4 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 102.4 kB (+24 B) · 1.9 kB CSS (0) | — | — · — · — |
| lab/ListInput | 81.5 kB (+24 B) · 1.6 kB CSS (0) | — | — · — · — |
| BreadcrumbItem | 67.1 kB (+22 B) · 11.4 kB CSS (0) | — | — · — · — |
| SideNavItem | 54.7 kB (+22 B) · 10.4 kB CSS (0) | — | — · — · — |
| lab/ThreeDChart | 4.9 kB (+22 B) · 20 B CSS (0) | — | — · — · — |
| lab/TourStep | 55.4 kB (+22 B) · 590 B CSS (0) | — | — · — · — |
| DropdownMenuRadioItem | 18.2 kB (+21 B) · 5.9 kB CSS (0) | — | — · — · — |
| Popover | 50.1 kB (+21 B) · 12.4 kB CSS (0) | — | — · — · — |
| ProgressBar | 10.8 kB (+21 B) · 7.3 kB CSS (0) | — | — · — · — |
| Selector | 76.9 kB (+20 B) · 15.7 kB CSS (0) | — | — · — · — |
| SelectorOption | 16.5 kB (+20 B) · 5.4 kB CSS (0) | — | — · — · — |
| TimeInput | 90.1 kB (+20 B) · 14.3 kB CSS (0) | — | — · — · — |
| BottomSheet | 35.1 kB (+19 B) · 7.5 kB CSS (0) | — | — · — · — |
| ChatLayout | 48.5 kB (+19 B) · 10.3 kB CSS (0) | — | — · — · — |
| DropdownMenuSubMenu | 56.0 kB (+19 B) · 9.9 kB CSS (0) | — | — · — · — |
| MultiSelector | 72.7 kB (+19 B) · 15.2 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 226.9 kB (+19 B) · 2.4 kB CSS (0) | — | — · — · — |
| Item | 13.3 kB (+18 B) · 4.9 kB CSS (0) | — | — · — · — |
| AlertDialog | 56.2 kB (+17 B) · 12.7 kB CSS (0) | — | — · — · — |
| DropdownMenuCheckboxItem | 18.2 kB (+17 B) · 5.9 kB CSS (0) | — | — · — · — |
| DropdownMenuItem | 17.4 kB (+17 B) · 5.5 kB CSS (0) | — | — · — · — |
| TopNavMegaMenu | 54.4 kB (+17 B) · 11.2 kB CSS (0) | — | — · — · — |
| lab/Comments | 232.2 kB (+17 B) · 2.5 kB CSS (0) | — | — · — · — |
| lab/TransferListSelector | 80.8 kB (+17 B) · 1.9 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 221.7 kB (+17 B) · 454 B CSS (0) | — | — · — · — |
| AppShell | 68.5 kB (+15 B) · 12.2 kB CSS (0) | — | — · — · — |
| PowerSearch | 184.0 kB (+15 B) · 20.6 kB CSS (0) | — | — · — · — |
| TopNavMenu | 58.3 kB (+15 B) · 10.5 kB CSS (0) | — | — · — · — |
| DropdownMenu | 73.0 kB (+14 B) · 15.4 kB CSS (0) | — | — · — · — |
| SideNavHeading | 58.8 kB (+14 B) · 10.6 kB CSS (0) | — | — · — · — |
| Tokenizer | 70.1 kB (+14 B) · 17.1 kB CSS (0) | — | — · — · — |
| lab/MobileTokenizer | 100.9 kB (+14 B) · 880 B CSS (0) | — | — · — · — |
| LayerProvider | 62.7 kB (+13 B) · 10.5 kB CSS (0) | — | — · — · — |
| Pagination | 85.9 kB (+13 B) · 16.0 kB CSS (0) | — | — · — · — |
| ContextMenuItem | 52.6 kB (+12 B) · 9.9 kB CSS (0) | — | — · — · — |
| Toast | 59.4 kB (+9 B) · 10.1 kB CSS (0) | — | — · — · — |
| TopNavHeading | 58.2 kB (+9 B) · 10.4 kB CSS (0) | — | — · — · — |
| DateInput | 103.3 kB (+6 B) · 15.3 kB CSS (0) | — | — · — · — |
| Collapsible | 22.5 kB (+1 B) · 4.5 kB CSS (0) | — | — · — · — |
| SideNav | 65.7 kB (0) · 10.9 kB CSS (0) | — | — · — · — |
| TreeList | 37.9 kB (0) · 5.0 kB CSS (0) | — | — · — · — |
Package size: @astryxdesign/core 6.08 MB packed (+44.2 kB); no new dependencies.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
|
/approve-spec 01a3a43 |
github-actions
Bot
removed
the
needs:spec-owner-review
|
AI review status for this pull request.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner: no current record gives SegmentedControl the selection slide, the drag-to-select, or the lift commit for a press that began on another segment, and the clauses for them are still in the open draft #8596, which also differs from this head (it keeps a mouse unchanged and changes label weights when a drag starts). An engineering owner can settle this by landing #8596, or folding its clauses into this pull request, and recording the decision with /approve-spec <full head SHA> on the new head; the author may do this if they are an engineering owner. Whatever that decision, please also:
- Reuse
actionGroupTracker's click-and-report and click-swallow helpers instead of the copies inuseSegmentedTouchTracking.ts. - Remove the press-gap statements both SegmentedControl records still carry in their at-a-glance and current-state sections.
- Move the animator attach and the resize observer in
useSegmentedSelectionMotion.tsinto a callback ref. - Narrow the title to what ships for every pointer (the drag), and split or justify the 6,966 authored lines; the owner-directed batch cap applies only once a current record settles the behaviour.
[Automated review]
| function clickAndReport(segment: HTMLElement): boolean { | ||
| const view = segment.ownerDocument.defaultView; | ||
| let clicked: Event | null = null; | ||
| const grab = (event: Event) => { | ||
| if (clicked == null && event.target === segment) { | ||
| clicked = event; | ||
| } | ||
| }; | ||
| view?.addEventListener('click', grab, true); | ||
| try { | ||
| segment.click(); | ||
| } finally { | ||
| view?.removeEventListener('click', grab, true); | ||
| } | ||
| return (clicked as Event | null)?.defaultPrevented === true; | ||
| } |
There was a problem hiding this comment.
This clickAndReport is a line-for-line copy of the private one in hooks/actionGroupTracker.ts (lines 160-175), and swallowLateClick/isLateClick above re-implement the tracker's swallowNextClick, which this control already arms for drags through swallowsNativeClick: true. A fix to one swallower (such as the touchstart disarm only this copy has) would miss the other. Could the tracker own the tap path (wait for the browser's click, else click and swallow), so this hook makes one call and these copies go?
There was a problem hiding this comment.
Done in 90d5552. The action-group tracker now owns the tap path (awaitBrowserClick, swallowNextClick, clickAndReport, isNativeClick), and this hook makes one call. The touchstart disarm is in the tracker's swallower, so both callers get it.
| useIsomorphicLayoutEffect(() => { | ||
| if (!isEnabled) { | ||
| return undefined; | ||
| } | ||
| motion.attachTrack(trackRef.current); | ||
| return () => motion.attachTrack(null); | ||
| }, [isEnabled, motion, trackRef]); |
There was a problem hiding this comment.
Attaching the animators here, and starting the resize observer in the Effect below, is work caused by the radiogroup attaching. SegmentedControl.tsx already merges refs, so a callback ref returned by this hook could attach both, detach on null and handle a replaced node (react-component-runtime INV3, INV6).
There was a problem hiding this comment.
Done in 90d5552. The hook returns attachTrack, a callback ref that attaches the animators and the resize observer. Its cleanup unobserves, hands the surface back and detaches, on unmount, a disable or a replaced node. SegmentedControl merges it with its other refs.
| // The center chases the finger, and the size stays at rest. Each chase | ||
| // starts explicitly at the call. A call from rest otherwise waits for the | ||
| // browser to start it. Android Chrome sends one touch move per frame, so | ||
| // the next move arrived before that start was known. Every move then | ||
| // restarted the chase from rest, and the lens stuck to its segment. |
There was a problem hiding this comment.
Non-blocking (comment style, knowledge-contracts INV29): this narrates a past bug over five sentences. Suggested: "Start each chase at the call. A chase from rest waits for the browser to start it. Android Chrome sends the next move first, so the lens would stick."
| | FR1 | The radiogroup container carries the current `segmented-control` target with `data-size`, sets `aria-label` to `label`, and renders its supplied children. | Current source, docs, and tests | | ||
| | FR2 | The group is one Tab stop: the selected segment, or, when the value matches no segment, the first segment not disabled by its own `isDisabled` (in a disabled group that segment keeps the stop, FR7); a disabled selected segment MUST keep the Tab stop (`spec:AST-085` FR1). ArrowLeft and ArrowRight move focus between enabled segments in the visual direction, flipping in right-to-left layouts (`design:bidirectionality` DR6), and wrap; Home and End jump to the ends; ArrowUp and ArrowDown do nothing. Disabled segments are skipped. | WAI-ARIA radio group pattern; current tests; `design:bidirectionality` DR6 | | ||
| | FR3 | When arrow, Home, or End navigation moves focus to an enabled segment, that segment's value is passed to `onChange` if it differs. A pointer press MUST select only on a click whose lift lands inside the segment (`spec:AST-020` FR9, DEC-5). Implementation gap: while focus is already in the group, a pointer press on another segment moves focus there and selects before any click, so a press that slides off still selects. Focus entering the group from outside never changes the value. | WAI-ARIA radio group pattern; current tests; `spec:AST-020` FR9, DEC-5 | | ||
| | FR3 | When arrow, Home, or End navigation moves focus to an enabled segment, that segment's value is passed to `onChange` if it differs. A pointer press MUST select only on a click whose lift lands inside the segment (`spec:AST-020` FR9, DEC-5). Focus entering the group from outside never changes the value. | WAI-ARIA radio group pattern; current tests; `spec:AST-020` FR9, DEC-5 | |
There was a problem hiding this comment.
FR3 drops the gap here, but the at-a-glance row (line 68) still says "A press while focused selects before any click" and the Current-state impact (lines 233-235) still lists "Press while focused (FR3)". SegmentedControlItem.spec.md lines 58 and 185-186 still say onClick cannot veto such a press. Please remove those so the records match this change.
There was a problem hiding this comment.
Done in 90d5552: removed from both records (the glance rows, the Item record's ORD1, and both current-state sections).
cixzhang
added
the
ci:run
01a3a43 to
90d5552
Compare
vjeux
changed the title
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 90d5552 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval waits on an engineering owner for one addition: useMediaQuery now returns its server default where window.matchMedia is missing, instead of throwing, and no spec covers that fallback in the public hook. An engineering owner can settle it by identifying or adding the governing spec (or asking for the change to be split out), confirm whether this pull request counts as the owner-directed batch for its 7,168 authored lines, and record the decision with /approve-spec <full head SHA>; the author may do this if they are an engineering owner. The slide, drag and lift-commit clauses are already owner-approved at this head.
Whatever that decision, please also:
- Amend
spec:AST-094FR11, which still limits lift activation to action-group members and the three slide-off toggles, so that it covers SegmentedControl's drag commit at the lift. - Reword
component:SegmentedControlFR3, which still says the lift must land inside the segment, to match FR13's nearest-segment commit (padding and gaps included), and update its Compatibility row. - Fix
SegmentedControl.doc.mjs(all three locales) and theSelectionMotionstory, which say a mouse or pen click keeps the colour fade, although every click now slides on the theme's timing. - Reuse
parseMotionRoleValue(extended with seconds and keyword curves) instead of the second CSS timing parser inuseSegmentedSelectionMotion.ts. - Drop the layout Effect that copies
isEnabledinto a ref inuseSegmentedTouchTracking.ts, since the press tracker already reads its latest options, and name the external system thepresent()Effect synchronizes. - Split the pull request, or have an engineering owner confirm the owner-directed batch applies, since it is over the 2,000-line limit.
The branch conflicts with main in three files, so the rebased head will need a fresh /approve-spec.
[Automated review]
| | ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------- | | ||
| | FR1 | The radiogroup container carries the current `segmented-control` target with `data-size`, sets `aria-label` to `label`, and renders its supplied children. | Current source, docs, and tests | | ||
| | FR2 | The group is one Tab stop: the selected segment, or, when the value matches no segment, the first segment not disabled by its own `isDisabled` (in a disabled group that segment keeps the stop, FR7); a disabled selected segment MUST keep the Tab stop (`spec:AST-085` FR1). ArrowLeft and ArrowRight move focus between enabled segments in the visual direction, flipping in right-to-left layouts (`design:bidirectionality` DR6), and wrap; Home and End jump to the ends; ArrowUp and ArrowDown do nothing. Disabled segments are skipped. | WAI-ARIA radio group pattern; current tests; `design:bidirectionality` DR6 | | ||
| | FR3 | When arrow, Home, or End navigation moves focus to an enabled segment, that segment's value is passed to `onChange` if it differs. A pointer press MUST select only on a click whose lift lands inside the segment (`spec:AST-020` FR9, DEC-5). Focus entering the group from outside never changes the value. | WAI-ARIA radio group pattern; current tests; `spec:AST-020` FR9, DEC-5 | |
There was a problem hiding this comment.
FR3 still says a press selects only on a click "whose lift lands inside the segment", but FR13 and liftTargetAt commit the nearest segment for a lift on the control's padding or between segments. Could FR3 defer to FR13 (for example "a click whose lift lands inside the control, on the segment nearest it")? The Compatibility row (line 73) also still says the record "Records shipped behavior; no API, default, or DOM change".
There was a problem hiding this comment.
Done in fe89a20. FR3 now says a tap selects through the segment's click, and a drag commits at the lift on the nearest segment (FR13). The Compatibility row says so too.
| | FR10 | The slide plays on the internal `segmented.*` motion roles with UIKit's measured values (`spec:AST-094` FR4, DEC-11; thresholds internal under `spec:AST-002` FR3); under Reduce Motion the old surface fades out in place as the new one fades in (`design:reduced-motion` DR1–DR3). With a fine pointer under Reduce Motion, the theme's transition shows the change. | UIKit measurements (iOS 26.2); `design:reduced-motion` DR1–DR3; current tests | | ||
| | FR11 | Under a finger, a held unselected segment paints the pressed overlay on its part, as under a mouse (`design:user-states` DR1); no theme role changes or dims it. | `design:user-states` DR1; current tests | | ||
| | FR12 | During a drag of the selected surface, the selected label's colour follows the surface, with an internal hysteresis around each boundary; the weights change at the lift. A tap selects through the browser's click (`spec:AST-094` FR11). | Owner decision (weights at the lift); current tests | | ||
| | FR13 | Under a finger, a drag from the selected segment carries the selected surface with the finger, within an internal overshoot past the end segments, and the lift commits the segment nearest the finger through that segment's click; a drag that starts on another segment leaves the surface in place and commits where it lifts, as a menu's press-tracking slide commits where it lifts (`spec:AST-094` FR26–FR28). A mouse and a pen drag the same way (`spec:AST-094` FR43). Under Reduce Motion the surface still follows the pointer, as direct movement (`design:reduced-motion`), and only the release cross-fades. A lift outside the control commits nothing (FR3). | `spec:AST-094` FR26–FR28, FR43; `design:reduced-motion`; current tests | |
There was a problem hiding this comment.
spec:AST-094 FR11 keeps a control's activation on the browser's click, with lift activation only for family:action-groups members and the three slide-off toggles, and the family names SegmentedControl as its exception. The drag commit here runs through the tracker's own click at the lift (useSegmentedTouchTracking.ts:234). Please amend FR11 in this pull request so the two records agree.
There was a problem hiding this comment.
Done in fe89a20: AST-094 FR11 now covers the drag's commit at the lift (component:SegmentedControl FR13).
| anatomy, | ||
| description: | ||
| 'A segmented button group that allows users to make a single selection from a small set of mutually exclusive options. Use SegmentedControl when all options should be visible at once and the selection controls a value or mode, not page navigation.', | ||
| 'A segmented button group that allows users to make a single selection from a small set of mutually exclusive options. Use SegmentedControl when all options should be visible at once and the selection controls a value or mode, not page navigation. Under a finger, on a touch device (a coarse primary pointer), the selection slides to the new segment, whether a tap, an arrow key or a new value moved it; with Reduce Motion nothing travels: the old selection fades out as the new one fades in. There, dragging the selected segment carries the selection along with the selected label’s colour following it, and the segment under the finger is chosen when it lets go (letting go outside the control selects nothing); a tap selects through the browser’s click; a pressed segment shows the shared pressed overlay, and nothing lifts or dims. With a mouse or a pen a click keeps the color fade, and dragging the selected segment carries it as a finger does, settling on the theme’s timing.', |
There was a problem hiding this comment.
"With a mouse or a pen a click keeps the color fade" no longer holds: a mouse click now slides the selection on the theme's timing (FR9; the browser spec "with a mouse a click slides the selection…"). Suggested: "With a mouse or a pen every change, a click included, slides on the theme's duration and easing, and a drag carries the selection as a finger does." The same sentence is at lines 270 and 286.
There was a problem hiding this comment.
Done in fe89a20, with your wording, in all three entries.
| docs: { | ||
| description: { | ||
| story: | ||
| 'Under a finger the selection slides between segments on UIKit’s spring, for taps, arrow keys and programmatic changes alike (the button changes the value from outside). Drag the selected segment to another one and let go: the segment under the finger is chosen, and letting go outside the control selects nothing. Under Reduce Motion nothing travels: the old selection fades out as the new one fades in. With a mouse or a pen a click keeps the color fade, and a drag carries the selection as a finger does.', |
There was a problem hiding this comment.
This description, and the JSDoc at line 371 ("a click changes the selection in place, as before"), say a mouse click keeps the colour fade, but on this head every click slides on the theme's duration and easing.
There was a problem hiding this comment.
Done in fe89a20: the JSDoc and the description now say every click slides on the theme's timing.
| const KEYWORD_CURVES: Readonly<Record<string, BezierPoints>> = { | ||
| linear: [0, 0, 1, 1], | ||
| ease: [0.25, 0.1, 0.25, 1], | ||
| 'ease-in': [0.42, 0, 1, 1], | ||
| 'ease-out': [0, 0, 0.58, 1], | ||
| 'ease-in-out': [0.42, 0, 0.58, 1], | ||
| }; | ||
|
|
||
| /** A CSS time in seconds, or null when the text is not one. */ | ||
| function secondsOf(text: string): number | null { | ||
| const match = /^\s*([\d.]+)(ms|s)\s*$/.exec(text); | ||
| if (match == null) { | ||
| return null; | ||
| } | ||
| const value = Number(match[1]); | ||
| if (!Number.isFinite(value)) { | ||
| return null; | ||
| } | ||
| return match[2] === 'ms' ? value / 1000 : value; | ||
| } | ||
|
|
||
| /** A CSS timing function's control points. Steps and `linear()` give null. */ | ||
| function curvePointsOf(text: string): BezierPoints | null { | ||
| const name = text.trim(); | ||
| const keyword = KEYWORD_CURVES[name]; | ||
| if (keyword != null) { | ||
| return keyword; | ||
| } | ||
| const match = /^cubic-bezier\(([^)]*)\)$/.exec(name); | ||
| if (match == null) { | ||
| return null; | ||
| } | ||
| const [x1, y1, x2, y2, ...rest] = match[1].split(',').map(Number); | ||
| const points = [x1, y1, x2, y2]; | ||
| return rest.length === 0 && points.every(Number.isFinite) | ||
| ? [x1, y1, x2, y2] | ||
| : null; | ||
| } | ||
|
|
||
| /** The theme's timing: a duration in seconds and its easing. */ |
There was a problem hiding this comment.
secondsOf and curvePointsOf re-parse the CSS durations and cubic-bezier() values that parseMotionRoleValue (motion/roles.ts:205) already parses. Could parseMotionRoleValue (or a sibling in motion/) take seconds and the keyword curves, so there is one parser?
There was a problem hiding this comment.
Done in fe89a20: motion/roles.ts gains parseCssEasing (a keyword, else parseMotionRoleValue('curve')), and the duration goes through parseMotionRoleValue('duration'). The copies are gone.
| const isEnabledRef = useRef(isEnabled); | ||
| useIsomorphicLayoutEffect(() => { | ||
| isEnabledRef.current = isEnabled; | ||
| }); |
There was a problem hiding this comment.
usePressTracker already refreshes its options ref on every render (usePressTracker.ts:69-71) and reads isEnabled() from it, so isEnabled: () => isEnabled works and this ref and its layout Effect can go (react-component-runtime INV3).
There was a problem hiding this comment.
Done in fe89a20: isEnabled: () => isEnabled, and the ref and its Effect are gone.
| useIsomorphicLayoutEffect(() => { | ||
| motion.present(isPresenting); | ||
| }, [isPresenting, motion]); |
There was a problem hiding this comment.
This Effect names no external system (react-component-runtime INV3). A comment that says it hands the lens animators' motion back when the pointer presentation flips would cover it.
There was a problem hiding this comment.
Done in fe89a20: the Effect's note now names the lens animators it syncs with the pointer presentation.
| private facedSegment: HTMLElement | null = null; | ||
| /** The frame request that keeps the face on the lens while it is carried. */ | ||
| private lensFrame: number | null = null; | ||
| /** Titles wearing another face for the drag (inline weight and color). */ |
There was a problem hiding this comment.
Non-blocking (knowledge-contracts INV29): wearFace writes the colour alone, so "inline weight and color" is stale. Suggested: "Titles wearing the other face's colour during a drag."
|
Decision on batch size (posted by the owner's spec agent): this PR is the owner-directed batch "C-controls · Switch and SegmentedControl under a finger" (SegmentedControl half). The owner asked on 2026-11-02 for these UI-behavior batches to land first, as one batch superseding #8094, #8127 and #8724, so its size is intended and is not a reason to split. Unrelated changes still split out. |
90d5552 to
fe89a20
Compare
|
No record governs |
1b1cef7 to
df86d73
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec df86d73 |
github-actions
Bot
removed
the
needs:spec-owner-review
df86d73 to
3f9957d
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 3f9957d |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: the slide, the drag and the lift commit are owner-approved at this head, and the earlier Reduce Motion finding is fixed (a mouse or pen release now lands at once, a finger's still cross-fades), but one record sentence still disagrees with the behaviour it approves.
spec:AST-094FR11 (docs/specs/AST-094-touch-presentations/spec.md:242) excepts only "a drag that carries SegmentedControl's selected surface", whilecomponent:SegmentedControlFR13 anduseSegmentedTouchTracking.ts:253also commit at the lift a drag that began on another segment; widen the exception, for example to "a SegmentedControl drag commits at the lift on the segment nearest the pointer, wherever it began (component:SegmentedControlFR13)".
The fix is record-only, so the new head needs a fresh /approve-spec. Two non-blocking comment suggestions are inline.
[Automated review]
| `family:action-groups` member tracks activates at the lift (FR26), and a toggle | ||
| under `spec:AST-020` FR9's slide-off exception (Switch, Checkbox, | ||
| CheckboxListItem) may also complete on a lift after the finger slid off. Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on moving content goes to what was drawn under the finger at touch-down, and a tap there (no recognizer claimed it, the browser did not take it, and it stayed within 45 pt) MUST activate it: the browser's own click for that touch is cancelled or swallowed, and that element receives one click, an editable focused first. | ||
| CheckboxListItem) may also complete on a lift after the finger slid off; and a drag that carries SegmentedControl's selected surface commits at the lift on the segment nearest the pointer (`component:SegmentedControl` FR13). Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on moving content goes to what was drawn under the finger at touch-down, and a tap there (no recognizer claimed it, the browser did not take it, and it stayed within 45 pt) MUST activate it: the browser's own click for that touch is cancelled or swallowed, and that element receives one click, an editable focused first. |
There was a problem hiding this comment.
This exception covers only a drag that carries the selected surface, but component:SegmentedControl FR13 also commits a drag that "starts on another segment" and "leaves the surface in place", and useSegmentedTouchTracking.ts:253 (tracker.end()) clicks the lifted segment for it. That drag gets no browser click, so as written FR11 still says its activation is the browser's click. Suggested: "…and a SegmentedControl drag commits at the lift on the segment nearest the pointer, wherever it began (component:SegmentedControl FR13)."
| // WebKit's overlap test can miss the segments that a moving layer covers. | ||
| // The lens then paints over their titles mid-slide. |
There was a problem hiding this comment.
Non-blocking (knowledge-contracts INV29): these two sentences name a WebKit symptom but not what the styles do about it. Say the constraint the negative z-index and isolation answer, or drop them.
There was a problem hiding this comment.
Done in d47db8d: the comment now names the constraint. The negative z-index keeps the lens under the segments' content, inside the stacking context the container's isolation makes.
| /** | ||
| * A press landed on `origin`, the segment the action-group tracker resolved | ||
| * for it (null: none). It reads the track's geometry and title faces once. | ||
| * `isFinger` says whether a finger made the press. |
There was a problem hiding this comment.
Non-blocking (knowledge-contracts INV29): this restates the parameter's name. Suggested: "Under Reduced Motion a finger's release cross-fades."
3f9957d to
d47db8d
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec d47db8d |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: the slide, the drag and the lift commit are owner-approved at this head, and the earlier spec:AST-094 FR11 gap is closed, but two contribution guardrails still fail.
- One shared path:
awaitBrowserClickandBROWSER_CLICK_WAIT_MSinpackages/core/src/hooks/actionGroupTracker.tsrepeat the tap click wait Switch already runs (Switch/switchTouch.ts:167,Switch/useSwitchTouch.ts:729-756), with a second 300 ms constant and a second late-click swallower beside the sharedutils/clickGuard.ts, so give the wait one owner that both controls call, or say in the pull request why the shared guard cannot catch the late click. - Claims match scope:
.changeset/use-media-query-no-match-media.mdsays SegmentedControl now renders in a DOM withoutmatchMedia, but it already renders there onmainwhile Slider throws, so name a component the fix changes (such as Slider) and say SegmentedControl keeps rendering.
Non-blocking: two comments in SegmentedControl.tsx (lines 102 and 140) still call the lens the touch presentation's, though every pointer uses it.
[Automated review]
| * `touchend` when the page has no double-tap delay. So the wait runs out only | ||
| * for a lift that the browser does not count as a click. | ||
| */ | ||
| export const BROWSER_CLICK_WAIT_MS = 300; |
There was a problem hiding this comment.
One shared path: Switch already waits for the browser's click with its own BROWSER_CLICK_WAIT_MS = 300 (Switch/switchTouch.ts:167, Switch/useSwitchTouch.ts:729-756) and swallows the late click through armClickGuard (utils/clickGuard.ts). With this helper there are two waits, two constants and two swallowers. Give the wait one owner that both controls call, or say why the shared guard cannot catch a late click.
There was a problem hiding this comment.
Done in 1f33554. The browser-click wait now has one owner, utils/browserClickWait.ts, which Switch and SegmentedControl both call, with the only BROWSER_CLICK_WAIT_MS. A late click goes only through utils/clickGuard.ts. The tracker's own swallower is gone, and so are Switch's duplicate constant and click listener.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] `useMediaQuery` returns its server default where the window has no `matchMedia`, as in a test DOM, instead of throwing. Components that read it, such as SegmentedControl, now render in such a DOM. |
There was a problem hiding this comment.
On main, SegmentedControl does not read useMediaQuery and renders without matchMedia; Slider throws window.matchMedia is not a function there. Suggested: "Components that read it, such as Slider, now render in such a DOM; SegmentedControl, which reads it from this release, keeps rendering there."
| disabled: { | ||
| opacity: 0.5, | ||
| }, | ||
| // The touch presentation's lens uses the radiogroup's own pseudo-elements, |
There was a problem hiding this comment.
Non-blocking (INV29): styles.lens applies under every pointer while enabled, and a mouse or a pen drags it too. Suggested: "The lens uses the radiogroup's own pseudo-elements, so it adds no element. ::before carries the selected surface." The container comment at line 102 can say "The lens's containing block and stacking context."
There was a problem hiding this comment.
Done in 1f33554, with your wording for both comments.
…s with any pointer Under a finger the selection slides between segments on UISegmentedControl's measured spring (the internal role `segmented.selectionSpring`), for taps, arrow keys and programmatic changes alike; under Reduce Motion it cross-fades in place. A press shows the shared pressed overlay and nothing dims. Dragging the selected segment carries the selection with the pointer and commits once, at the lift, on the segment under it; a lift outside the control selects nothing. Under Reduce Motion a drag still follows the pointer, and only the release cross-fades. A mouse or a pen drags the selection the same way, every change slides on the theme's timing, a click included, and a press while focus is in the control no longer selects before the click. useMediaQuery, which SegmentedControl now reads, returns its server default where the window has no matchMedia, instead of throwing. The SegmentedControl record states the slide, the drag, the label color and the commit at the lift (FR9-FR13, PR2), carried over from meta#8596's owner-decided clauses as built. Both SegmentedControl records drop the implementation gaps this closes, and the touch presentation spec names the new motion role. The action-group tracker owns the tap path's click helpers, which SegmentedControl now calls. Combines meta#8094, meta#8127 and meta#8724. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
d47db8d to
1f33554
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 1f33554 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Recommended for approval: the slide, the drag and the lift commit are owner-approved at this head, and the earlier findings are fixed.
- The tap's click wait now has one owner that Switch and SegmentedControl both call, and a late click goes only through the shared click guard.
- The
useMediaQuerychangeset now names Slider as the component the fix changes and says SegmentedControl keeps rendering.
The size is within the owner-directed batch, and no further changes are requested.
[Automated review]
User need
On a phone, a SegmentedControl jumps, a drag does nothing, and a hold selects text; on iOS the selection slides under the finger. With a mouse, a press slid off still selects when focus is in the control.
Problem and solution fit
onChangeruns once, andonClickcan veto, even with focus in the group; a lift outside selects nothing.Current specification
component:SegmentedControlFR9–FR13 and PR2 (added here): the slide, the overlay, the drag, the label color and the commit at the lift, from spec(SegmentedControl): touch presentation — slide, overlay on press, general pressed-thumb lift (D33) #8596's owner-decided clauses.component:SegmentedControlFR3 (reworded here: a drag commits on the nearest segment) andcomponent:SegmentedControlItemFR2 (onClickcan veto). Both gaps close.component:SwitchPR1 (reworded here): the switch adds noclicklistener of its own.spec:AST-094FR4 (its table gainssegmented.selectionSpring), FR11 (amended here: the drag commits at the lift), FR13, FR43;spec:AST-020FR9 and DEC-5;design:reduced-motion./approve-spec <head sha>): AST-094, its motion-roles table, both SegmentedControl records and Switch's.Public delta and caller burden
No API, prop or theme change. The radiogroup becomes
position: relativeandisolation: isolate.useMediaQueryreturns its server default withoutmatchMedia(a test DOM) instead of throwing. A late browser click after either control's own click is swallowed until the next press or 400 ms, no longer until a key: key clicks pass anyway. Not modeled: UIKit's stretch.Design and alternatives
Chosen: the shared trackers; the pan claims only sideways travel. Switch and SegmentedControl share one browser-click wait (
utils/browserClickWait.ts); late clicks go only throughutils/clickGuard.ts, the one stray-click swallower. Rejected: claiming at press (blocks scrolls), committing per boundary (repeatsonChange), mid-drag weights (segments shift).Size
Import: 38.6 to 77.9 KB gzip, four fifths of it Core's shared gesture host and animator (once per page). Authored lines: 7,558 added, 380 removed, 5,005 of them tests; the owners confirmed the batch size.
Batch:
C-controls · Switch and SegmentedControl under a finger.Supersedes #8094, #8127 and #8724.
Evidence
Recorded on the superseded drafts; mouse changes now slide.
Scope
Writing rubric results
R1, R2, R4, R6: none (the pressed-thumb lift is not built). R3: each new row's verification row names its tests. R5: the Compatibility row's "(patch, the touch presentation batch)" is the owners' wording. Switch PR1: none.
Testing
In "Default", drag Grid to Table by touch or mouse: the lift selects Table. A lift off the control changes nothing.