Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
|
Gestures-side review at 8723e79:
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This cannot be approved until an engineering owner decides on draft spec:AST-094: it is the only authority for every touch press, motion and gesture change in this diff, and it proposes amendments to several current records. An engineering owner records that decision by commenting /approve-spec 8723e795a9d6dee9c22f65ef2bcc692ff375fcef on this pull request, and the author can do so if they are an engineering owner; FR8 should first conform to the current reduced-motion record (opacity only, 200 ms or less, no spring). Separately, these fixes are needed:
- Rebase once the prerequisite pull requests (#8118's stack through #8356) have landed, so the diff holds this pull request's three commits instead of 42,628 authored lines.
- Gate the list row press layer (
interactionOverlayStyles.cell) on@media (pointer: coarse)like Button's, since it now hides a desktop row's own::beforedecoration, whichspec:AST-017FR59 rules out for touch work. - Key the divider-hiding rule on a finger press so a trackpad press on a tablet keeps the dividers, and add fine-pointer before-and-after evidence for the mouse and Space press paint on touch-first devices.
- Update the pending changesets that still describe the delegated press, the press profiles,
highlightDelay/quickTapFlashandVelocityStrategy.
[Automated review]
| animationDuration: null, | ||
| animationTimingFunction: null, | ||
| animationFillMode: null, | ||
| isolation: 'isolate', |
There was a problem hiding this comment.
This layer has no @media (pointer: coarse) gate, unlike control below, so on a desktop every interactive Item, ListItem and TreeList row gets isolation: isolate and an opacity-0 ::before, and a row's own ::before decoration from xstyle is hidden at rest. spec:AST-017 FR59 keeps fine-pointer appearance unchanged for touch work; gating this layer the way control is gated would fix it.
| default: colorVars['--color-border'], | ||
| '@media (pointer: coarse)': { | ||
| default: null, | ||
| '[data-astryx-press="on"]': 'transparent', |
There was a problem hiding this comment.
A mouse or trackpad press on a touch-first device also writes data-astryx-press="on" (pressButton, whose buttonDown step paints on), and this rule keys only on the coarse primary pointer. So a trackpad press on a tablet hides the dividers, although the comment above says they stay drawn with a mouse. Could the rule key on a finger press instead, with a test of a mouse press under a coarse primary pointer?
| 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.
Under Reduce Motion, current design:reduced-motion DR2 limits a coarse-pointer transition to an opacity-only fade of 200 ms or less, and DR1 rules out springs. This 0.5 s cross-fade and the menu spring go past both, and the record's current-state impact does not list that record. The code already follows DR2 (reducedMotionCrossFadeMs: 200 in motion/uikit/timing.ts).
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] UIControl press tracking with control and cell profiles, the shared press paint, delegated press tracking and the gesture hooks. Not exported from the package root; touch presentations reach them through the experimental subpaths (see their entry). No component uses them yet. |
There was a problem hiding this comment.
This pull request removes the control and cell profiles, the gestures press paint and the delegated press tracking, so if this entry ships with the rest of the stack the changelog announces pieces that do not exist. The same goes for gesture-hook-fixes.md (highlightDelay, quickTapFlash), gesture-host-fixes.md (the delegated press) and touch-experimental-subpaths.md (VelocityStrategy).
meta-cla
Bot
added
the
CLA Signed
8723e79 to
f68068e
Compare
f68068e to
35f10e7
Compare
There was a problem hiding this comment.
This can't be approved yet: the touch press, list and theme-motion changes this diff carries from the stack have no current spec authority, since they rest on draft spec:AST-094, which is only proposed in #8378, and the 250 ms ease-in-out touch release contradicts the 200 ms in component:Button FR15. An engineering owner needs to decide whether spec:AST-094 governs these changes and amend FR15, recording the approval on #8378 with /approve-spec and that pull request's full head SHA so the record becomes current on main (the author can do this as an engineering owner), or say which changes should leave the stack. Separately, these fixes are needed:
- Rebase once the prerequisite pull requests land, so the diff holds this pull request's three commits instead of 42,887 authored lines.
interactionOverlayStyles.cellreplaces the row'sanimationNamewithout the Reduce Motion arm, so list rows still fade for 200 ms with a fine pointer; spreadingstrengthReleaseOutsideCoarsethere fixes it.- The divider rule keys only on a coarse primary pointer, so a trackpad or mouse press on a tablet hides the dividers; key it on a finger's press and show fine-pointer before and after.
gesture-press-tracking.mdstill announces the delegated press tracking this pull request removes.
[Automated review]
| // declares it (its clock and curve are untouched). | ||
| animationName: { | ||
| default: null, | ||
| [PRESSED_FADING]: {default: pressRelease, [COARSE]: 'none'}, |
There was a problem hiding this comment.
Composed after the row's overlay variant, this animationName replaces the variant's whole, so it drops the '@media (prefers-reduced-motion: reduce) and (not (pointer: coarse))': 'none' arm. With a fine primary pointer and Reduce Motion, a finger's press on an interactive Item, ListItem or TreeList row still fades for 200 ms (in Chromium the row keeps the strength keyframes while Button reads animation-name: none), against design:reduced-motion DR3 and press-paint-reduced-motion.md. Spreading ...strengthReleaseOutsideCoarse here would keep this arm set plus the Reduce Motion one.
35f10e7 to
bb771ca
Compare
There was a problem hiding this comment.
This can't be approved yet: the touch press, list and theme-motion changes this diff carries from the stack have no current spec authority, since the record proposed for them, draft spec:AST-094, is only in #8378, and the 250 ms ease-in-out touch release contradicts the 200 ms in component:Button FR15. An engineering owner needs to decide whether spec:AST-094 governs these changes and amend FR15, recording the approval on #8378 by commenting /approve-spec with that pull request's full head SHA so the record becomes current on main (the author can do this as an engineering owner), or say which changes should leave the stack. Separately, these fixes are needed:
- Rebase once the prerequisite pull requests land, so the diff holds this pull request's three commits instead of 42,858 authored lines.
interactionOverlayStyles.cellreplaces the row'sanimationNamewithout the Reduce Motion arm, so list rows still fade for 200 ms with a fine pointer; spreadingstrengthReleaseOutsideCoarsethere fixes it.- The divider rule and
holdSelectionkey only on a coarse primary pointer, so a mouse or trackpad press on a tablet hides the dividers and is held; key them on a finger's press and show fine-pointer before and after. gesture-press-tracking.mdand line 228 of the gestures README still describe the delegated press this pull request removes.
[Automated review]
| // declares it (its clock and curve are untouched). | ||
| animationName: { | ||
| default: null, | ||
| [PRESSED_FADING]: {default: pressRelease, [COARSE]: 'none'}, |
There was a problem hiding this comment.
Composed after the row's overlay variant, this animationName replaces the variant's whole, so it drops the '@media (prefers-reduced-motion: reduce) and (not (pointer: coarse))': 'none' arm. With a fine primary pointer and Reduce Motion, a finger's press on an interactive Item, ListItem or TreeList row still fades for 200 ms, against design:reduced-motion DR3 and press-paint-reduced-motion.md. Spreading ...strengthReleaseOutsideCoarse here instead keeps this arm set plus the Reduce Motion one.
| default: colorVars['--color-border'], | ||
| '@media (pointer: coarse)': { | ||
| default: null, | ||
| '[data-astryx-press="on"]': 'transparent', |
There was a problem hiding this comment.
On a touch-first device a mouse or trackpad press also writes data-astryx-press="on" (pressButton in utils/pressFeedback.ts), and these arms key only on the coarse primary pointer, so a trackpad press on a tablet hides the dividers although the comment above says they stay drawn with a mouse (spec:AST-017 FR59). Could this key on a finger's press, with a test of a mouse press under a coarse primary pointer?
| ); | ||
| if ( | ||
| row == null || | ||
| !isTouch(row.ownerDocument.defaultView) || |
There was a problem hiding this comment.
This checks only the device's primary pointer, so on a tablet a mouse or trackpad click also gets a hold, while list-selection-hold.md says the hold is null with a mouse. Checking how the activation was made (or a finger marker from the press controller) would match the changeset.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: UIControl press tracking with control and cell profiles, the shared press paint, delegated press tracking and the gesture hooks. Not exported from the package entry point; no component uses them yet. |
There was a problem hiding this comment.
This pull request removes the tracker's control and cell profiles, the gestures press paint and the delegated press tracking, so if the stack ships in one release the changelog lists them beside press-machine-one-writer.md, which says they are gone. Could this entry describe only what remains?
| off the iOS callout and text selection on its element. | ||
|
|
||
| While a finger or a pen is down, the host turns them off too on the | ||
| element of a press tracker (attached or delegated: a control or a cell) |
There was a problem hiding this comment.
The delegated press tracker is gone after this pull request, so "attached or delegated: a control or a cell" can drop "or delegated".
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsButton (@astryxdesign/core) · View in Storybook
ClickableCard (@astryxdesign/core) · View in Storybook
ContextMenu (@astryxdesign/core) · View in Storybook
Item (@astryxdesign/core) · View in Storybook
MoreMenu (@astryxdesign/core) · View in Storybook
MultiSelector (@astryxdesign/core) · View in Storybook
SelectableCard (@astryxdesign/core) · View in Storybook
Thumbnail (@astryxdesign/core) · View in Storybook
TreeList (@astryxdesign/core) · 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.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/AlertDialog | +3.4KB |
+814B |
| core/AppShell | +3.2KB |
+829B |
| core/AvatarGroupOverflow | +3.3KB |
+827B |
| core/Banner | +3.4KB |
+829B |
| core/BaseTypeahead | +3.4KB |
+835B |
| core/BreadcrumbItem | +3.4KB |
+838B |
| core/Button | +3.4KB |
+820B |
| core/Calendar | +3.4KB |
+818B |
| core/Carousel | +3.4KB |
+822B |
| core/ChatComposer | +3.3KB |
+825B |
| core/ChatComposerInput | +3.3KB |
+826B |
| core/ChatComposerTokenElement | +3.3KB |
+826B |
| core/ChatDictationButton | +3.4KB |
+820B |
| core/ChatLayout | +3.3KB |
+821B |
| core/ChatLayoutScrollButton | +3.4KB |
+820B |
| core/ChatSendButton | +3.3KB |
+820B |
| core/CheckboxInput | +3.3KB |
+811B |
| core/CheckboxListItem | +3.4KB |
+964B |
| core/ClickableCard | +3.3KB |
+857B |
| core/CodeBlock | +3.4KB |
+839B |
| core/Collapsible | +3.3KB |
+869B |
| core/CommandPalette | +3.4KB |
+830B |
| core/CommandPaletteItem | +3.3KB |
+874B |
| core/ComplexSelector | +3.4KB |
+827B |
| core/ContextMenu | +3.4KB |
+971B |
| core/ContextMenuItem | +3.4KB |
+861B |
| core/DateInput | +3.3KB |
+828B |
| core/DateRangeInput | +3.3KB |
+818B |
| core/DateTimeInput | +3.2KB |
+834B |
| core/DialogHeader | +3.4KB |
+821B |
| core/DrawerHeader | +3.4KB |
+821B |
| core/DropdownMenu | +3.4KB |
+973B |
| core/DropdownMenuCheckboxItem | +3.4KB |
+831B |
| core/DropdownMenuItem | +3.4KB |
+861B |
| core/DropdownMenuRadioItem | +3.4KB |
+831B |
| core/DropdownMenuSubMenu | +3.2KB |
+833B |
| core/FileInput | +3.3KB |
+815B |
| core/IconButton | +3.4KB |
+820B |
| core/Item | +3.6KB |
+857B |
| core/LayerProvider | +3.4KB |
+828B |
| core/Lightbox | +3.4KB |
+829B |
| core/Link | +3.3KB |
+821B |
| core/ListItem | +3.3KB |
+964B |
| core/Markdown | +3.5KB |
+976B |
| core/MobileNav | +3.3KB |
+828B |
| core/MobileNavToggle | +3.4KB |
+820B |
| core/MoreMenu | +3.5KB |
+973B |
| core/MultiSelector | +3.2KB |
+808B |
| core/NumberInput | +3.3KB |
+829B |
| core/Outline | +3.3KB |
+859B |
| core/Pagination | +3.4KB |
+823B |
| core/Popover | +3.3KB |
+820B |
| core/PowerSearch | +3.3KB |
+828B |
| core/RadioListItem | +3.3KB |
+816B |
| core/SegmentedControlItem | +3.3KB |
+866B |
| core/SelectableCard | +3.4KB |
+852B |
| core/Selector | +3.2KB |
+828B |
| core/SelectorOption | +3.5KB |
+854B |
| core/SideNav | +3.3KB |
+833B |
| core/SideNavCollapseButton | +3.4KB |
+821B |
| core/SideNavHeading | +3.3KB |
+825B |
| core/SideNavItem | +3.3KB |
+828B |
| core/Step | +3.3KB |
+833B |
| core/Stepper | +3.4KB |
+821B |
| core/Switch | +3.3KB |
+821B |
| core/Tab | +3.3KB |
+842B |
| core/TabMenu | +3.3KB |
+818B |
| core/Table | +3.4KB |
+973B |
| core/TableCell | +3.3KB |
+975B |
| core/TableHeaderCell | +3.3KB |
+976B |
| core/TableSelectionToolbar | +3.3KB |
+827B |
| core/TextInput | +3.3KB |
+826B |
| core/Thumbnail | +3.4KB |
+807B |
| core/TimeInput | +3.3KB |
+831B |
| core/Toast | +3.4KB |
+820B |
| core/ToggleButton | +3.4KB |
+819B |
| core/Token | +3.3KB |
+851B |
| core/Tokenizer | +3.4KB |
+822B |
| core/TopNav | +3.3KB |
+829B |
| core/TopNavHeading | +3.3KB |
+821B |
| core/TopNavItem | +3.3KB |
+869B |
| core/TopNavMegaMenu | +3.3KB |
+833B |
| core/TopNavMegaMenuItem | +3.3KB |
+866B |
| core/TopNavMenu | +3.3KB |
+828B |
| core/TreeList | +3.6KB |
+847B |
| core/Typeahead | +3.4KB |
+816B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 33.7KB | 34.7KB | +1008B |
Accessibility Audit
Status: 7 accessibility violation(s) found — 7 serious.
Button pattern - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/10 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
Checkbox pattern - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/8 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
ClickableCard - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/5 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
Radio group pattern - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 3/16 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
RadioList - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 3/14 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
SelectableCard - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/4 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
Thumbnail - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 1/12 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
Visual Regression
48 of 504 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 556,900 |
| RichTextEditor | Markdown parity: side by side | probe | light | 538,816 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 28 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
UIKit's two-event filter keeps the last moving value after a pause; the note now says so as UIKit's behavior and that Astryx's release reads 0 once the finger has rested (spec:AST-100 FR2), so the table no longer reads as Astryx's rule.
Her arena work (the touch depth, a view's delivery only to involved recognizers, the reset count, touchBeganElsewhere's delivery, the simultaneity rule) reaches the recognizer's plumbing through the internal view, as this PR keeps it out of the public type; the new deliverBeganElsewhere joins that view.
Her foundation fixes for the review items this PR also answered take her form: the swipe's segment reading with directions ruled out for good, the tap's lift centroid, the claim release that counts a recognized discrete gesture, Hover's restart and the idle recognizer's comment. Both sides' tests stay: her spacing, 40-degree, re-basing and settling cases beside this PR's 20-degree, re-based release and mid-hover cases.
There was a problem hiding this comment.
This can't be approved yet: the diff rewrites component:Button FR15 from a 200 ms touch release to spec:AST-094's 250 ms control clock, which no current spec states. An engineering owner needs to approve that rewording by commenting /approve-spec with this pull request's full head SHA (the author can, as an engineering owner), or say the touch release keeps 200 ms. Separately, these fixes are needed:
- Rebase once the prerequisite pull requests land, so the diff holds this pull request's three commits instead of 44,982 authored lines.
- The touch press release plays fixed 250 ms and 200 ms constants, so a theme's
timing.press.buttonReleaseMs,highlightOutMsorhighlightOutCurve, whichdefineThemeaccepts, never reaches it (spec:AST-094FR4). - The divider rule and
holdSelectionkey only on a coarse primary pointer, so a mouse or trackpad press on a tablet hides the dividers and holds the row; key them on a finger's press, test it, and add fine-pointer before-and-after evidence for rows (spec:AST-017FR59). gesture-press-tracking.mdstill announces the delegated press this pull request removes, and the selection lock cites aspec:AST-094FR41 the record does not have.
[Automated review]
| // after a cancel. With a mouse the dividers stay drawn. | ||
| borderBlockEndColor: { | ||
| default: colorVars['--color-border'], | ||
| '@media (pointer: coarse)': { |
There was a problem hiding this comment.
On a touch-first device a mouse or trackpad press also writes data-astryx-press="on" (pressButton in utils/pressFeedback.ts, reached whenever stylesheetDroppedActive() is true), and these arms key only on the coarse primary pointer. So a trackpad press on a tablet hides both dividers, although the comment above says they stay drawn with a mouse (spec:AST-017 FR59). Could they key on a finger's press, with a dividers test of a mouse press under a coarse primary pointer?
| ); | ||
| if ( | ||
| row == null || | ||
| !isTouch(row.ownerDocument.defaultView) || |
There was a problem hiding this comment.
This checks only the device's primary pointer, so on a tablet a trackpad click that pushes from a row is held selected, while list-selection-hold.md says the hold is null with a mouse. Checking how the activation was made (or a finger marker from the press controller) would match the changeset.
| * How long a control's release takes, in ms: UIButton's highlight release | ||
| * (`button.highlight.releaseMs`), for every other surface. | ||
| */ | ||
| controlReleaseMs: 250, |
There was a problem hiding this comment.
timing.press.buttonReleaseMs is a registered theme motion role (motion/roleKinds.generated.ts:56), and astryx docs motion tells theme authors the code reads --motion-timing-press-button-release-ms when the motion starts, but this constant (and PRESS_PROFILES in utils/pressFeedback.ts) plays 250 ms whatever the theme sets. spec:AST-094 FR4 requires a touch motion to read its role and play no literal; the release arms could read the role variable with this value as the fallback, and the controller's clock likewise.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: UIControl press tracking with control and cell profiles, the shared press paint, delegated press tracking and the gesture hooks. Not exported from the package entry point; no component uses them yet. |
There was a problem hiding this comment.
This pull request removes the tracker's control and cell profiles, the gestures press paint and the delegated press tracking, so a release carrying the stack lists them beside press-machine-one-writer.md, which says they are gone. Could this entry describe only what remains?
…eme role Both release clocks are now their roles' variables with the table's value as fallback (timing.press.buttonReleaseMs for a control, highlightOutMs for a cell), so the test matches them by role name instead of a bare duration.
The press releases are their theme roles' variables with the table's value as the fallback (var(--motion-timing-press-button-release-ms,.25s) for a control, highlight-out-ms for a cell). build-css.test.mjs's Reduce Motion and desktop-rule cases read a clock's fallback and find UIButton's by its role, as pressPaintReducedMotion.test.ts does, instead of matching a bare duration: the two that matched '.25s' found nothing and failed. Each still fails when a Reduce Motion arm runs past 200 ms or a control's layer release runs outside the coarse gate.
…s tracker leaves The press tracker reports control events and paints nothing, so the tests meta#8356 added read its press another way: the per-event unit case checks only the delivered touchDown and the browser's pan; the browser fixture's kept press exposes its tracker, and the proof waits for keepsTouch() after the content-touch delay, checks it still holds after the drag, then touchUpInside at the lift.
There was a problem hiding this comment.
This can't be approved yet: the diff rewrites current component:Button FR15 from a 200 ms touch release to a 250 ms one, and it adds holdSelection, the held list row and the divider hiding, which no current spec covers. An engineering owner needs to approve the FR15 rewording by commenting /approve-spec 036c419651e886d2ee77e38560d195d3b9310d91 on this pull request (the author can, as an engineering owner), and to name the record that governs those List changes or say they leave the stack. Separately, these fixes are needed:
- Rebase once the prerequisite pull requests land, so the diff holds this pull request's commits (about 1,600 lines) instead of 43,965 authored lines, and resolve the conflicts with
mainin fourmotion/uikitfiles. - Read
timing.curves.movein the control release (interactionOverlay.stylex.ts:263), and maketiming.press.flashMsandtiming.press.highlightInMsreach the press or stop registering them (spec:AST-094FR4). - Key the divider hiding and
holdSelectionon a finger's press, so a trackpad press on a tablet keeps the dividers and holds nothing, as the changesets say (spec:AST-017FR59). - Drop the
perEventPointerpress-tracker carve-out, which current FR19 doesn't grant, and updategesture-press-tracking.md, which still announces the pieces this pull request removes.
[Automated review]
| * A control's release curve: UIButton's, UIKit's easeInEaseOut | ||
| * (`button.highlight.releaseCurve`). | ||
| */ | ||
| const CONTROL_RELEASE_EASE = pressConsts.controlReleaseEase; |
There was a problem hiding this comment.
The button table names timing.curves.move as UIButton's release curve, but this plays the literal, so a theme's curves: {'timing.curves.move': ...} never reaches the control release. With --motion-timing-curves-move: cubic-bezier(0, 0, 1, 1) on the root, Button's ::before still released on cubic-bezier(0.42, 0, 0.58, 1), while the duration role did reach it. spec:AST-094 FR4 asks for the role: var(--motion-timing-curves-move, cubic-bezier(0.42, 0, 0.58, 1)), as the cell release already does with its own curve role.
| // Motion fade, which no theme may lengthen (design:reduced-motion DR4); | ||
| // and the keyboard's own timing, which motion in step with the keyboard | ||
| // rides (spec:AST-094 FR4). | ||
| exclude: [ |
There was a problem hiding this comment.
press.flashMs and press.highlightInMs are not excluded here, so they become theme roles that astryx docs motion lists and defineTheme accepts. Nothing reads --motion-timing-press-flash-ms or --motion-timing-press-highlight-in-ms, though: the quick-tap flash runs PRESS_FLASH_MS (100 ms) in utils/pressGesture.ts. Either read the flash role in the controller's timerMs the way the fade does, or exclude both here if they are not motion (spec:AST-094 FR4).
| // fading. A selection hold (the row kept selected through a navigation | ||
| // push) hides them the same way until its deselect begins, and again | ||
| // after a cancel. With a mouse the dividers stay drawn. | ||
| borderBlockEndColor: { |
There was a problem hiding this comment.
On a device whose primary pointer is coarse, the controller also paints a mouse or trackpad press (pressButton in utils/pressFeedback.ts), and these arms key only on (pointer: coarse), so a trackpad press on a tablet hides both dividers. In phone emulation, a mouse press on the second row of the compact divided list turned both dividers rgba(0, 0, 0, 0). The comment above and list-dividers-hide-on-press.md say they stay drawn with a mouse, and spec:AST-017 FR59 asks for a named reason and fine-pointer evidence. Could this key on a finger's press, with a test of a mouse press under a coarse primary pointer?
| ); | ||
| if ( | ||
| row == null || | ||
| !isTouch(row.ownerDocument.defaultView) || |
There was a problem hiding this comment.
This checks only the device's primary pointer, so on a tablet a trackpad click that pushes from a row holds it selected, while list-selection-hold.md says the hold is null with a mouse (spec:AST-017 FR59). Checking how the activation was made, through the event's pointerType or a finger marker from the press controller, would match the changeset.
| PressTrackerOptions, | ||
| 'getBounds' | 'view' | ||
| > & { | ||
| /** |
There was a problem hiding this comment.
After this pull request the tracker paints nothing, so the reason given here ("The control paints press feedback per event") no longer holds. A tracker attached with perEventPointer still takes the host's listeners on a fine primary pointer, which current spec:AST-094 FR19 grants only to spec:AST-057's row swipe. No component uses it yet; could press consumers drop the option, keeping it for recognizers?
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: UIControl press tracking with control and cell profiles, the shared press paint, delegated press tracking and the gesture hooks. Not exported from the package entry point; no component uses them yet. |
There was a problem hiding this comment.
This pull request removes the tracker's control and cell profiles, the shared press paint and the delegated press tracking, so a release that carries the stack would list them beside press-machine-one-writer.md, which says they are gone. Could this entry describe only what remains?
…nothing uses With the delegated press folded into utils/pressFeedback.ts, nothing adds a delegated consumer type: the host's addDelegatedConsumer, its attribute walk on the hit path and the DelegatedConsumerType export go (meta#8379's c2cd820, reapplied at this layer). Co-authored-by: Christopher Chedeau <vjeux@users.noreply.github.com>
…e short From the press convergence (meta#8379, superseded): with one writer of the press attribute, any write ends a fade another painter left running, so a press released while it runs fades on its own clock. The test fails when writePress leaves that fade running. The ButtonPress interaction spec's header names the press feedback instead of the removed delegated press.
|
Closing as superseded. This PR's press convergence (one writer of the press attribute in |
Depends on #8118 and #8356 (and their predecessors); review the last three commits.
Maintainer impact
Two press machines and writers paint
data-astryx-press: the controller behindusePressFeedback(utils/pressFeedback.ts), and the gesture system's delegated press and painter (gestures/delegatedPress.ts,gestures/pressPaint.ts), which no component installs. Each press change lands twice; a selection hold writes the attribute directly.Intended invariant
One press machine and one writer of the press attribute, as settled in #8073's review;
usePressFeedback's API, every press's paint, clocks and release, and #8118's layers stay as they are (design:user-statesDR1, DR5;spec:AST-094FR11).Change
paintPressandreadPressPaintmove intoutils/pressFeedback.ts, and the controller writes through them: a fade's clock and the attribute never drift apart, and a press never overwrites aheldrow.selectionHold.tswrites and clearsheldthroughpaintPress.gestures/delegatedPress.tsandgestures/pressPaint.tsare removed, with their internal exports.PressTrackerreports UIControl's control events and paints nothing; the gesture host drops its delegated consumer, which nothing uses.PressTrackerandusePressTrackerlose their highlight options andPRESS_PROFILES; no package entry point exports them.spec:AST-094FR41's native long-press selection, FR24's coarse-only touch listeners,subscribePressPaint); this PR dropsisPressFeedbackInstalledwith its only caller, and the one writer keeps reporting only the controller's paints.By design (
spec:AST-094FR11): a control's press ends when the browser takes the touch, and activation stays the browser's click; UIControl's tracking out to 70 pt and its re-highlight on the way back are not ported.Evidence
gestures/pressPaint.tsand the controller), another painter's fade cut the controller's release short; the new test fails there.Scope
Testing
pnpm -F @astryxdesign/core typecheck;pnpm -F @astryxdesign/lab typecheck;pnpm check:repo;pnpm check:changesets; lint and prettier on touched files.pnpm vitest run scripts/build-css.test.mjs packages/core/src/{utils,gestures,hooks,Item,List,TreeList,Button,ClickableCard,SelectableCard,Thumbnail,IconButton,ToggleButton,motion,RadioList,CheckboxList,Switch,TabList,AlertDialog,DropdownMenu} packages/lab/src internal/interaction-harness/src: 4,069 pass (DropdownMenuLazySheet, whosebeforeAllimport can pass its 10 s hook timeout under the full parallel run, passes alone). The CLI discovery and doc suites: 1,102 pass.pnpm storybook:build, thenpnpm test:interaction --project chromium Button List Item TreeList ClickableCard SelectableCard gestures motion internal/interaction-harness: 128 pass, 3 skip.node internal/interaction-harness/scripts/webkit-container.mjs --config playwright.interaction.config.ts --project=webkit ButtonPressRelease ButtonPress ListRowPress gestures: 37 pass, 19 skip.pnpm exec playwright test PressModality.a11y CardPress.a11y PressedState.a11y MenuPress.a11y --project chromium: 30 pass.Stack (merge in this order): #8118's stack, then #8270 → #8309 → #8315 → #8336 → #8320 → #8293 → #8356 → this PR.