Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
github-actions
Bot
added
the
needs:spec-owner-review
94b1c01 to
772101a
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request needs an engineering owner's spec decision before it can be approved, and it needs changes either way.
Spec decision. As filed against main, the branch rewrites component:Button FR15 (a 200 ms × tempo release) and design:reduced-motion DR2 (colors cross-fade on a touch screen), and it adds requirements main does not state: spec:AST-094 FR5's exit-token sentence, the press-release clauses in component:ClickableCard FR5, component:SelectableCard FR4 and component:Thumbnail FR4, component:List FR11 with DEC-6 (closing OQ1), and design:reduced-motion DEC-4. An engineering owner records the decision by commenting /approve-spec 772101aed922045524391d467b3e1c4d32982b30 on this pull request (the author may do so as an engineering owner), or the records can be approved in their own pull requests first.
Required changes:
- Rebase onto
main, because the branch conflicts with it and the auto-mergedpackages/core/src/utils/pressFeedback.tsdeclaresreleaseAnimationsOfandendAfterReleasetwice, sincemainalready holds #9758's change. .changeset/cards-touch-release-exit-tokens.mdpromises a release "capped at the press's release time" under a coarse pointer, but the head plays the shared release on every pointer with no cap..changeset/cards-touch-release-fine-pointer.mdannounces a fix for a fine-pointer finger release thatmainnever shipped, because the cards there already use the shared release..changeset/list-row-press-compositor.mdsays the row release keeps "the same color, clock and curve as before", while the row now plays--duration-exitand--ease-exit.- Miss:
ButtonPress.interaction.browser.spec.ts,ListRowPress.interaction.browser.spec.tsandListRowPressPaths.interaction.browser.spec.tsimporttestfrom@playwright/test, which the lint config forbids in browser specs, sopnpm lintfails until they import from@astryxdesign/a11y-spec/playwright. - At 4,421 authored lines from six separable increments, the pull request should carry only its top commit (rebase after #8326 and #9890 land) or become a declared stack in which every pull request targets
main. component:ButtonFR15 still lists "the 0.98 press scale also applies under reduced motion" as an implementation gap, though this change fixes it..changeset/press-release-exit-tokens.mdcredits no contributor, sopnpm check:changesetsfails.
The private duration parser and the unused test import noted earlier are fixed.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a coarse primary pointer, the press release of ClickableCard, SelectableCard and Thumbnail now fades on the theme's exit duration and easing, capped at the press's release time. |
There was a problem hiding this comment.
At this head the cards play the shared pressedAlpha release on every pointer, with no cap, so this sentence describes a release that no longer ships. press-release-exit-tokens.md already names the cards; this changeset can go.
There was a problem hiding this comment.
Done in 5f73b41: this changeset is deleted, and press-release-exit-tokens.md names the cards.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] With a fine primary pointer, a finger's press release on ClickableCard, SelectableCard and Thumbnail now fades on the theme's exit duration and easing. A mouse press is unchanged. |
There was a problem hiding this comment.
On main the cards already compose pressedAlpha alone, so there is no fine-pointer card release to fix in a released version; press-release-exit-tokens.md covers the change.
There was a problem hiding this comment.
Done in 5f73b41: the fine-pointer changeset is gone; no card changeset ships here.
| '@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. Under a finger, while a row is pressed and through its release, the press paints on the row's `::before` and the row is its own stacking context; at rest nothing of it applies, so a row's own `::before` decoration and its children's stacking are as they were. During a press the layer is the row's `::before`, and any `::before` property a caller sets, from `xstyle` (which `Item` composes after the layer) or from a stylesheet, wins over the layer's: a decoration with its own size and background jumps to the row's corner, and the row shows no pressed fill until the release ends. Decoration that should stay through a press belongs on `::after` or a child element. With a mouse or trackpad the row is unchanged: no layer and no new stacking context. |
There was a problem hiding this comment.
The row layer plays --duration-exit and --ease-exit (CELL_RELEASE, RELEASE_EASE in privateAttributes/press.stylex.ts), not the same clock and curve as before. Suggested: "on the theme's exit motion tokens".
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] A press's release now fades on the theme's exit motion tokens, `--duration-exit` and `--ease-exit`, instead of 200 ms, scaled by the theme's `tempo`, on a curve of its own. This covers every release that fades: a finger, mouse, pen or key press on a phone or tablet, and a pen press on a desktop, on buttons, cards, switches, checkboxes, list and tree rows, tabs, menu items and action groups. With the default tokens the release is 175 ms and holds its colour a little longer before it goes. A theme that changes `--duration-exit` or `--ease-exit` now changes the release too; a theme's `tempo` still does, through `--duration-exit`. Under Reduce Motion a finger's release still fades for no more than 200 ms. A mouse press on a desktop is unchanged. |
There was a problem hiding this comment.
This body credits no contributor, so pnpm check:changesets fails: add an @handle line.
| | 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). 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, and instant under reduced motion. 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 | | ||
| | 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 on release on the theme's exit motion tokens (`--duration-exit`, `--ease-exit`; `spec:AST-094` FR5). 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, and instant under reduced motion. 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.
The row still ends with "Implementation gap: the 0.98 press scale also applies under reduced motion", but Button.tsx now adds the reduced-motion scale(1) arms and this record drops the matching Current-state bullet. Delete the sentence.
| * (utils/pressFeedback.ts, as installed by hooks/usePressFeedback.ts) | ||
| */ | ||
|
|
||
| import {expect, test, type Locator, type Page} from '@playwright/test'; |
There was a problem hiding this comment.
eslint.config.js forbids @playwright/test in browser specs (no-restricted-imports); import test and expect from @astryxdesign/a11y-spec/playwright, as Switch.interaction.browser.spec.ts does.
| */ | ||
|
|
||
| import {readFile} from 'node:fs/promises'; | ||
| import { |
There was a problem hiding this comment.
Same lint error as the Button spec: import test and expect from @astryxdesign/a11y-spec/playwright.
| * ListRowPress.interaction.browser.spec.ts | ||
| */ | ||
|
|
||
| import {expect, test, type Locator, type Page} from '@playwright/test'; |
There was a problem hiding this comment.
Same lint error: import test and expect from @astryxdesign/a11y-spec/playwright.
|
/approve-spec 772101a |
github-actions
Bot
removed
the
needs:spec-owner-review
Every press release that fades now plays `--duration-exit` and `--ease-exit` instead of 200 ms, scaled by the theme's tempo, on a curve of its own: the shared overlay's touch release, the controller's release clock (which action groups read too), Switch's press layers and the list row layer. A desktop pen press, which the controller fades, moves to the tokens too. Under Reduce Motion a finger's release is no longer than 200 ms, as before. A mouse press on a desktop is unchanged.
772101a to
5f73b41
Compare
github-actions
Bot
added
the
needs:spec-owner-review
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
|
/approve-spec 5f73b41 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: the record changes carry an engineering owner's approval at this head, and one conflict between records needs fixing before this can be approved.
Required change:
- Miss:
design:reduced-motionDR2 and DEC-4 now say a control's hover or selection colour applies at once under reduced motion on any pointer, butcomponent:SwitchFR9 and FR15 (not changed here) say the switch's track colour cross-fades with its thumb under a finger, and Switch ships that fade; scope the DR2 and DEC-4 sentence to exclude a colour that changes with a cross-fading part, such as Switch's track, or amend Switch FR9 and FR15 and its colour fade.
CI has not run on this head yet (it is a draft), so the browser contracts still need to pass there. Non-blocking: a few comments still describe the old release clock (theme/themeMotion.ts:145, gestures/PressTracker.ts:155) or name the deleted utils/rowPressLayer.stylex.ts.
[Automated review]
| applies at once on any pointer, with no added element (DEC-3). A control's | ||
| hover or selection color, such as SelectableCard's selection border and | ||
| ring, also applies at once on any pointer, as on iOS (DEC-4). So does the |
There was a problem hiding this comment.
This makes every selection colour apply at once under reduced motion, but component:Switch FR9 and FR15 still cross-fade the switch's track colour with its thumb under a finger, and Switch ships that fade. Either scope it here and in DEC-4 (for example, "…except a colour that changes with a part that cross-fades, such as Switch's track with its thumb (component:Switch FR15)") or amend Switch FR9 and FR15.
Under reduced motion a press never scales a Button, and ClickableCard, SelectableCard and Thumbnail change their hover and selection colors at once on every pointer. Under a coarse primary pointer the three cards' press release fades on the theme's exit duration and easing, capped at the press controller's release clock (component:ClickableCard FR5, component:SelectableCard FR4, component:Thumbnail FR4). The press still shows at once. Under reduced motion that fade takes at most 200 ms, opacity only. A fine primary pointer keeps the timing it had for hover and a mouse press.
ClickableCard, SelectableCard and Thumbnail drop their copies of the press release and compose the shared pressedAlpha alone. Under the touch presentation it fades on the exit tokens, and the press controller's clock reads the same token, so the cap goes too. With a fine primary pointer, a finger's release on these cards now fades on the exit tokens, as other touch press releases do. A mouse press keeps the cards' desktop press styling.
5f73b41 to
29b854e
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 29b854e |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: the record changes carry an engineering owner's approval at this head, but the branch no longer merges into main.
Required change:
- Regression: #9890 has merged into
mainand this branch still carries its pre-merge commit, sodocs/specs/components/Button/Button.spec.md(FR15) conflicts; rebase ontomain, drop #9890's commit, and keepmain's FR15 without the implementation-gap sentence that #8326's code now meets.
Under Reduce Motion the Button and card changes behave as the approved records say: a pressed Button keeps its size, and the cards' colour changes are instant with a mouse and opacity-only on a touch screen. Non-blocking: Thumbnail.spec.md line 62 and FR4 call Thumbnail a card ("the card's desktop press styling"). Once #8326 lands, the rebase leaves only the top commit, as the description plans.
[Automated review]
| | 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). 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, and instant under reduced motion. 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 | | ||
| | 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 on release on the theme's exit motion tokens (`--duration-exit`, `--ease-exit`; `spec:AST-094` FR5). 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, and instant under reduced motion. | Current source and tests; `spec:AST-094` FR11 and DEC-21 for the mouse and pen press | |
There was a problem hiding this comment.
| or styling output changes | ||
| - Released default preserved: `no` for the touch press release: it fades on | ||
| the theme's exit duration and easing, as FR4 and `spec:AST-094` FR5. A | ||
| mouse press keeps the card's desktop press styling. |
There was a problem hiding this comment.
Non-blocking: Thumbnail is not a card. Suggested: "A mouse press keeps its desktop press styling", here and in FR4.
|
Folded into #8326. |
Stack: #9890 and #8326 → this pull request. Review only the top commit, "fix(core): the cards' touch press release is the shared one". The commit below it merges #8326 (c377f91) into #9890's rebased head (82dd709).
User impact
This affects people who press ClickableCard, SelectableCard or Thumbnail with a finger.
spec:AST-094FR5).--duration-exitand--ease-exit, like other touch press releases, also for a finger with a fine primary pointer. The press controller's clock reads the same token, so there is nothing left to cap.Problem and solution fit
interactionOverlayStyles.pressedAlphaalready releases on--duration-exitand--ease-exit, withmin(--duration-exit, 200ms)under Reduce Motion, andpressFadeMsreads--duration-exit. So the cards composepressedAlphaalone, as they do on main, and no new variant is needed.Expected behavior and authority
spec:AST-094FR5, as fix(core): a press's release fades on the theme's exit motion tokens #9890 records it: "Every press release that fades does so on the theme's exit motion tokens (--duration-exit,--ease-exit), on every device and pointer".component:ClickableCardFR5,component:SelectableCardFR4 andcomponent:ThumbnailFR4, in the spec owner's wording for this change: "A press shows at once. Under the touch presentation, its release fades on the theme's exit motion tokens (--duration-exit,--ease-exit), perspec:AST-094FR5; a mouse press keeps the card's desktop press styling." With fix(core): a press's release fades on the theme's exit motion tokens #9890 the press clock follows the tokens, so fix(core): no press scale or card color fade under reduced motion #8326's cap clause goes.pressedAlphaafter fix(core): a press's release fades on the theme's exit motion tokens #9890 merges, with no edits to the shared press module from two pull requests at once. This pull request is that follow-up.Smallest restoration
ClickableCard.tsx,SelectableCard.tsxandThumbnail.tsx: theSHARED_RELEASE*constants,styles.touchReleaseand its composition are removed, and the import is{interactionOverlayStyles}again. fix(core): no press scale or card color fade under reduced motion #8326's Reduce Motion arms on the overlay, the border and the selected root stay.utils/sharedPressRelease.test.ts(the sync test for the copies) is deleted;hasCardReleaseFadeleaves__tests__/pressState.ts, which is then fix(core): a press's release fades on the theme's exit motion tokens #9890's file;hasReleaseFadeagain, as on main;ReducedMotionPress.a11y.chromium.spec.ts, the fine-pointer case expects a finger's release on the exit tokens.spec:AST-094FR5, and the bullet says a mouse press keeps the card's desktop press styling. Governing rules addspec:AST-094/FR5.cards-touch-release-exit-tokens.mdis deleted. fix(core): a press's release fades on the theme's exit motion tokens #9890'spress-release-exit-tokens.mdalready names the cards, and on main the cards compose the shared release, so no card changeset ships here.Rebase
When #8326 and #9890 merge, this branch rebases onto main and keeps only the top commit.
Overlaps with open pull requests
__tests__/pressState.ts. Whichever merges second rebases.Writing rubric results
Run over the three changed records:
component:ClickableCard,component:SelectableCardandcomponent:Thumbnail.proposes,we decided,previously,a port ofandnowfinds nothing. The changed FR clauses, Compatibility bullets and readiness rows are present tense.#followed by digits,/pull/or/issues/).component:ClickableCardOQ1 is unanswered and keeps its two options. The other two records have none.Evidence
ReducedMotionPress.a11y.chromium.spec.tson a Storybook built at the merge commit): 1 of 6, the fine-pointer case, on a finger's release, which still plays the cards' copy of the old clock and curve.ReducedMotionPress.a11y.chromium.spec.ts6/6,CardPress.a11y.chromium.spec.ts9/9 and Button's accessibility contract 20/20, on a Storybook built at this head.hasReleaseFadeholds them to the shared release.Touch frames against #8326 and #9890
Local probe, not part of the diff. Real Chromium with phone emulation (402×714, touch, coarse primary pointer), one real tap on each card through touch input. When the press controller writes the release, the probe pauses the release animation and seeks it to each time below (the controller holds the release until that animation finishes, so nothing ends early). At each time it reads the surface's
--astryx-press-alpha, the overlay's computed paint, and a screenshot of the card. The comparison build is the merge of #8326's and #9890's heads. Measured at this pull request's earlier head 94b1c01, against that merge (05ccb64); this head rebuilds the same fold on #9890's amended head, whose change to the press code keeps the same release clock.--duration-exitis 125 ms, under the 200 ms bound.Scope
Testing
test:ui20,148/20,148;test:node16,492 passed, 2 expected failures, none failed.ReducedMotionPress.a11y.chromium.spec.ts6/6,CardPress.a11y.chromium.spec.ts9/9,Button.a11y.chromium.spec.ts20/20.CI=true pnpm linton the changed files, strict ESLint,check:knowledgeandreview:prepass.