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
meta-cla
Bot
added
the
CLA Signed
ed0fcb5 to
3f6d699
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is on hold: the press changes here rest on the new draft spec:AST-094, namely UIKit as the touch reference (DEC-1), the compositor release (FR5) and the control press profile (FR11). That includes UIButton's 250 ms release and Button's ::before becoming its press layer, and no engineering owner has approved the record for this head. 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 what in the record should change. Separately from that decision:
- Land the earlier pull requests in the stack, or retarget this one, so the diff under review is this change's 1,064 lines instead of 29,396.
- Fix FR7's citation of
spec:AST-090, which does not exist in the repository. - Say in the changesets that a caller's
::beforeon Button now merges with the press layer and stays atopacity: 0, and that an iPad mini in portrait switches from the bottom sheet to the popover.
[Automated review]
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
No spec:AST-090 exists in docs/specs at this head or on main, so an owner can't read the FR1 and FR10 this amendment departs from. Cite the record's real id and path, or drop the clause until that record lands.
There was a problem hiding this comment.
This citation is in the AST-094 record from #8052, which this PR stacks on. The finding has gone to that record's owner.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A button's or a card's touch press release now runs on the compositor: it keeps fading while the page's script is busy instead of freezing at full strength and snapping off, and it paints nothing per frame. The release follows UIButton's highlight release (250 ms, ease-in-out); a list row's stays on the deselect clock. The pressed colour and the mouse states are unchanged. |
There was a problem hiding this comment.
This doesn't tell callers that Button's ::before is now the press layer, with isolation: isolate on the button. StyleX merges xstyle per property, so a caller's ::before keeps the layer's opacity: 0, z-index: -1 and inset: 0, and is invisible at rest unless it sets opacity. Worth saying here and in Button's docs.
There was a problem hiding this comment.
Fixed in 4dc4418: the changeset and Button's docs now say that Button paints its touch press on its own ::before and sets isolation: isolate. A ::before styled through xstyle merges into that layer, which rests at opacity: 0; set its opacity or style a child element instead. Moving the layer to an element Button owns stays open in the owner review named in the body.
| '@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.
On the base, (max-width: 768px) and (pointer: coarse) gives a 744 px portrait iPad mini the bottom sheet. The new (width < 744px) and (pointer: coarse), (height < 500px) and (pointer: coarse) gives it the popover, so it switches rather than keeps the popover. This is also Selector's default.
There was a problem hiding this comment.
This changeset belongs to #8062 (the touch presentation resolver), which this PR stacks on. The finding has gone to that PR's owner.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsButton (@astryxdesign/core) · View in Storybook
ClickableCard (@astryxdesign/core) · View in Storybook
Item (@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.1KB |
+805B |
| core/AppShell | +2.9KB |
+811B |
| core/AvatarGroupOverflow | +2.5KB |
+234B |
| core/Banner | +3.1KB |
+807B |
| core/BaseTypeahead | +3.1KB |
+816B |
| core/BreadcrumbItem | +3.5KB |
+1.0KB |
| core/Button | +3.1KB |
+806B |
| core/Calendar | +3.1KB |
+801B |
| core/Carousel | +3.1KB |
+805B |
| core/ChatComposer | +3.1KB |
+805B |
| core/ChatComposerInput | +3.1KB |
+808B |
| core/ChatComposerTokenElement | +3.1KB |
+808B |
| core/ChatDictationButton | +3.1KB |
+805B |
| core/ChatLayout | +3.2KB |
+803B |
| core/ChatLayoutScrollButton | +3.1KB |
+805B |
| core/ChatSendButton | +3.1KB |
+804B |
| core/CheckboxInput | +2.5KB |
+237B |
| core/CheckboxListItem | +3.1KB |
+767B |
| core/ClickableCard | +3.0KB |
+813B |
| core/CodeBlock | +3.1KB |
+819B |
| core/Collapsible | +2.5KB |
+260B |
| core/CommandPalette | +2.6KB |
+236B |
| core/CommandPaletteItem | +2.5KB |
+264B |
| core/ComplexSelector | +3.2KB |
+799B |
| core/ContextMenu | +3.5KB |
+1.1KB |
| core/ContextMenuItem | +3.1KB |
+805B |
| core/DateInput | +3.3KB |
+807B |
| core/DateRangeInput | +3.3KB |
+794B |
| core/DateTimeInput | +3.3KB |
+818B |
| core/DialogHeader | +3.2KB |
+807B |
| core/DrawerHeader | +3.2KB |
+806B |
| core/DropdownMenu | +3.5KB |
+1.2KB |
| core/DropdownMenuCheckboxItem | +3.0KB |
+637B |
| core/DropdownMenuItem | +3.0KB |
+657B |
| core/DropdownMenuRadioItem | +3.0KB |
+637B |
| core/DropdownMenuSubMenu | +2.9KB |
+631B |
| core/FileInput | +3.2KB |
+795B |
| core/IconButton | +3.1KB |
+806B |
| core/Item | +3.2KB |
+659B |
| core/LayerProvider | +3.1KB |
+812B |
| core/Lightbox | +3.1KB |
+808B |
| core/Link | +2.4KB |
+237B |
| core/ListItem | +3.0KB |
+767B |
| core/Markdown | +3.2KB |
+1.1KB |
| core/MobileNav | +3.1KB |
+809B |
| core/MobileNavToggle | +3.1KB |
+803B |
| core/MoreMenu | +3.4KB |
+1.2KB |
| core/MultiSelector | +3.1KB |
+796B |
| core/NumberInput | +3.2KB |
+805B |
| core/Outline | +2.4KB |
+261B |
| core/Pagination | +3.3KB |
+1.0KB |
| core/Popover | +2.9KB |
+797B |
| core/PowerSearch | +3.4KB |
+1.0KB |
| core/RadioListItem | +3.0KB |
+626B |
| core/SegmentedControlItem | +2.5KB |
+257B |
| core/SelectableCard | +3.0KB |
+813B |
| core/Selector | +3.3KB |
+1.0KB |
| core/SelectorOption | +3.1KB |
+656B |
| core/SideNav | +3.1KB |
+809B |
| core/SideNavCollapseButton | +3.1KB |
+802B |
| core/SideNavHeading | +3.0KB |
+809B |
| core/SideNavItem | +3.1KB |
+812B |
| core/Step | +2.5KB |
+252B |
| core/Stepper | +3.2KB |
+803B |
| core/Switch | +2.5KB |
+234B |
| core/Tab | +2.5KB |
+253B |
| core/TabMenu | +3.1KB |
+801B |
| core/Table | +3.4KB |
+1.1KB |
| core/TableCell | +3.5KB |
+1.1KB |
| core/TableHeaderCell | +3.5KB |
+1.2KB |
| core/TableSelectionToolbar | +3.1KB |
+812B |
| core/TextInput | +3.1KB |
+808B |
| core/Thumbnail | +3.1KB |
+800B |
| core/TimeInput | +3.2KB |
+799B |
| core/Toast | +3.1KB |
+806B |
| core/ToggleButton | +3.1KB |
+806B |
| core/Token | +2.5KB |
+262B |
| core/Tokenizer | +2.8KB |
+804B |
| core/TopNav | +3.2KB |
+819B |
| core/TopNavHeading | +3.0KB |
+816B |
| core/TopNavItem | +2.5KB |
+261B |
| core/TopNavMegaMenu | +3.1KB |
+810B |
| core/TopNavMegaMenuItem | +2.5KB |
+261B |
| core/TopNavMenu | +3.0KB |
+812B |
| core/TreeList | +3.1KB |
+657B |
| core/Typeahead | +3.2KB |
+800B |
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 | 34.6KB | 35.7KB | +1.2KB |
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/3 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/11 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
55 of 502 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 | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| 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 35 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
3f6d699 to
068e8b1
Compare
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: the press changes rest on draft spec:AST-094 (DEC-1, FR5, FR11's 250 ms ease-in-out control release, FR13), and Button's ::before becoming its press layer has no current spec. Any engineering owner listed in .github/ENGOWNERS 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 the decision, please address the points below.
- Narrow the diff to this change (1,064 of the 29,399 authored lines) by landing #8117 and its predecessors or retargeting onto its branch.
.changeset/press-release-compositor.mdand.changeset/list-row-press-compositor.mdshould say a caller's own::beforemerges into the press layer and stays hidden at rest, not that it replaces the press paint..changeset/touch-presentation.mdshould say an iPad mini in portrait now gets the anchored popover instead of the sheet.spec:AST-094FR7 citesspec:AST-090, which is not onmain.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A button's or a card's touch press release now runs on the compositor: it keeps fading while the page's script is busy instead of freezing at full strength and snapping off, and it paints nothing per frame. The release follows UIButton's highlight release (250 ms, ease-in-out); a list row's stays on the deselect clock. The pressed colour and the mouse states are unchanged. |
There was a problem hiding this comment.
Button's ::before now carries the press layer (and Button gains isolation: isolate). A caller who styles Button's ::before through xstyle keeps the layer's opacity: 0, inset: 0 and z-index: -1, so their content is hidden until the button is pressed. Please say so here, with the migration a released styling change needs, or move the layer to an element Button owns.
There was a problem hiding this comment.
Fixed in 4dc4418: the changeset and Button's docs now say that Button paints its touch press on its own ::before and sets isolation: isolate. A ::before styled through xstyle merges into that layer, which rests at opacity: 0; set its opacity or style a child element instead. Moving the layer to an element Button owns stays open in the owner review named in the body.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A list row's touch press release keeps fading while the page runs script. `ListItem`, `Item` and `TreeList` rows used to stay fully pressed after the finger lifted until the main thread was free, and every frame of the fade repainted the row; the release now runs on the compositor, with the same color, clock and curve as before. On an interactive row the press paints on the row's `::before`, so a row's own `::before` set through `xstyle` or `className` replaces the press paint. |
There was a problem hiding this comment.
A row's own ::before does not replace the press paint: StyleX merges ::before property by property, so an xstyle ::before keeps the layer's opacity: 0 and is hidden at rest, and a className rule can at most override the properties it sets.
There was a problem hiding this comment.
This changeset belongs to #8077 (list rows), which this PR stacks on. The finding has gone to that PR's owner.
| '@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.
At 744 x 1133 with a coarse pointer the base matched (max-width: 768px) and (pointer: coarse) and opened the bottom sheet; this head opens the anchored popover. Suggest: "An iPad mini in portrait now gets the anchored popover instead of the sheet."
There was a problem hiding this comment.
This changeset belongs to #8062 (the touch presentation resolver), which this PR stacks on. The finding has gone to that PR's owner.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
spec:AST-090 is not on main (no docs/specs/AST-090* record), so an approver cannot see which FR1 and FR10 this departs from. Please cite the record that holds them, or drop the clause; line 235 repeats the citation.
There was a problem hiding this comment.
This citation is in the AST-094 record from #8052, which this PR stacks on. The finding has gone to that record's owner.
068e8b1 to
add4faf
Compare
There was a problem hiding this comment.
This is on hold for a spec decision, and three fixes are needed either way. Approval needs an engineering owner to approve the draft spec:AST-094 direction this change rests on (UIKit as the touch reference, releases on the compositor, and UIButton's 250 ms ease-in-out control release, which amends design:user-states DR1/DR3) and Button's ::before becoming its press layer. An engineering owner records that decision with /approve-spec <full head SHA> (the author may, as an engineering owner), or says which part should change.
- Land #8117 and its predecessors first, or retarget onto its branch: this pull request carries 31,222 authored lines.
- Correct the changesets:
touch-presentation.mdsays an iPad mini in portrait keeps the popover, but it moves from the sheet to the popover;list-row-press-compositor.mdandpress-release-compositor.mdmisdescribe or omit how a caller's::beforemerges into the press layer. - Fix the citation of
spec:AST-090inspec:AST-094FR7 and its impact list; no such record exists.
[Automated review]
add4faf to
4dc4418
Compare
There was a problem hiding this comment.
This is on hold for an engineering-owner decision, and several fixes are needed either way. The press changes rest on the draft spec:AST-094 (UIKit as the touch reference, releases on the compositor, and UIButton's 250 ms ease-in-out release for every non-cell touch press), and no record yet governs Button's ::before press layer or the new usePressFeedback profile option, holdSelection and pressLayer.stylex exports: an engineering owner approves or amends that direction by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner) and names the record for the layer and the exports.
- Under a finger a RadioList row now paints the whole row instead of its circle, which
design:user-statesDR1 rules out, so the indicator fix from #8233 has to land in this pull request or before it. - The new compact test (
(width < 744px)intouchPresentation.ts) contradictsspec:AST-012FR9: an iPad mini in portrait loses the bottom sheet, so build it from the theme's compact definition. - Land #8117 and its predecessors first, or retarget onto its branch, since this pull request carries 31,362 authored lines.
- Correct the changesets (the iPad mini line in
touch-presentation.md, "replaces" inlist-row-press-compositor.md, and the missingclassNamecase and other controls' new clock inpress-release-compositor.md) and thespec:AST-090citation inspec:AST-094FR7.
[Automated review]
| export const TOUCH_QUERY = '(pointer: coarse)'; | ||
|
|
||
| /** The viewport is compact: layout behaviours. */ | ||
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; |
There was a problem hiding this comment.
Current spec:AST-012 FR9 and public-component-api INV17 define compact as width below the theme's effective md point (768 by default) or height below 500, and say no resolver keeps numbers of its own. With (width < 744px), coarse-pointer screens 744-767 px wide (an iPad mini in portrait) now get the anchored popover from Selector, MultiSelector, DropdownMenu, ContextMenu and MoreMenu where FR9 gives the bottom sheet. Build this from the theme's compact definition instead.
| // The touch press on a compositor layer (after the overlay, whose | ||
| // own touch paint it turns off): the release keeps fading while | ||
| // script runs. | ||
| isInteractive && pressLayerStyles.cell, |
There was a problem hiding this comment.
With the press on this ::before cell layer, a RadioList row no longer paints its circle: RadioListItem suppresses Item's background colour and image but not this layer, and the circle reads --_press-paint-image, which the layer turns transparent. Under a finger the whole row paints instead, against design:user-states DR1 ("RadioList · Indicator paints --color-overlay-pressed"). The fix in #8233 depends on this pull request, so it needs to land with this change rather than after it.
There was a problem hiding this comment.
Kept as a separate pull request, one intent each: #8233 gives the RadioList and CheckboxList indicators their own press layer and lands with or right after this one. Both bodies state that order.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A button's or a card's touch press release now runs on the compositor: it keeps fading while the page's script is busy instead of freezing at full strength and snapping off, and it paints nothing per frame. The release follows UIButton's highlight release (250 ms, ease-in-out); a list row's stays on the deselect clock. The pressed colour and the mouse states are unchanged. Button now paints the touch press on its own `::before` and sets `isolation: isolate`: a `::before` you style through `xstyle` merges into that layer, which rests at `opacity: 0`, so set its `opacity` or style a child element instead. |
There was a problem hiding this comment.
This names only xstyle, but a className rule on Button's ::before also keeps the layer's opacity: 0 and z-index: -1 and is hidden at rest. It also covers "a button's or a card's" release, while the overlay's default arm moves the other control surfaces (Switch, Link, Tab, CheckboxInput and more) to 250 ms ease-in-out as well.
There was a problem hiding this comment.
Fixed in 4f42af6: the changeset names a className rule as well as xstyle (the layer rests at opacity: 0 and z-index: -1), and says every control on the shared overlay, Switch, Link, Tab and CheckboxInput included, now releases on UIButton's 250 ms ease-in-out.
…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.
…tion constant From review on meta#8890: spec:AST-094 FR4 lists the 200 ms Reduce Motion cap among the touch motion roles and says a component never plays a literal. The release capped its clock at a literal 200 in pressFeedback.ts (REDUCED_MOTION_MAX_MS) and at a literal 200ms in the overlay's reduced release duration. Both now read the shared constant: the controller timing.reducedMotionCrossFadeMs, the stylesheet reducedMotionFadeConsts.duration. Both are 200 ms, so nothing on screen changes; the built stylesheet's cap is still 200 ms. Red first: a unit case that stubs timing.reducedMotionCrossFadeMs at 120 ms found the release still fading at 120 ms on the change before this one; it now ends there.
…Motion constant too The row press layer capped its release under reduced motion at a literal 200ms (CELL_RELEASE_REDUCED), the same cap the review on meta#8890 asked the overlay to read from the shared constant (spec:AST-094 FR4: the 200 ms Reduce Motion cap is a role, never a literal). It now reads reducedMotionFadeConsts.duration, as the overlay does; both are 200 ms, and the built stylesheet's cap is still 200 ms. A unit case holds every reduced-motion cap in the layer and the overlay to the constant. Red first: it found the layer's literal on the change before this one.
…ame window Both 'a dip held before its first frame lands once the hold is …' proofs waited for no running animation, then one frame and a timeout, assuming the next step starts within a frame. The dip lands on a timer due on the call's own clock, though, and the proofs passed only because the evaluate round trips covered the gap: in WebKit on a loaded box the window closed first and the proof read the element mid-dip (opacity 0, translate none; reproduced 1 in 4 with the round trips removed). They now wait on the call's own finished, then on the browser's own animations still running or waiting for their first frame, and read nothing from the animator.
…ment at once and settles then A Reduce Motion dip whose landing runs more than its whole fade back in late (a hold released long after the dip was due) played a 1 ms opacity animation, which WebKit keeps pending until its next frame, and settled the call on a 1 ms timer: the call's finished could resolve while the element still showed opacity 0. The animator's notes say the element appears at once there. Fails now: the landing plays one opacity animation and the call has not settled.
…or the Space key's 200 ms, a list row's 200 ms Measures each release on a touch-first device (component:Button FR15): the release animation's own duration, read when `fading` is written; `fading` lasting at least that clock; and `fading` going within a few frames of the animation's end. Frames, not ms, bound the end, so a loaded machine cannot fail it. Passes in Chromium and WebKit on this head, before the fold onto meta#8117's new head, and is kept through it as the proof that the fold changes no timing.
…#8890, meta#8360 045f666) into the press release The release's 250 ms for a finger on a control stays where it was, in the control press layer's CSS, and a mouse's or the Space key's press keeps 200 ms through its input marker. The controller's own profile clock goes with the profiles meta#8117 dropped: the release-end code from meta#8890 waits for the running release animation, so a finger's release still ends at 250 ms. This branch's own copy of that code gives way to meta#8890's.
…element at once and settles A Reduce Motion landing that runs at least the whole fade back in late had nothing left of the cross-fade, but still played a 1 ms opacity animation and settled the call 1 ms later. WebKit keeps that animation pending until its next frame, so the call's finished could resolve with the element still at opacity 0. Such a landing now writes the end opacity at once (handing a borrowed one back to the stylesheet), plays nothing, and settles the call then, as the animator's notes say.
…ot the timing table From review (an inline comment on meta#8077 about this line): the press controller, which every pressable component loads, imported the whole UIKit timing table for one value, its reduced-motion cap, while architecture:component-style-authoring INV6 ships a component only the rows of a shared table it uses. It now reads the fade's own export, reducedMotionCrossFade.durationMs, as the animator does; the value is the same 200 ms. Red first: a bundle of pressFeedback.ts carried the table's other rows (stateChangeMs, buttonReleaseMs) on the change before this one; it now carries the fade alone.
…th dividers drawn From review on meta#8077: component:List FR11's verification asks for a pressed row between two divided rows, and no case used a list with dividers. A touch case opens the Interactive story with hasDividers, holds the middle row and releases it, and checks the divider above it and its own divider below, by computed border and by the pixels of each divider's strip, while held, halfway through the release fade and after it; each strip must differ from the row's surface just inside it at rest, so a match is a drawn divider. It passes in Chromium and WebKit. A page style that hides a pressed row's divider fails it while the row is held.
…duce Motion constant meta#8077's FR4 check (interactionOverlay.cell.test.ts) asks every release cap in the overlay to read the Reduce Motion constant through a min(). This branch's reduced-motion arms used the constant directly; they now take min(<the release's clock>, the constant), so a control's 250 ms and a mouse's or the Space key's 200 ms both cap at 200 ms, as before. The build's reduced-motion check reads the cap.
… or press profile The selection hold and the press profiles are gone from the base, so the Item case and the shared checks' notes no longer mention a held arm, a cell marker or a profile clock; the checks themselves are unchanged.
… judges its first frame on that frame's time 'a spring started right before a block plays in full after it' waited a fixed 300 ms of wall clock after the block, then read the screencast. In the full interaction run on a loaded box no moving frame had reached the screencast by then, and the proof failed on an empty read. It now waits until the page shows the box a quarter of the way along and three more frames reach the screencast. Its bound changes form, not strength: the curve's own start (the handle's started) must come after the block, and the first moving frame may be no further along than a curve started a frame before the block ended, at that frame's own time, which is the old 30 px for a frame right after the block and still holds when a loaded machine sends frames sparsely. A curve dated at the call is 107 px in.
There was a problem hiding this comment.
This is on hold for an engineering-owner decision on the rewritten component:Button FR15, and some changes are needed either way. Current FR15 says a finger's press fades out over 200 ms and is instant under reduced motion; this pull request paints it on Button's own ::before and fades it over 250 ms under a coarse pointer (a 200 ms cross-fade under Reduce Motion) and gives every other control on the shared overlay the same 250 ms release, so an engineering owner needs to approve or amend that text by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. Separately:
- Please land #8117, #8338 and #8350 first and rebase: the diff carries 32,770 authored lines, and this change's own 2,400 still need a split or a justification.
- FR15 groups a pen with the mouse, but the press controller runs a pen as a finger, so on a tablet it gets the 250 ms layer release; please make the two agree and add a pen case to the timing spec.
- The stacked gesture host still attaches nothing where the primary pointer is fine, which current
spec:AST-094FR19 and FR43 rule out; that fix belongs in the gesture pull request.
[Automated review]
| | FR12 | With `tooltip`, the tooltip attaches to the Button element itself (no wrapper element), opens above it, and its content is added to `aria-describedby` after any caller value. | Current source; real-browser accessibility test | | ||
| | FR13 | A standalone Button paints the shadow token for its `elevation`. Inside a ButtonGroup it paints no shadow and reflects `data-elevation="none"`, whatever `elevation` says. | `family:buttons` FR8–FR10; current source and tests | | ||
| | FR14 | The root carries `astryx-button` and reflects the effective `data-variant`, `data-size`, and `data-elevation`. | `family:buttons` FR10; `architecture:component-theming-surface`; tests | | ||
| | FR15 | Hover paints the shared interaction overlay on hover-capable pointers, and a press paints the shared pressed overlay through the press controller: at once under a mouse or a pen (`spec:AST-094` FR11's fine-pointer profile) and on FR11's control profile under a finger, fading out over 200 ms on release at `tempo` 1, scaled by the theme's `tempo` (`spec:AST-108` FR4). Under a coarse primary pointer the overlay paints on Button's own `::before`, present while pressed and through its fade-out, and a touch press fades over UIButton's measured release instead (250 ms at `tempo` 1, `timing.press.buttonReleaseMs`); a Space press keeps the 200 ms fade, as every press does with a fine primary pointer (`spec:AST-017` FR59). Without a reduced-motion preference, a standalone Button also scales to 0.98 on a mouse press, or while the touch press reads on; under reduced motion it MUST NOT scale (`design:reduced-motion` DR1). All of this paint follows the element's exposed disabled state: a Button that is natively disabled or exposes `aria-disabled="true"` (busy-only, disabled with a tooltip, or a caller's own `aria-disabled`) shows no overlay and no press scale, even when it stays operable. Inside a ButtonGroup the press scale does not apply. Button's hover and press transitions are fast. Under reduced motion they are instant with a fine primary pointer, and a touch release under a coarse primary pointer cross-fades within 200 ms instead (`design:reduced-motion` DR2, DR3). Implementation gap: the 0.98 press scale also applies under reduced motion; only its transition becomes instant. | Current source and tests; `spec:AST-094` FR11 and DEC-21 for the mouse and pen press | |
There was a problem hiding this comment.
This rewrites current FR15, which says a finger's press fades "out over 200 ms on release" and that press transitions are "instant under reduced motion": under a coarse primary pointer the press now paints on Button's own ::before, present through its fade-out, a touch press fades over 250 ms, and a reduced-motion touch release cross-fades within 200 ms. An engineering owner needs to approve or amend it by commenting /approve-spec <full head SHA>. Two points ride on it: the sentence keeps a pen with the mouse, while the controller presses a pen as a finger, and "250 ms at tempo 1" implies a tempo scaling the fixed 250ms in controlPressLayer.ts does not do (main's 200 ms has the same gap).
| // A mouse or a key keeps the fine pointer's 200 ms release on any surface; | ||
| // the marker tells the stylesheet's release arms the same. | ||
| element.setAttribute( | ||
| PRESS_INPUT_ATTRIBUTE, | ||
| pointerId == null ? 'key' : 'mouse', | ||
| ); |
There was a problem hiding this comment.
Only pressButton marks a press, and onPointerDown sends every pointer that is not a mouse down the finger path (lines 722-777), so on a tablet a pen gets Button's ::before layer and the 250 ms release, while the rewritten FR15 groups the pen with the mouse. Either route pointerType === 'pen' here with its own marker so it keeps 200 ms, or say in FR15 that a pen on a coarse primary pointer presses as a finger, and add the pen case to ButtonPressTiming.
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - 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 | ||
| * document, window or element listener, no `touch-action` or other style, | ||
| * nothing delivered, so a finger on a touchscreen laptop gets the | ||
| * browser's defaults. The host follows the query: turning coarse installs | ||
| * it and styles every registration; turning fine cancels every touch in | ||
| * flight and removes it all. A development warning names the first attach | ||
| * under a fine pointer. |
There was a problem hiding this comment.
Current spec:AST-094 FR19 says "Recognizers attach for every pointer, with FR43's exceptions", and FR43 that gestures attach for "a mouse, a pen, and a finger, not only a coarse primary pointer"; this host installs nothing where the primary pointer is fine, and cites FR19 for it. No shipped component attaches the host in this diff, but the fix belongs in the gesture pull request before this one rebases onto it.
…e surface releaseAnimationsOf counted only the press owner's own animations (on it, or its ::before or ::after). A component that draws its release on a descendant, such as CheckboxInput's release on its box wrapper's ::after, then lost the end of it: fading went at the 200 ms clock while the 250 ms release still ran, and the frame it went relaid and repainted the box. It now also counts a CSS release animation on a descendant whose nearest press owner (closest [data-astryx-pressable]) is this element, the same nearest-owner rule the layers read by, so a press owner nested inside plays its own press's release and never extends this one. Red first: a unit case whose release runs on a box inside the row found fading gone at the clock on the change before this one; it now holds to the release's end. A second case keeps a nested press owner's release, on it or inside it, out of the row's.
…ts own timeline One run measured a 250 ms release whose `fading` lasted 241.6 ms on the page's clock: an animation's start is a frame's time, which can come before the write, so a bound on the page's clock read a whole release as short. The check now takes the animation's start from its own timeline (when it is ready) and the timeline's time when `fading` goes, and asks that it ran to its end; WebKit drops the animation at the removal, so its progress cannot be read there.
There was a problem hiding this comment.
This is on hold for an engineering-owner decision on the rewritten component:Button FR15, and some changes are needed either way. Current FR15 says a finger's press fades out over 200 ms and is instant under reduced motion, while this pull request paints it on Button's own ::before, fades it over 250 ms under a coarse pointer (a 200 ms cross-fade under Reduce Motion) and gives every other control on the shared overlay the same 250 ms release, so an engineering owner needs to approve or amend that text by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. Separately:
- Please land #8360, #8890, #8077, #8117 and #8350 first and rebase: the diff carries 33,092 authored lines, and this change's own 2,570 still need a split or a justification.
- FR15 groups a pen with the mouse, but the press controller runs a pen as a finger, so on a tablet it gets the 250 ms layer release; please make the two agree and add a pen case to the timing spec.
- The stacked gesture host still attaches nothing where the primary pointer is fine, which current
spec:AST-094FR19 and FR43 rule out; that fix belongs in the gesture pull request.
[Automated review]
| | FR12 | With `tooltip`, the tooltip attaches to the Button element itself (no wrapper element), opens above it, and its content is added to `aria-describedby` after any caller value. | Current source; real-browser accessibility test | | ||
| | FR13 | A standalone Button paints the shadow token for its `elevation`. Inside a ButtonGroup it paints no shadow and reflects `data-elevation="none"`, whatever `elevation` says. | `family:buttons` FR8–FR10; current source and tests | | ||
| | FR14 | The root carries `astryx-button` and reflects the effective `data-variant`, `data-size`, and `data-elevation`. | `family:buttons` FR10; `architecture:component-theming-surface`; tests | | ||
| | FR15 | Hover paints the shared interaction overlay on hover-capable pointers, and a press paints the shared pressed overlay through the press controller: at once under a mouse or a pen (`spec:AST-094` FR11's fine-pointer profile) and on FR11's control profile under a finger, fading out over 200 ms on release at `tempo` 1, scaled by the theme's `tempo` (`spec:AST-108` FR4). Under a coarse primary pointer the overlay paints on Button's own `::before`, present while pressed and through its fade-out, and a touch press fades over UIButton's measured release instead (250 ms at `tempo` 1, `timing.press.buttonReleaseMs`); a Space press keeps the 200 ms fade, as every press does with a fine primary pointer (`spec:AST-017` FR59). Without a reduced-motion preference, a standalone Button also scales to 0.98 on a mouse press, or while the touch press reads on; under reduced motion it MUST NOT scale (`design:reduced-motion` DR1). All of this paint follows the element's exposed disabled state: a Button that is natively disabled or exposes `aria-disabled="true"` (busy-only, disabled with a tooltip, or a caller's own `aria-disabled`) shows no overlay and no press scale, even when it stays operable. Inside a ButtonGroup the press scale does not apply. Button's hover and press transitions are fast. Under reduced motion they are instant with a fine primary pointer, and a touch release under a coarse primary pointer cross-fades within 200 ms instead (`design:reduced-motion` DR2, DR3). Implementation gap: the 0.98 press scale also applies under reduced motion; only its transition becomes instant. | Current source and tests; `spec:AST-094` FR11 and DEC-21 for the mouse and pen press | |
There was a problem hiding this comment.
This rewrites current FR15, which says a finger's press fades "out over 200 ms on release" and that press transitions are "instant under reduced motion": under a coarse primary pointer the press now paints on Button's own ::before through its fade-out, a touch press fades over 250 ms, and a touch release under Reduce Motion cross-fades within 200 ms. An engineering owner needs to approve or amend this text by commenting /approve-spec <full head SHA>; the pen also needs settling (see the comment on pressFeedback.ts).
| // A mouse or a key keeps the fine pointer's 200 ms release on any surface; | ||
| // the marker tells the stylesheet's release arms the same. | ||
| element.setAttribute( | ||
| PRESS_INPUT_ATTRIBUTE, | ||
| pointerId == null ? 'key' : 'mouse', | ||
| ); |
There was a problem hiding this comment.
Only pressButton writes this marker, and onPointerDown sends every pointer that is not a mouse down the finger path (lines 734-789), so on a tablet a pen gets the ::before layer and the 250 ms release while FR15 groups the pen with the mouse. Either route pointerType === 'pen' here with its own marker so it keeps 200 ms, or say in FR15 that a pen on a coarse primary pointer presses as a finger, and add a pen case to ButtonPressTiming.interaction.browser.spec.ts.
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - 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 | ||
| * document, window or element listener, no `touch-action` or other style, | ||
| * nothing delivered, so a finger on a touchscreen laptop gets the | ||
| * browser's defaults. The host follows the query: turning coarse installs | ||
| * it and styles every registration; turning fine cancels every touch in | ||
| * flight and removes it all. A development warning names the first attach | ||
| * under a fine pointer. |
There was a problem hiding this comment.
Current spec:AST-094 FR19 says "Recognizers attach for every pointer, with FR43's exceptions", and FR43 that gestures attach for "a mouse, a pen, and a finger, not only a coarse primary pointer"; this header, refresh and applyStyle install nothing where the primary pointer is fine. This is #8360's code, so it needs fixing there before this pull request rebases onto it.
Depends on #8117 (on #8077, which carries #8890 and #8360) and #8350. Review only this PR's change: vjeux/astryx@uikit/c01-pr4-review-base...uikit/c01-press-release (base: #8117 with #8077 and #8360); of its 42 files, #8350's six (
packages/cli/**,cli-component-name-case.md) are theirs, andpress-paint-reduced-motion.mdis the reduced-motion fade #8338 proposed, closed in favor of this PR.#8233 lands with or right after this PR: its indicator layer takes
controlPressConsts.controlReleaseEase(internal,utils/controlPressLayer.stylex.ts).User impact
Anyone tapping a button, card or row on a touch-first device: while script runs after a tap, the release stays pressed, then snaps, or is cut off partway.
Problem and solution fit
Under a coarse primary pointer, Button paints the press on a
::before(controlPressLayerStyles.control, internal) and the cards fade their::after(controlOver): an opacity animation on UIButton's release (250 ms ease-in-out), on the compositor; under reduced motion, the Reduce Motion cross-fade. Button's layer andisolationexist only during a press. Any other pointer is unchanged: no layer, noisolation, the 200 ms release; a mouse or Space press on a touch-first device keeps it too.Expected behavior and authority
component:ButtonFR15 (this PR's touch release, on #8646's text);usePressFeedback's docs;design:user-statesDR1, DR5;design:reduced-motion; UIButton's release (motion/uikit/button.ts); the desktop rule,spec:AST-017FR59/DEC-25.Smallest restoration
control(Button) andcontrolOver(cards) inutils/interactionOverlay.stylex.ts.data-astryx-press-input); a list row keeps its own layer's 200 ms.endAfterRelease), not on a clock from the lift that a busy first frame outruns.:activeunchanged.component-size.js, gzip, base → head): Button JS 47.1 KB (+1.0 KB), CSS +738 B; ClickableCard and SelectableCard 16.1 KB (+843, +846 B), CSS +725 B; Thumbnail 54.7 KB (+990 B), CSS +733 B.Evidence
From the iOS simulator and the Android emulator (Android's hold injector-paced):
Seeked, the release matches UIButton's curve within 0.005; no Layout or Paint.
Scope
Testing
pnpm typecheck: 0 errors; lint: 0 errors;pnpm check:knowledge;pnpm test:ui(16,810 of 16,811; the one is MultiSelector's lazy bottom-sheet case, a load red test(Selector, MultiSelector): wait for the on-demand sheet module before the first open #8642 fixes) and the node project, whose only failures fail the same way on main locally (story-tree.test.ts,release/cache.test.mjs's lock cases). The local gates are the evidence: pushes carry[skip ci], so no CI run is expected.pnpm -F @astryxdesign/core build, thenpnpm vitest run scripts/build-css.test.mjs: 19 pass, including that UIButton's clock, curve andisolationsit behind the coarse gate, the control layers' release on::beforeand::after, the on arm's specificity, and the reduced-motion arms (DR2: each release capped by the Reduce Motion constant on a coarse pointer; DR3animation-name: noneoutranking the release otherwise).pnpm vitest run packages/core/src/utils/pressPaintReducedMotion.test.ts: the reduced-motion fade outranks a control's clock.pnpm storybook:build, then the Button, ListRowPress, ListRowPressPaths and ClickableCard interaction specs: Chromium 41 pass, 1 skip; WebKit (node internal/interaction-harness/scripts/webkit-container.mjs --config playwright.interaction.config.ts --project=webkit) 28 pass, 14 skip: the screencast, the rendering trace, native scrolling and the fine-pointer reduced-motion case are Chromium-only.ButtonPressTiming.interaction.browser.spec.ts: a finger's release on a button runs 250 ms, a mouse's and the Space key's 200 ms, and a list row's 200 ms, each read as the release animation's own duration, withfadinglasting the whole animation and going within three frames of its end. It passes in Chromium and WebKit on this PR's previous head and on this one, so moving onto the new base changes no timing.ButtonPressRelease"after a busy first frame, the release keeps a button's layer until its strength reaches 0" passes at the head in Chromium and WebKit.pnpm exec playwright teston the PressModality, PressedState, Button and CardPress a11y specs (Chromium): 37 pass.::beforeand noisolation, and a finger on a touch screen beside the mouse gets the strength's 200 ms release on the element, as before (ButtonPressRelease, 3 Chromium cases with(pointer: coarse)asserted false;Button.test.tsx, the three cards' tests andpressFeedback.test.tsin unit). All fail on the previous head (3570663ae8) and pass here.ButtonPressRelease"the release of a button does no Layout or Paint per frame" failed (Paint 2): the browser dropped the 125 ms press-scale transition's layer mid-release. Withwill-change: transformwhile the press attribute is on or fading, a trace shows Paint only at the touch and the release's start and end; 5 of 5 pass.::beforeand noisolation, so a::beforeits caller styles keeps its opacity, z-index and position:Button.test.tsx(layerOnlyWhilePressed) andButtonPressRelease("no press layer and no isolation at rest") fail on the previous head (fa86418) and pass here.motion/uikit/values.test.ts(from feat(core): the internal touch foundation: press tracking and the animator #8076) pins the press release curve topress.highlightOutCurve; it reads the fading arm outside the coarse-pointer and Reduce Motion media, since a control's coarse release plays UIButton's curve. It fails on the merged head without that and when the deselect's curve drifts.pressFeedback.test.ts"another painter's fade never cuts the controller's own release short": a fade another painter left running ends at the controller's write; it fails whenwritePressleaves that fade running.motion/uikit/button.tsno longer answersastryx component button:pnpm vitest run --project node packages/cli/api/component packages/cli/foundation/discoverypass.controlPressConsts.controlReleaseEasesits besidecontrolReleaseMsin the internalutils/controlPressLayer.stylex.ts, off core's published paths (UIButton's curve, for fix(core): a checkbox or radio press releases on the compositor #8233's indicator layer); pressFeedback.test.ts holds it tobutton.highlight.releaseCurve.Writing rubric results
component:Button(FR15: the touch press's::beforeand release, added to #8646's text):Stack (merge in this order): #8360 → #8890 → #8077 → #8117 → this PR; #8350 at any point before it.