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
needs:spec-owner-review
![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: no current spec governs the new editingControl="delete" value, and the diff also adds the spec:AST-094 record, which has no exact-head owner approval yet. An engineering owner can record approval of the proposed direction by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner; the decision should also say which record will state the delete control, including what keyboard and fine-pointer users get. Independent of that decision, these changes are still needed:
- Land #8369, #8375 and their chain first, then rebase so this pull request's diff against
mainis the delete-control commit alone. - The swipe release rule carried in from that chain no longer fires on a fling, which
spec:AST-057FR5 requires, so settle it in its own pull request. - Gate the delete disc on the same conditions Item uses to serve a swipe, so disabled and
interactiveRefrows do not show a control that does nothing. - Read the touch state from
ListContextinstead of subscribing to media queries in everyListItem.
[Automated review]
| const isTouch = useTouchPresentation().isTouch; | ||
| const controlKind = | ||
| ctx?.editingControl === 'delete' | ||
| ? isTouch && (restProps.swipeActions?.trailing?.length ?? 0) > 0 |
There was a problem hiding this comment.
This shows the delete disc on every touch row with trailing actions, but Item serves a swipe only when the row is not disabled, has no interactiveRef and has role listitem or none (Item.tsx:651-657). On a disabled or delegate row useItemSwipeReveal() is null, so the disc renders at full strength and its tap returns at line 468. Gating 'delete' on the same conditions (at least !isDisabled and no interactiveRef) and adding a disabled-row test would fix it.
| // a finger only, so with a mouse there is none (desktop unchanged), and a | ||
| // row with nothing to reveal keeps the indent alone. | ||
| const CheckboxControl = useIndicator('checkbox'); | ||
| const isTouch = useTouchPresentation().isTouch; |
There was a problem hiding this comment.
Every ListItem now holds its own (pointer: coarse) and compact-viewport subscriptions through useTouchPresentation(), in every list, editing or not. List already resolves the same fact once (List.tsx:299); publishing it in ListContext would avoid the per-row fan-out that react-component-runtime INV7 rules out.
| if (isConfirmed) { | ||
| return 'perform'; | ||
| } | ||
| if (behavior === 'commit' || wasConfirmed) { |
There was a problem hiding this comment.
With this rule a fling opens or closes the row and never fires the outermost entry, under commit too. spec:AST-057 FR5 on main still says "a drag past the commit point or a fling MUST fire the outermost entry". This comes from the #8375 chain, so it needs FR5 amended by an owner, or the rule conformed, in that pull request.
meta-cla
Bot
added
the
CLA Signed
c7cb859 to
ecb585b
Compare
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: no current spec governs the new editingControl="delete" value, and the diff also adds the spec:AST-094 record, which has no owner approval on this head and is being decided in #8378. An engineering owner can record approval of the proposed direction by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner; the decision should also name the record that will state the delete control, including what keyboard and fine-pointer users get. Independent of that decision, these changes are still needed:
- Land #8369, #8375 and their chain first, then rebase so this pull request's diff against
mainis the delete-control commit alone. - The swipe release rule carried in from that chain no longer fires on a fling, which
spec:AST-057FR5 requires, so settle it in that chain's pull request. - Gate the delete disc on the same conditions Item uses to serve a swipe, so disabled and
interactiveRefrows do not show a control that does nothing. - Read the touch state from
ListContextinstead of subscribing to media queries in everyListItem.
[Automated review]
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: no current spec governs the new editingControl="delete" value, so an engineering owner needs to provide or identify the record that states it, including what keyboard and fine-pointer users get and, because the disc is an element rows render only under touch, the exception spec:AST-017 FR60 requires with before-and-after element counts. An engineering owner can record approval of the proposed direction by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Independent of that decision, these changes are still needed:
- Land #8369, #8375 and their chain first, then rebase so this pull request's diff against
mainis the delete-control commit alone. - The swipe release rule carried in from that chain no longer fires on a fling, which
spec:AST-057FR5 requires, so settle it in that chain's pull request. - Gate the delete disc on the same conditions Item uses to serve a swipe, so disabled and
interactiveRefrows do not show a control that does nothing. - Read the touch state from
ListContextinstead of subscribing to media queries in everyListItem.
[Automated review]
| edge === 'bleed' && editingStyles.controlBleed, | ||
| ]; | ||
| const editingControl = !showsControl ? null : controlKind === 'delete' ? ( | ||
| <ListDeleteControl isEditing={isEditing} xstyle={controlStyles} /> |
There was a problem hiding this comment.
This span exists only under a coarse pointer, so each row with trailing actions has one more element under a finger than with a mouse. spec:AST-017 FR60 on main keeps list items at the strict budget ("Under touch, a component renders no more elements than under a fine pointer"; "Anything beyond this needs an owner exception with before and after counts"), so this needs that exception with the story's fine- and coarse-pointer element counts, or the disc drawn on an existing element.
…meta#8067, meta#8056 and meta#8055 fixes) into the presented start Four files both sides changed: the animator's attach flushes waiting presented calls before the element changes and leaves demotion to her later step after the landing's commit; dispose flushes them, then lands pending moves as her version does; the test's style stub takes her pseudo-element stylesheet and keeps this side's live-animation hold; the README and the browser spec's header take her wording, with this side's added proofs.
…by the merge The previous merge commit kept conflict markers in GestureHost.ts's imports. The host keeps this branch's type-only imports of the hover and pan recognizers (its guards duck-type them, so a page ships only the recognizers it constructs).
… its own module)
…eps flag goes
Under the Reduce Motion plan no segment is a step function: a waiting move
is a landing, and stop() lands it through cancelLanding('opacity'). The
step-segment landing block meta#8055 adds to stop() never runs here, nor does
the steps flag's sampling branch; both go.
…te waits) into the storyboards # Conflicts: # internal/interaction-harness/README.md
…egan-elsewhere; a recognizer's own scroll is not the browser taking the finger From CodeBunny on meta#8195, fixed once here where the host enters: - spec:AST-094 FR28: a touch with no recognizer or consumer on its path never reached the arena, so a recognizer tracking a finger was never told about a second finger beside it (a menu press could not end on one). GestureArena.untrackedBegan reports such a touch to recognizers that are tracking touches; it creates no record, claim or lock, and its moves and end stay the page's. - spec:AST-094 FR30: a recognizer that scrolls a scroller itself (a menu's edge autoscroll) was cancelled by its own write. The internal GestureHost.ownScroll(scroller) makes onScroll skip that scroller until two frames pass; a real pan in that window still reports itself through pointercancel or touchcancel. Three tests; two fail on the previous head.
…tion control change
…election hold change
…touch row heights change
…list updates change
…it mode change
…tion control change
… control change
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: no current spec governs the new editingControl="delete" value, so an engineering owner needs to provide or identify the record that states it, including what keyboard and fine-pointer users get and, because the disc is an element rows render only under touch, the spec:AST-017 FR60 exception with before-and-after element counts. An engineering owner can record approval of the proposed direction by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Independent of that decision, these changes are still needed:
- Land #8369, #8375 and their chain first, then rebase so this pull request's diff against
mainis the delete-control change alone. - The swipe release rule carried in from that chain no longer fires on a fling, which
spec:AST-057FR5 requires, so settle it in that chain's pull request. - That chain also imports
Item's swipe layer, with the gesture system and animator, statically wheremainloads it lazily; CI measuresItem's import at about 31 KB gzip larger, so keep it lazy or flag and justify the cost there. - Read the touch state from
ListContextinstead of subscribing to media queries in everyListItem, and add tests showing no delete disc on disabled andinteractiveRefrows.
[Automated review]
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsItem (@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/BreadcrumbItem | +31.1KB |
+480B |
| core/CheckboxList | +14.7KB |
+79B |
| core/CheckboxListItem | +32.3KB |
+904B |
| core/ContextMenu | +35.9KB |
+943B |
| core/ContextMenuItem | +30.9KB |
+448B |
| core/DropdownMenu | +30.9KB |
+934B |
| core/DropdownMenuCheckboxItem | +30.6KB |
+436B |
| core/DropdownMenuItem | +30.5KB |
+446B |
| core/DropdownMenuRadioItem | +30.6KB |
+436B |
| core/DropdownMenuSubMenu | +30.5KB |
+446B |
| core/Item | +30.5KB |
+440B |
| core/List | +15.0KB |
+184B |
| core/ListItem | +32.4KB |
+904B |
| core/Markdown | +36.3KB |
+897B |
| core/MoreMenu | +30.9KB |
+934B |
| core/Pagination | +30.8KB |
+369B |
| core/PowerSearch | +33.2KB |
+459B |
| core/RadioListItem | +30.9KB |
+411B |
| core/Selector | +31.6KB |
+418B |
| core/SelectorOption | +30.6KB |
+442B |
| core/Table | +36.0KB |
+923B |
| core/TableCell | +35.9KB |
+933B |
| core/TableHeaderCell | +35.9KB |
+942B |
| core/TreeList | +13.7KB |
+439B |
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.3KB | 35.1KB | +852B |
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:
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
|
Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed. |
|
Closing per Cindy's decision on #8687: edit experiences compose through |
Review only this PR's change: vjeux/astryx@3b4885d...9e116b7 (depends on #8369 and #8375; 3b4885d merges their heads).
User need
An iOS table in edit mode shows a red minus on each deletable row that opens its Delete action; a
Listcannot.Problem and solution fit
editingControl="delete", UIKit's.deletestyle: under a finger, each row whose trailing swipe actions can rest open shows a disc in the theme's error colour with a bar across it (21.6 pt), the disc the control itself, the bar its::before.swipeBehavior="commit"(nothing rests open), keeps the indent alone, as UIKit's.none.useTouchPresentation().isTouch(spec:AST-094FR13).Size (gzip, against its parent, by the size report's tool): ListItem JS +1,020 B and CSS +103 B, List +31 B; no module added.
Current specification
None governs it. Proposed public API, pending owner review (
component:List). Open owner question: a fine pointer has no swipe actions, so it gets no delete control (desktop unchanged,spec:AST-017FR59).Public delta and caller burden
<List isEditing editingControl="delete">; rows passswipeActions.trailing. No new exports.Evidence
Recorded on the iOS 26.2 simulator (Mobile Safari, its touch input); a real-device check is the release gate.
Scope
Testing
Written first and red: 7 unit tests (which rows show it, none with a mouse, semantics, the slide, opening, closing, Done), then a commit row shows none (unit, with the seam mocked and through the real row); the browser spec covers the geometry, real touch, Done, reduced motion and a mouse.
Green on #8076 62ecda1: root build and
pnpm typecheck, Storybook typecheck, eslint, build-css, CLI, CI duration guard, theuiproject's 16,120 tests (load timeouts pass alone), and the edit-mode stories with a mouse. Interaction proofs: Chromium 96 and WebKit 77, TreeList's Reduce Motion case included. Storycore-list--editing-delete.