Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
|
/approve-spec 96b5d65 |
There was a problem hiding this comment.
Changes requested: the lift behaves as the approved FR14 describes, but two things need fixing first.
- In the default light and dark themes the lifted selected surface is the page's own colour with no edge, so holding a segment erases the track around it (an end segment loses the track's end, a middle segment splits it in two); give the lifted surface a visible edge, such as the theme's elevation shadow Switch puts on its lifted thumb, and add a browser assertion that the track stays visible around a held segment.
- The usage description in
SegmentedControl.doc.mjsnow says both that pressing the selected segment lifts it and that "nothing lifts or dims"; end it with "a pressed unselected segment shows the shared pressed overlay, and nothing dims" instead.
The spec changes are approved for this revision and the unit suites pass. The rest is optional: a stale line in motion/MEASUREMENTS.md, two comment wordings, and the FR14 row of the verification map, which fills only two of its five columns.
[Automated review]
| scaleY: (carried.box.height + height) / carried.box.height, | ||
| }; | ||
| this.isLifted = true; | ||
| this.springLens(lifted, this.springFrom(lifted, 'lift')); |
There was a problem hiding this comment.
In the default theme the lens is an opaque copy of the selected surface: white with no shadow in light mode, rgb(38, 38, 38) in dark mode, the page's own colour in both. This lift grows it past the track, so for as long as the finger is down the track around the held segment disappears (an end segment loses the track's end, a middle one splits it in two). Consider giving the lifted lens an edge while lifted, such as the theme's elevation shadow Switch puts on its lifted thumb, and a browser assertion that the track stays visible around a held segment.
There was a problem hiding this comment.
Done in 91c7ee4. While lifted, the lens carries the theme's low elevation shadow (--shadow-low, the token on Switch's lifted thumb), set on the radiogroup and written onto the lens at the lift. The hand-back clears it. The browser lift case now asserts that the held lens's computed box-shadow is the resolved theme shadow, not none, and FR14 states the edge.
| 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. 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 every change, a click included, slides on the theme’s duration and easing, and dragging the selected segment carries it as a finger does.', | ||
| '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, pressing the selected segment lifts it, and dragging it 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 every change, a click included, slides on the theme’s duration and easing, and dragging the selected segment carries it as a finger does.', |
There was a problem hiding this comment.
This sentence now says "pressing the selected segment lifts it" and later "a pressed segment shows the shared pressed overlay, and nothing lifts or dims". Suggest ending it with "a pressed unselected segment shows the shared pressed overlay, and nothing dims" (the same edit at lines 243 and 259).
There was a problem hiding this comment.
Done in 91c7ee4, with your wording, in all three strings.
| * The lens chases a dragging finger on the shared `springs.fingerTracking` | ||
| * role, which matches the values UIKit reports for this control. A flick | ||
| * carries the selection no further than a slow drag: UIKit projects no | ||
| * flick, so the lift's position alone decides. |
There was a problem hiding this comment.
Optional: "lift" now also names the lens growth, so "the lift's position alone decides" is ambiguous. Suggest "the release position alone decides".
There was a problem hiding this comment.
Done in 91c7ee4: "the release position alone decides".
| // A lifted lens shrinks back on the lift's spring while it moves on | ||
| // the slide's. |
There was a problem hiding this comment.
Optional: this repeats the method's JSDoc just above ("a lifted lens shrinks back on the lift's"), so the inline comment can go.
96b5d65 to
91c7ee4
Compare
|
/approve-spec 91c7ee4 |
There was a problem hiding this comment.
Changes requested: the lift and its new edge behave as the approved FR14 describes while a segment is held, but two things need fixing first.
design:user-statesDR3 (its rule, anatomy and matrix rows, and example) andcomponent:SegmentedControlItemFR5 still say the selected segment keeps its raised surface unchanged during a hold under every pointer, so amend them in this PR to allow the touch lift (or drop the lift), and update the "selected-segment exception" line in the SegmentedControl record.- After a drag that commits another segment,
select()repaints the lens from the destination (useSegmentedSelectionMotion.ts:633) and drops the lifted shadow while the surface is still grown, so for about 0.1 s the white surface again covers the track's edges; keep the lifted shadow until the hand-back and assert it after a committing drag.
The spec changes are approved for this revision, the consumer description now reads consistently, and the unit and Chromium lift tests pass. One optional cleanup: the browser spec comment at SegmentedControl.interaction.browser.spec.ts:450-451 still gives the old "a pressed thumb's lift is a theme's" reason.
[Automated review]
| this.isLifted = true; | ||
| this.springLens(lifted, this.springFrom(lifted, 'lift')); | ||
| // Until the hand-back clears the lens. | ||
| this.track?.style.setProperty( |
There was a problem hiding this comment.
This shadow does not survive a drag that commits another segment: select() repaints the lens from the destination at line 633 (paintLayer(track, 'lens', destination)), which overwrites --_segmented-lens-box-shadow while settleLens is still shrinking the lifted lens. In the neutral theme the grown surface then has no edge for about 0.1 s. Consider re-applying the lifted shadow after that repaint until the hand-back, and asserting the lens's box-shadow after a committing drag.
There was a problem hiding this comment.
| }; | ||
| this.isLifted = true; | ||
| this.springLens(lifted, this.springFrom(lifted, 'lift')); | ||
| // Until the hand-back clears the lens. |
There was a problem hiding this comment.
Optional (comment style): a full sentence reads better here, for example "The hand-back clears this shadow with the lens."
There was a problem hiding this comment.
Done in c2bc372: the note is now the flag's doc, in full sentences.
| | FR8 | The radiogroup owns `role`, `aria-label` (from `label`), `aria-disabled`, and `data-size`; a caller value for any of them is replaced. Other base props reach the radiogroup, and `ref` reaches it. Implementation gap: a caller `aria-describedby` is replaced, by the disabled-reason id or by nothing, against `architecture:public-component-api` INV5. | `architecture:public-component-api` INV5; current source and tests | | ||
| | FR9 | Under a finger (`spec:AST-094` FR13), a change of selection slides the selected surface from the old segment to the new one as one surface, the radiogroup's `::before`, animating transform and opacity only; under a fine pointer it slides on the theme's duration and easing. | Owner direction; `spec:AST-094` FR13; current source and tests | | ||
| | 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. A held selected segment lifts the selected surface instead (FR14). | `design:user-states` DR1; current tests | |
There was a problem hiding this comment.
design:user-states DR3 (with its anatomy and matrix rows and example) and component:SegmentedControlItem FR5 still say the selected item keeps its raised surface unchanged during a hold under every pointer, and line 204 of this record still defers to that exception. Amending them in this PR keeps the records consistent with this sentence and FR14.
There was a problem hiding this comment.
Done in c2bc372: design:user-states DR3 (rule, anatomy and matrix rows, example), component:SegmentedControlItem FR5 and this record's line now say the selected surface takes no pressed overlay and lifts under the touch presentation (FR14), staying unchanged under a fine primary pointer.
91c7ee4 to
c2bc372
Compare
c2bc372 to
ba3e59d
Compare
…nt lifts its surface, as UIKit's does The selected surface grows by 24 × 16 pt on UIKit's measured lift spring (critically damped, 0.25 s) and settles back on it at the release. While lifted it carries the theme's low elevation shadow, as Switch's lifted thumb does, until the surface is handed back, so the track stays visible around it. The lift follows the presentation: a finger or a pen on a touch device lifts it, while a desktop mouse and Reduce Motion lift nothing. The record gains FR14 and the `segmented.lift` motion role. FR14 names the lift as the one exception to design:user-states DR3's unchanged surface, which this change leaves as it is. UIKit projects no flick, so the release position still decides the segment.
ba3e59d to
76c0718
Compare
|
/approve-spec 76c0718 |
There was a problem hiding this comment.
Changes requested: the lift and its edge behave as the owner-approved FR14 describes, but one current record still contradicts them.
design:user-statesDR3 (its rule, anatomy row, matrix row and example) still says the selected SegmentedControl item keeps its surface unchanged during a hold under every pointer, and FR14's "one exception" sentence cannot override another current record (architecture:knowledge-contractsINV9 and DEC-1), so restore the DR3 amendment from the previous push, have DR3 link to FR14 instead, or drop the lift.
The edge now survives a committing drag, which settles the earlier finding. One optional comment-style suggestion is inline.
[Automated review]
| | 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. A held selected segment lifts the selected surface instead (FR14). | `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 a finger's release cross-fades; under a fine pointer a mouse's or a pen's release lands at once (DR3). A lift outside the control commits nothing (FR3). | `spec:AST-094` FR26–FR28, FR43; `design:reduced-motion`; current tests | | ||
| | FR14 | On a press, the selected surface lifts as Switch's thumb does (`component:Switch` FR13), with the lift's scale and timing measured from UISegmentedControl (iOS 26.2); it settles back at the release. While lifted it carries the theme's low elevation shadow, as Switch's lifted thumb does, so the track stays visible around it. The lift follows the presentation, as Switch's does: under the touch presentation a pen's press lifts as a finger's does, and under a fine primary pointer nothing lifts. A flick follows UISegmentedControl's measured rule: UIKit projects no flick, so the lift commits the segment nearest the finger, as FR13 states. Under Reduce Motion nothing lifts, and the selection cross-fades as FR10 states. The selected surface's lift under the touch presentation is the one exception to `design:user-states` DR3's unchanged surface, for consistency with Switch's thumb lift (`component:Switch` FR13). | Owner direction, 2026-11-02 (the selected surface lifts for consistency with Switch's thumb lift); UIKit measurements (iOS 26.2): the lens grows by 24 × 16 pt on a critically damped 0.25 s spring and settles back on it; a fast flick commits where a slow drag to the same point does; `component:Switch` FR13; `design:reduced-motion` | |
There was a problem hiding this comment.
This sentence makes FR14 an exception to design:user-states DR3, but DR3 itself (rule, anatomy row, matrix row and example) still says the selected item keeps its surface unchanged under every pointer, and architecture:knowledge-contracts INV9 and DEC-1 don't let a narrower record override another current one. The previous push's DR3 amendment resolved this; please restore it (or have DR3 link to FR14 instead of stating the unchanged surface), or drop the lift.
| // lifted thumb. The lens copies the selected surface, which can be the | ||
| // page's own colour, so without an edge a lifted lens would hide the | ||
| // track around it. |
There was a problem hiding this comment.
Optional (comment style, INV29): this sentence carries two topics in 26 words. Suggested: "The lens copies the selected surface, which can match the page. Without an edge, a lifted lens hides the track around it."
|
/revoke-spec 76c0718 |
User need
On iOS 26 a press on a segmented control's selected segment lifts its surface under the finger, as Astryx's Switch thumb already does. Our SegmentedControl stays flat, so a press on the selected segment shows no feedback. Specs decision, Nov 2 2026: the selected surface lifts for consistency with Switch's thumb lift.
Problem and solution fit
Current specification
component:SegmentedControlFR14 (added here): the spec owners' text, the lifted edge, the presentation rule, the flick sentence for no projection, the Reduce Motion line and the owners' DR3 exception sentence. FR11 gains "A held selected segment lifts the selected surface instead (FR14)".design:user-statesDR3's unchanged surface;component:SegmentedControlItemFR5 (reworded here) defers to it.design:user-statesis unchanged.spec:AST-094FR4: its table gainssegmented.lift, andmotion-roles.mdis regenerated.component:SwitchFR13 (the lift this matches);design:reduced-motion./approve-spec <head sha>): AST-094, its motion-roles table and both SegmentedControl records.Measurements (iOS 26.2 simulator, iPhone 17 Pro)
UIViewSpringAnimationBehaviorSettings(1.0, 0.25) on the lens. A per-frame fit of a 600 ms hold with that spring leaves 0.16 pt rms on the lift and 0.05 pt on the settle.Public delta and caller burden
No API, prop or theme change. The lifted surface paints up to 6 px above and below the track and 10 px past its ends, as UIKit's does; the track clips nothing.
Design and alternatives
Chosen: a built-in measured motion role,
segmented.lift, as Switch's thumb lift is. #8596's D33 pressed-thumb lift role is still unmerged. Whichever lands second sets D33's default to this measured lift. Not in scope: UIKit's cancel rules (an open question) and UIKit's stretch along a slide.Scope
Writing rubric results
FR14, FR11 and SegmentedControlItem FR5: R1–R6 none. FR14's first and last sentences are the spec owners' wording.
Testing
In "Default", press and hold the selected segment by touch: its surface grows with a soft shadow, and it settles back when you let go. With a mouse, nothing lifts.
specs:for --diff