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
PR Analysis Report
Modified ComponentsCheckboxInput (@astryxdesign/core)
Item (@astryxdesign/core)
TreeList (@astryxdesign/core)
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/CheckboxInput | +3.2KB |
+243B |
| core/CheckboxListItem | +3.9KB |
+763B |
| core/ContextMenu | +4.1KB |
+765B |
| core/ListItem | +3.9KB |
+763B |
| core/Markdown | +4.1KB |
+765B |
| core/MultiSelector | +3.4KB |
+236B |
| core/RadioListItem | +3.7KB |
+610B |
| core/Table | +4.1KB |
+757B |
| core/TableCell | +4.1KB |
+756B |
| core/TableHeaderCell | +4.1KB |
+769B |
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.3KB | +773B |
Accessibility Audit
Status: 2 accessibility violation(s) found — 2 serious.
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:
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 | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: the 44 px coarse-pointer targets for CheckboxInput and RadioList, and the List touch rows, press layer and holdSelection export carried in from the stacked pull requests, rest only on the draft spec:AST-094, which no owner has approved yet. An engineering owner records that decision by commenting /approve-spec c99f34a8cd361498a635ae8dc918e42f458296c4 on this pull request (the author may do so as an engineering owner), or by saying which of these behaviours should wait for AST-094 to be approved on main. Whatever that decision, please also:
- keep the 44 px row off the CheckboxInputs that other components embed, since on a touch screen MultiSelector options grow from 36 to 56 px, compact TableSelection rows from 33 to 53 px and Markdown task items from 32 to 52 px, or document and test that growth;
- rebase onto
mainonce #8045, #8052 and #8085 land, so this pull request's diff is only the touch-target commit (it is 15,494 authored lines today, andpr-a11yfails because the branch predates a spec file thatmain's workflow runs); - fix the references to files and records that do not exist:
utils/touchPresentation.ts,sources.test.ts,packages/core/src/gestures, and AST-094's citation of an AST-090 record.
[Automated review]
| {...(isDisabled ? undefined : pressable)} | ||
| {...stylex.props( | ||
| // Under a finger the row is at least as tall as an iOS control. | ||
| touchTargetStyles.row, |
There was a problem hiding this comment.
On a touch screen this floor applies to every CheckboxInput, including the ones MultiSelector, TableSelection and List task markers embed: their rows grow from 36 to 56 px, 33 to 53 px and 32 to 52 px. Could it apply only where this row is the target (not for isLabelHidden, inert or read-only marker uses), with those composers in the touch tests?
| }); | ||
|
|
||
| /** | ||
| * The touch presentation's query (utils/touchPresentation.ts TOUCH_QUERY), |
There was a problem hiding this comment.
utils/touchPresentation.ts and TOUCH_QUERY don't exist at this head (the same reference is in List/listRow.stylex.ts:36). Point at the real source of the query or drop the reference.
| 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.
There is no AST-090 record on main or in this pull request, so this amendment (and the one at line 235) has nothing to amend yet. Cite it once it exists, or drop the reference.
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
There was a problem hiding this comment.
Approval needs an engineering owner to settle the governing spec: the 44 px coarse-pointer targets for CheckboxInput and RadioList, and the List touch rows, press layer and holdSelection export carried in from the stacked pull requests, rest only on the draft spec:AST-094, and whether the targets ship on their own coarse-pointer query or wait for the shared touch resolver is still open. An engineering owner records that decision by commenting /approve-spec e877f6946b2b99a33f002fada63d412e4046bb28 on this pull request (the author may do so as an engineering owner), or by naming which of these must wait for AST-094 to be approved on main. Whatever that decision, please also:
- remove
reducedMotion: 'keep'fromutils/selectionHold.ts(lines 268 and 283), because under reduced motion the deselect still runs 350 ms with a finger and with a mouse, wheredesign:reduced-motionallows at most 200 ms on touch and an immediate change otherwise; - rebase onto
mainonce #8045, #8052 and #8085 land, so the diff is only the touch-target commits (15,545 authored lines today); - conform AST-094 FR8 to
design:reduced-motionor name that amendment, and fix the references toutils/touchPresentation.ts,sources.test.ts,packages/core/src/gesturesandspec:AST-090, which do not exist.
[Automated review]
| ? {} | ||
| : {velocity: {opacity: -timing.velocity}}), | ||
| }, | ||
| {at, reducedMotion: 'keep'}, |
There was a problem hiding this comment.
With reducedMotion: 'keep' (here and at line 283) the deselect plays its full 350 ms under prefers-reduced-motion, with a finger and with a mouse alike. design:reduced-motion asks for an opacity change of 200 ms or less on a coarse pointer and an immediate change otherwise, with no opt-out in core (DR2–DR4). Could the hold drop 'keep' and resolve the deselect that way, with a browser test under reduced motion?
| 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.
A 0.5 s navigation cross-fade is longer than the 200 ms design:reduced-motion DR2 allows on a touch screen, and the pending-review note under FR8 names only the draft design:motion DR5. Could FR8 follow DR2, or name the amendment to design:reduced-motion so an owner sees it?
| }); | ||
|
|
||
| /** | ||
| * The touch presentation's query (utils/touchPresentation.ts TOUCH_QUERY), |
There was a problem hiding this comment.
utils/touchPresentation.ts and TOUCH_QUERY don't exist on main or at this head. Point at the real source of the query, or drop the reference until the resolver lands.
e877f69 to
9282c28
Compare
There was a problem hiding this comment.
Approval needs an engineering owner to settle the governing spec: the 44 px coarse-pointer targets for CheckboxInput and RadioList rest only on the draft spec:AST-094, which no owner has approved for this head. An engineering owner records that decision by commenting /approve-spec 9282c287898d26192284c5f6a251282ce3ae1624 on this pull request (the author may do so as an engineering owner), or by saying the targets should wait until a 44 px touch target is approved on main. Whatever that decision, please also:
- remove the theme
platformadaptation condition from this stack, or land an approved amendment tospec:AST-012FR2 first, because FR2 closes thewhenvocabulary towidth,height,pointer,contrastandmotion; - rebase onto
mainonce the stacked pull requests land, so the diff is only the touch-target commits (33,535 authored lines today, 429 of them this pull request's); - align AST-094 FR8, FR4, DEC-1 and FR13 with
design:reduced-motionDR2 and with the code that ships, and fix the references tospec:AST-090andsources.test.ts, which do not exist.
[Automated review]
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
spec:AST-012 FR2 closes the when vocabulary: "A when object may contain width, height, pointer, contrast, and motion … Future environment fields require explicit additions to this closed vocabulary." No record admits platform, and the theme authoring guide and template already teach it. Could this axis come out of the stack, or wait for an approved FR2 amendment?
| measures (0.55 for a scroll view and a sheet), and a release MUST pick its | ||
| rest point by projecting its velocity the way its UIKit counterpart does: | ||
| with a deceleration rate, or with a measured projection time. | ||
| - **FR4 — Physics are not themable in touch presentations.** A touch |
There was a problem hiding this comment.
FR4 (and DEC-1) say touch physics are not themable, but this stack ships theme motion roles (defineTheme({motion: {springs, durations, curves}})) and holdSelection reads its deselect clock and curve from them. Could FR4 and DEC-1 describe what the roles allow, or the roles wait, so an owner approving this record approves what ships?
9282c28 to
10c2d49
Compare
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for the 44 px coarse-pointer target that CheckboxInput and RadioList get here: no record states that size, and the draft spec:AST-094 this branch carries names none and has no owner approval for this head. An engineering owner records that decision by commenting /approve-spec 10c2d49a4fa0c19a3e7fbcbae4505ee855728aa3 on this pull request (the author may do so as an engineering owner), or by saying the targets should wait until a record that states a 44 px coarse-pointer target is approved on main. Whatever that decision, please also:
- keep the new row press layer off fine pointers, since its unconditional
::beforehides a caller's::beforedecoration on interactive Item, ListItem and TreeList rows with a mouse too, whichspec:AST-017FR59 does not allow for touch work; - drop the
platformaxis from the theme template (packages/cli/assets/theme.template.ts), becausedefineThemenow rejects it andspec:AST-012FR2 does not admit it; - make the failing
pr-interactioncase pass: the theme-spring proof inanimator.interaction.browser.spec.tsfails because nothing registers the springs table, so a theme'ssprings.*roles reach no animation unless something registers that table; - rebase onto
mainonce the stacked pull requests land, or retarget onto #8085, so the diff is only the touch-target commits (35,149 authored lines today, 433 of them this pull request's); - align AST-094 FR8, FR4, DEC-1 and FR13 with
design:reduced-motionDR2 and with the code that ships, and fix the references tospec:AST-090and "AST-094 OQ4", which do not exist.
[Automated review]
| isolation: 'isolate', | ||
| '::before': { | ||
| content: '""', | ||
| position: 'absolute', | ||
| inset: 0, | ||
| zIndex: -1, | ||
| borderRadius: 'inherit', | ||
| pointerEvents: 'none', | ||
| backgroundColor: colorVars['--color-overlay-pressed'], | ||
| // On at once while the press is believed, and while a selection hold | ||
| // keeps the row selected (its own animations move it from there); | ||
| // nothing transitions it. | ||
| opacity: { | ||
| default: 0, |
There was a problem hiding this comment.
isolation and this ::before (with opacity: 0 by default) apply at every pointer, so on an interactive Item, ListItem or TreeList row a caller's ::before from xstyle is hidden at rest with a mouse too. spec:AST-017 FR59 lets touch work change fine-pointer appearance only to fix a defect mouse users hit or for a named cross-device consistency change. Could the layer's content and isolation sit under @media (pointer: coarse)?
| * are primary-pointer precision, contrast preference, motion preference, and | ||
| * platform (`ios` for the browsers of iPhone and iPad, `other` for the rest). |
There was a problem hiding this comment.
The stack removed the platform condition: themeAdaptations.ts accepts only the closed axes and throws … .platform is not supported., and spec:AST-012 FR2 closes the when vocabulary. A theme author who follows this template gets a build error; could the platform clause come out?
|
|
||
| export {createAnimator} from '../animator'; | ||
| export {solveSpring, toSpring} from '../spring'; | ||
| export {springs} from '../uikit/springs'; |
There was a problem hiding this comment.
This page never calls registerSpringsTable(), and reading a table no longer registers it, so in animator.interaction.browser.spec.ts:321 the themed box plays the table's spring and pr-interaction fails (111.7 px off its themed curve). No shipped module registers the springs table either. Could the table be registered here, or spring roles resolve without an explicit registration?
| @@ -177,6 +190,13 @@ const STATIC_EXPORTS = { | |||
| types: './dist/utils/interactionOverlay.stylex.d.ts', | |||
| default: './dist/utils/interactionOverlay.stylex.js', | |||
| }, | |||
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | |||
There was a problem hiding this comment.
AST-094 has only OQ1 and OQ2, so this new stable ./motion/reducedMotion.stylex export points at an open question that does not exist. Could it cite the record that governs it, or ship under an experimental subpath?
10c2d49 to
1f32db9
Compare
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for the 44 px coarse-pointer target that CheckboxInput and RadioList get here: no current record states that size, and the pull request itself marks it as proposed and pending owner review. An engineering owner records that decision by commenting /approve-spec 1f32db965954a61a6e29b435510203f160f0fba9 on this pull request (the author may do so as an engineering owner), or by saying the targets should wait until a record that states a coarse-pointer target size is approved on main. Whatever that decision, the diff also needs narrowing:
- rebase onto
mainonce the stacked pull requests land, or retarget onto #8085, so the diff carries only the touch-target commits (34,549 authored lines today, 433 of them this pull request's).
[Automated review]
|
Posted by Cindy's agent on her behalf. Cindy's decision on target size: 44 px, for touch only. Requirements:
vjeux, could you update the PR to match these? |
1f32db9 to
2a302ea
Compare
meta#8076's new head carries main 70ea859 with meta#8073's writer of the press attribute on main's press controller, and its README names both writers. utils/pressFeedback.ts keeps that controller with this line's own changes on it: the pointer, drag, scroll and visibility listeners through the shared input stream (meta#8356) and touchcancel only while the primary pointer is coarse (meta#8270); the key path stays at the window's bubble phase, so a Space press the page cancels paints nothing. usePressTracker stays in its own module, one tracker for a component's life (meta#8315), so meta#8073's rebuilt-tracker hook and its test are not taken. The harness files are meta#8076's.
…s, and the action-group tracker writes its own The header and writePress called it the one writer of the press attribute, but main's action-group tracker writes its groups' presses itself. They now say what holds, as meta#8076's gestures README does.
…ss release change # Conflicts: # packages/core/src/utils/pressFeedback.ts
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
… touch row target change
… release change # Conflicts: # packages/core/src/utils/pressFeedback.test.ts
…re foundation meta#8360) into the touch row target change
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
…wever late a busy first frame started it The touch press controller removed `fading` on a clock started at the lift. A busy main thread delays the release animation's first frame, and with it its end, so the clock cut the release short mid-fade. The controller now checks at that clock for the surface's own finite release animation (on it, or its `::before` or `::after`) and, while one runs or still waits for its first frame, ends the release when it finishes or is cancelled; with none, the release ends at the clock. Timers become cancel functions, so a release that waits on an animation is cancelled the same way. The clock is the table's, capped at 200 ms under reduced motion, and the overlay's release animation is capped the same way (design:reduced-motion DR2: a touch transition finishes in 200 ms or less); the stylesheet tests read the capped durations. This reaches every surface the touch press controller drives (buttons, switches, checkboxes, cards, menu rows, list rows). Split out of meta#8077 on review; meta#8077 builds its compositor row release on it. Red first: with the previous controller, three of the seven new cases fail (a release whose first frame came late, one still waiting when the clock ran out, another painter's release); the other four guard what stays (a cancelled release ends, no animation ends at the clock, other animations are ignored, a new press owns its paint).
…e row press release change # Conflicts: # packages/core/src/__tests__/pressState.ts # packages/core/src/utils/interactionOverlay.stylex.ts # packages/core/src/utils/pressFeedback.test.ts # packages/core/src/utils/pressFeedback.ts # scripts/build-css.test.mjs
…d the changeset says what a caller's ::before does From review on meta#8077: - The headers of motion/uikit/list.ts, both ListRowPress browser specs and interactionOverlay.cell.test.ts, and the notes in build-css.test.mjs and __tests__/pressState.ts, pointed the row press layer at utils/interactionOverlay.stylex.ts; it lives in utils/rowPressLayer.ts (rowPressLayerStyles.cell), its clocks in rowPressLayer.stylex.ts (cellPressConsts). - Notes that described the release as ending on a timer now say the controller checks at the release clock and the release ends with its animation; the stylesheet tests no longer parse a theme motion role the release stopped playing. - The changeset said a caller's ::before decoration takes the layer's paint during a press. Under a finger any ::before property a caller sets wins over the layer's, so 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; it now says so and keeps the advice to decorate with ::after or a child.
…ontroller change split out) into the touch row target change # Conflicts: # .changeset/list-row-press-compositor.md
…le uses From review on meta#8085: the exemption let a file that paints a press pass the guard whenever it rendered a marked component through `xstyle`. It came with ListItem's divider arms, which the revert of meta#8079 took out, so no file at this head reaches it, and in place it would wave through a file whose press styles never reach that component's root. The guard is again exactly as on the line below; it passes without the exemption.
…never needed From review on meta#8077: the build-css press-paint check filtered out a `--_press-paint: transparent` declaration, with a note, that the row layer never writes (only the overlay declares the press paint), and interactionOverlay.cell.test.ts defined a `hostRules` helper no case uses. Both go; the stylesheet check is as on the line below again.
…ers a row with an end control From review on meta#8085: - `isTouchClick` and its comment sat between `Item`'s docblock and `export function Item`, so the docblock attached to the helper and `Item` lost its description in editors and Storybook. The helper now sits above the docblock. - component:List FR10's verification names a row with an end control. A new List story puts a compact row whose end content is a switch between two interactive rows, and the browser spec checks that the interactive rows grow only to half the gap to the switch, that the browser hit-tests the switch at its center, and that a real tap toggles it with no tap reaching the rows beside it. With the hit area's neighbour clamp removed, the case fails (a band of 5.5 px where half the gap is 2 px).
…ast its divider From review on meta#8085, two ways a tap in an interactive row's band missed the row: - The band is the row's ::after, and the rows after it are positioned with no z-index, so a later row that takes no taps (a static row, such as the switch row in the "With end controls" story) painted and hit-tested over the band. The layer now has z-index: 1 on itself; its even-odd clip still leaves the row's own box to the row's content, and the row still makes no stacking context. - The layer is offset from the row's padding box, while the insets are measured from its border box, so below a divided row the band was the divider's width short. A host that draws a border now declares it (--_hit-border-{top,right,bottom,left}), and a divided ListItem declares its divider's, so the band starts at the divider's outer edge. Above everything at z-index auto, the band still takes no tap from a control near the list: the hit area's neighbour clamp measures every interactive element on the page, not only rows. The "With end controls" story gains a button in its header and one 4 px after the list, and a browser case hit-tests both; with the clamp removed it fails (a 6 px band where half the gap is 2 px). Red first: the browser spec now reads the band past the border box and hit-tests points inside Network's band. On the previous CSS, the end-control case read a 1 px band where half the gap is 2 px and a compact divided row's band read -1 px; with z-index removed, a point 0.5 px below Network hit the Wi-Fi row.
…nd the list bounds what rises From review on meta#8085 and meta#8106: - A ListItem with swipeActions is a stacking context at rest (Item isolates and translates its swipe root), so its touch band's z-index ordered only inside it and a following static row still took taps in the band. Under a finger, a swipeable interactive row now raises itself (z-index: 1 on the row; with a mouse nothing changes). The "With end controls" story's Network row is now swipeable, and the browser case checks that the row isolates and that taps in its band are its own. - The list's ul now isolates (isolation: isolate), so what rises inside it (a row's band, a swipeable row, and later a crossing row) orders above the rows after it but stays below a page-level sticky header at the same z-index (spec:AST-027 DEC-1, DEC-2). Red first: with Network swipeable, a point 0.5 px below it hit the Wi-Fi row; the unit cases for the list's isolation and the swipeable row's raise failed on the previous CSS.
From review on meta#8085: ListItem raised every interactive row with a swipeActions prop, while Item makes a row a stacking context (serves its swipe actions) only when a side has an entry, and not for a disabled row, a link root, a role that forbids interactive descendants, or a row whose press goes to a control inside it (interactiveRef). Rows with empty swipe sides or a delegate control were raised for nothing. Item's condition is now one function, servesSwipeActions, which Item and ListItem both use. Red first: in List.test.tsx, a row with empty swipe sides and a row with a delegate control both carried the raise on the change before this one.
ListItem declares --_hit-border-bottom (the divider's width, 0 on the last row) for the touch hit layer, and List.doc.mjs did not list it, so derivedVarRegistry.test.ts failed: "List has undocumented CSS vars in source: --_hit-border-bottom". It is now in List.doc.mjs theming.vars[] as a private var, and in VARS_WITHOUT_DERIVED_MAPPING with its reason: it is the ::after layer's geometry, which follows --border-width, not a property a theme author sets (as with --_thumbnail-hit-inset and --_input-clear-hit-inset). Red first: "List: all source vars are in doc file" failed on the change before this one; it passes now (122 of 122).
There was a problem hiding this comment.
Changes requested: the CheckboxInput and RadioList hit areas and the --size-touch-target token match spec:AST-094 FR42 and the target-size decision on this pull request, and they leave every layout and mouse interaction unchanged. This branch, though, still merges a snapshot of #8085 that makes up 30,787 of its 31,801 changed lines and that #8085 has since moved past. Please land #8085 first, then rebase so this pull request carries only its own 16 files; nothing else found blocks approval.
[Automated review]
Depends on #8085. Review only this PR's change: vjeux/astryx@ecc8779...49c8e40. Implements
spec:AST-094FR42 (decision) and List FR10 (#8571) for the maintainers' target size decision.User need
On a phone, a standalone CheckboxInput's target is its 24 px row, and RadioList rows are 24 px with 8 px dead strips; iOS controls are 44 × 44 pt, so fingers miss.
Why: targets are the drawn sizes, and the look must not grow (FR42).
Evidence
iOS 26.2 Mobile Safari: a tap 5 px above a CheckboxInput's 24 px box toggled it, and a tap 2 px above the second RadioList row, in its half of the 8 px gap, selected that row. Page-measured: rows stay 24 px; the hit areas reach 10 px past the outer edges and 4 px into each gap. At rest nothing moved:
Problem and solution fit
Under a coarse pointer each control keeps its size and takes #8085's shared invisible hit area: it grows toward the new
--size-touch-target(44 px, never lower) and stops at half the gap to each neighbour, so targets never overlap; a middle RadioList option reaches 32 px.Current specification
spec:AST-094FR42: no size increase for touch in the default look; 44 px through an invisible hit area clamped to half the gap.Public delta and caller burden
New theme token
--size-touch-target(44px). Coarse pointer only; nothing changes size, and a mouse sees no change.Touch-only JavaScript (INV6): CheckboxInput's import grows 2,943 B gzip and RadioListItem's 2,986 B, all in the shared hit-area modules (
utils/hitArea,utils/touchHitArea,utils/touchQueries) that List rows already import, so a page carries them once however many controls use them; ListItem's grows 356 B for the shared token read.analyze-pr.js).Design and alternatives
Scope
Testing
CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; the full core suite (16 workers) passes with no failure (688 files, 16,805 tests); TouchTargets, ListTouchHitArea and ListRowPress pass in Chromium (31, plus ListRowPress's WebKit-only video case skipped) and TouchTargets and ListTouchHitArea in WebKit (23); Checkbox, CheckboxList and RadioGroup a11y contracts 66/66; mouse clicks still activate. The node project's reds:story-tree.test.ts(main's A11y story titles, fixed in test(storybook): run the story-tree check where Storybook is built, and bring the tree back to its shape #8509),cache.test.mjs's two npm-lock cases (a Node started under another file name, fixed in fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520), andbuild-css.test.mjs's list-row hit-layer case, which fails on feat(List): under a finger, an interactive row's hit target grows toward 44 px where its neighbours leave room #8085's head alone: the band's edges now add the row's border width, and that test still expects the inset alone.--size-touch-target, the token this pull request admits: the shared hit area grows towardmax(44 px, token), read in px, rem or em, and again on a theme change and on a pointer change through one pointer listener shared by every watch (FR24), mergeable with main a73f030:CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; TouchTargets, ListTouchHitArea and ListRowPress in Chromium 27 passed (1 skip) and WebKit 20/20; Checkbox, CheckboxList and RadioGroup a11y contracts 66/66; units 16,168 underCI=true, with load timeouts that test(DropdownMenu): catch an eager sheet import when it is requested, not by a timer #8353, test(Resizable): load the package root while the file is collected, not inside a timed test #8471 and test(Selector, MultiSelector): wait for the on-demand sheet module before the first open #8642 fix; the CLI files that timed out in the full run pass alone (11 files, 396 tests); mouse clicks still activate. Red first: the token cases (60 px raises, 30 px leaves 44,3.75remand3emread 60, a re-read when the pointer turns coarse and when the theme changes) fail on feat(List): under a finger, an interactive row's hit target grows toward 44 px where its neighbours leave room #8085's plain 44 px; a guard keeps one pointer listener however many hosts.::afteris its hit layer under a finger, mergeable with main d9b43cf:CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; TouchTargets, ListTouchHitArea and ListRowPress in Chromium 26 passed (1 skip) and WebKit 19/19; Checkbox, CheckboxList and RadioGroup a11y contracts 66/66; units 16,167 underCI=true, with load timeouts that test(DropdownMenu): catch an eager sheet import when it is requested, not by a timer #8353 and test(Resizable): load the package root while the file is collected, not inside a timed test #8471 fix; the CLI files that timed out in the full run pass alone (11 files, 406 tests); mouse clicks still activate.CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; TouchTargets, ListTouchHitArea and ListRowPress in Chromium 25 passed (1 skip) and WebKit 18/18; Checkbox, CheckboxList and RadioGroup a11y contracts 66/66; units 16,133 underCI=true, with load timeouts named: the test(Item): wait for the swipe layer's module, not a one-second window #8352 family (Item's and List's lazy swipe layer), test(Selector, MultiSelector): wait for the on-demand sheet module before the first open #8642's sheet first loads, and a cold import of the root barrel; mouse clicks still activate.utils/hitArea.tsis main's), mergeable with main:CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; TouchTargets, ListTouchHitArea and ListRowPress in Chromium 25 passed (1 skip) and WebKit 18/18; Checkbox, CheckboxList and RadioGroup a11y contracts 66/66; units 16,123, with load timeouts that pass alone (test(Selector, MultiSelector): wait for the on-demand sheet module before the first open #8642's sheet, test(Item): wait for the swipe layer's module, not a one-second window #8352's swipe warm-up); mouse clicks still activate.test:ui; under load the MultiSelector bottom-sheet and ItemswipeActionsfirst-load waits timed out, fixed by test(Selector, MultiSelector): wait for the on-demand sheet module before the first open #8642 and test(Item): wait for the swipe layer's module, not a one-second window #8352, and both pass alone);pnpm buildthen rootpnpm typecheckclean;CI=true pnpm lint0 errors; changesets and token docs (283 tokens); node tests 355 (build-css, theme template, isolation, CLI docs); TouchTargets Chromium 9/9 and WebKit 9/9; ListTouchHitArea and ListRowPress pass; Checkbox, CheckboxList and RadioGroup a11y contracts 66/66; mouse clicks still activate.touchTarget.test.tsxcases fail on the merge without this change (the CheckboxInput hit layer and its tap, RadioList's unchanged geometry and its layer), and the CheckboxList FR10 cases fail on 057a771 (its floors).TouchTargets(Chromium and WebKit): each control's box equals a mouse page's (RadioList in both orientations, CheckboxList at every density); the point just outside a CheckboxInput box, a RadioList row's half of the gap and a compact CheckboxList row's edge belong to that control and a tap there picks it; the other half of the gap belongs to the first row; a 60 px theme token reaches further; a mouse gets no layer; composers (a Markdown task list, a compact table's selection column, a MultiSelector option) keep their pitch.