Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's spec decision. ContextMenu's new touch timeline (growth from 150 ms, commit at 400 ms, menu at 735 ms), the trigger growing while its menu is up, the touch scroll lock and the no-haptic choice for iOS Safari rest only on the draft ContextMenu record and the unapproved spec:AST-094. An engineering owner can record the decision by commenting /approve-spec <full head SHA> on this pull request (the author can do so as an engineering owner), or say which decision should change. Whatever that decision is, these changes are still needed:
- Land the stacked predecessors first, or retarget onto the last of them, so that this diff is the 1,840-line ContextMenu commit rather than 24,862 authored lines.
- Regenerate the
search closeCLI golden, which the new ContextMenu guidance pushes from 44 to 45 matches, so that the requiredtestcheck passes. - Re-attach the press recognizer when the trigger node changes, because a
triggerAsswitch currently leaves it on the detached element (useGestureRecognizer.ts:71-79,ContextMenu.tsx:882). - Move the
setIsTouchOpen(false)andrelease()calls from the Effect atContextMenu.tsx:744-753into the close handlers, and restore the two consumer-handler regression cases from #7167. - Add the
## Writing rubric resultsfor the new AST-094 record to the description.
[Automated review]
| { | ||
| guidance: true, | ||
| description: | ||
| 'Leave a little room around a block trigger: a held finger grows it from its center, by about 6%, until the menu closes.', |
There was a problem hiding this comment.
This guidance ends with "until the menu closes", and the CLI search scores guidance text. The required test check fails here: test-node (1/4) finds 45 matches for astryx search close where packages/cli/test/__golden__/no-icon-set/search-close.json expects 44. Please regenerate that golden, or reword the line.
| useEffect(() => { | ||
| if (wasOpenRef.current && !isOpen) { | ||
| setIsTouchOpen(false); | ||
| pressFeedbackRef.current.release(); | ||
| } | ||
| wasOpenRef.current = isOpen; | ||
| }, [isOpen]); |
There was a problem hiding this comment.
This Effect sets state (setIsTouchOpen(false)) and starts the spring-back in reaction to isOpen, and it synchronizes no external system (react-component-runtime INV2/INV3). The handlers that close the menu can do both calls directly: the layer's onHide and updateOpenState(false). Then the trigger springs back with the close itself, and the extra render goes away.
| useIsomorphicLayoutEffect(() => { | ||
| const element = ref.current; | ||
| if (element == null) { | ||
| return undefined; | ||
| } | ||
| return getGestureHost(element.ownerDocument).attach(element, recognizer, { | ||
| manageStyle, | ||
| }); | ||
| }, [ref, recognizer, manageStyle]); |
There was a problem hiding this comment.
The attach is keyed on the ref object, so it never re-runs when the node behind the ref changes. ContextMenu swaps its trigger between div and span with triggerAs, and after that swap the recognizer, its touch-action and its non-passive listener stay on the detached node, so a held finger opens nothing in iOS Safari. Attaching through a callback ref, or re-running the attach on node change, handles replacement (react-component-runtime INV6).
| expect(document.body.style.position).toBe(''); | ||
| }); | ||
|
|
||
| it('passes touch events through to the consumer’s own handlers', () => { |
There was a problem hiding this comment.
This replaces two #7167 regression cases only in part. Please add a held press with consumer touch handlers that still opens at 735 ms, and an early lift whose consumer onTouchEnd calls preventDefault() and opens nothing (component-test-sufficiency INV3).
7ef4051 to
bd95cd6
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the touch direction. The ContextMenu touch timeline, the growing trigger, the touch scroll lock and the no-haptic choice for iOS Safari rest only on the draft ContextMenu record and the unapproved spec:AST-094, so an owner needs to approve or reject them and the AST-094 requirements behind them. Any engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author can do so as an engineering owner. Whatever the decision, these fixes are still needed:
- Land or retarget the predecessor pull requests so this one carries only the ContextMenu change (24,866 authored lines today) and lint can finish.
- Re-attach the press recognizer when
triggerAschanges, and clear the touch-opened state in the close handlers instead of an Effect. - Restore the two #7167 touch-handler regression cases on the new timeline, and keep a caller's
touch-actionfromtriggerXstyle. - Remove the gestures README section for the missing
touchPresentationmodule, and fix AST-094's citation of the nonexistent AST-090.
[Automated review]
| return getGestureHost(element.ownerDocument).attach(element, recognizer, { | ||
| manageStyle, | ||
| }); | ||
| }, [ref, recognizer, manageStyle]); |
There was a problem hiding this comment.
This attachment is keyed on the ref object, which never changes, so when ContextMenu swaps the trigger element (triggerAs, ContextMenu.tsx:882) the new node never gets the recognizer, its touch-action or its touchmove claim, and the detached node keeps them (react-component-runtime INV6). Attach through a callback ref, or key the attachment on the element, and add a test that toggles triggerAs and long-presses the new node.
| pressFeedbackRef.current.release(); | ||
| } | ||
| wasOpenRef.current = isOpen; | ||
| }, [isOpen]); |
There was a problem hiding this comment.
This Effect sets isTouchOpen and releases the press feedback in reaction to isOpen, and its comment names no external system it synchronizes (react-component-runtime INV2/INV3). The close paths already run in handlers (updateOpenState, the layer onHide, closeMenu): clear the state and call release() there and drop the Effect.
| expect(document.body.style.position).toBe(''); | ||
| }); | ||
|
|
||
| it('passes touch events through to the consumer’s own handlers', () => { |
There was a problem hiding this comment.
This replaces the two #7167 cases ("composes consumer touch handlers with long-press detection" and "clears a pending long-press when a consumer prevents default on touch end") but only checks that the handlers run on a down/up. Please restore both on the new timeline: with a consumer onTouchStart the menu still opens at 735 ms, and with an onTouchEnd that prevents default a lift before 400 ms opens nothing (component-test-sufficiency INV3).
| element.style.getPropertyValue(property), | ||
| ); | ||
| } | ||
| element.style.setProperty(property, value); |
There was a problem hiding this comment.
Writing touch-action inline here overrides a caller who sets it through ContextMenu's triggerXstyle (an inline declaration beats the StyleX class), for example touch-action: none on a draggable row with a context menu; on base the trigger set no touch-action. Consider leaving an existing non-auto touch-action in place, and test it.
| ## Touch presentation | ||
|
|
||
| Whether a component presents itself the touch (UIKit) way is answered once, | ||
| in `../utils/touchPresentation.ts`: |
There was a problem hiding this comment.
../utils/touchPresentation.ts does not exist in this pull request or on main, and nothing else references useTouchPresentation or the queries listed below. Please drop this section, or ship it with the module.
| 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 spec:AST-090 on main or in this pull request, so owners cannot weigh a departure from its FR1 and FR10. Cite the record that holds those requirements, or drop the departure (line 235 repeats it).
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Table | 🔴 124.1 kB (+24.2 kB) · 17.3 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 🔴 115.5 kB (+24.2 kB) · 16.8 kB CSS (0) | — | — · — · — |
| TableCell | 🔴 115.9 kB (+24.2 kB) · 17.0 kB CSS (0) | — | — · — · — |
137 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| ContextMenu · Storybook | 🔴 114.7 kB (+24.1 kB) · 16.6 kB CSS (0) | 929 (+227) | — · ✓ · — |
| Markdown | 🔴 186.3 kB (+24.1 kB) · 19.5 kB CSS (0) | — | — · — · — |
| Tokenizer | 85.5 kB (+683 B) · 18.1 kB CSS (0) | — | — · — · — |
| DateTimeInput | 196.0 kB (+639 B) · 18.1 kB CSS (0) | — | — · — · — |
| DateInput | 173.5 kB (+608 B) · 16.4 kB CSS (0) | — | — · — · — |
| PowerSearch | 271.8 kB (+601 B) · 23.1 kB CSS (0) | — | — · — · — |
| ChatComposer | 68.9 kB (+600 B) · 11.2 kB CSS (0) | — | — · — · — |
| ChatComposerInput | 67.0 kB (+592 B) · 10.6 kB CSS (0) | — | — · — · — |
| ChatComposerTokenElement | 67.0 kB (+592 B) · 10.6 kB CSS (0) | — | — · — · — |
| Typeahead | 75.0 kB (+588 B) · 12.1 kB CSS (0) | — | — · — · — |
| BaseTypeahead | 62.6 kB (+580 B) · 10.3 kB CSS (0) | — | — · — · — |
| Lightbox | 103.2 kB (+177 B) · 9.8 kB CSS (0) | — | — · — · — |
| TimeInput | 149.9 kB (+158 B) · 15.5 kB CSS (0) | — | — · — · — |
| Slider | 102.0 kB (+145 B) · 8.7 kB CSS (0) | — | — · — · — |
| AlertDialog | 95.9 kB (+136 B) · 13.7 kB CSS (0) | — | — · — · — |
| Toast | 89.4 kB (+115 B) · 9.6 kB CSS (0) | — | — · — · — |
| Switch | 96.4 kB (+108 B) · 9.8 kB CSS (0) | — | — · — · — |
| SegmentedControl | 89.6 kB (+42 B) · 6.8 kB CSS (0) | — | — · — · — |
| ButtonGroup | 40.1 kB (+37 B) · 3.7 kB CSS (0) | — | — · — · — |
| Drawer | 44.8 kB (+19 B) · 8.2 kB CSS (0) | — | — · — · — |
| SideNav | 81.2 kB (+11 B) · 10.7 kB CSS (0) | — | — · — · — |
| ListItem | 65.0 kB (+9 B) · 10.9 kB CSS (0) | — | — · — · — |
| TopNav | 63.3 kB (+9 B) · 10.2 kB CSS (0) | — | — · — · — |
| TopNavHeading | 66.3 kB (+8 B) · 10.4 kB CSS (0) | — | — · — · — |
| AppShell | 81.5 kB (+7 B) · 12.8 kB CSS (0) | — | — · — · — |
| AvatarGroupOverflow | 44.5 kB (+7 B) · 5.5 kB CSS (0) | — | — · — · — |
| BreadcrumbItem | 76.0 kB (+7 B) · 11.5 kB CSS (0) | — | — · — · — |
| FileInput | 64.2 kB (+7 B) · 10.6 kB CSS (0) | — | — · — · — |
| TopNavMegaMenu | 61.1 kB (+7 B) · 11.2 kB CSS (0) | — | — · — · — |
| ComplexSelector | 65.9 kB (+6 B) · 11.3 kB CSS (0) | — | — · — · — |
| Link | 48.8 kB (+6 B) · 6.9 kB CSS (0) | — | — · — · — |
| ChatDictationButton | 52.3 kB (+5 B) · 9.4 kB CSS (0) | — | — · — · — |
| CheckboxInput | 59.9 kB (+5 B) · 9.1 kB CSS (0) | — | — · — · — |
| InfoTip | 43.3 kB (+5 B) · 5.3 kB CSS (0) | — | — · — · — |
| SideNavHeading | 66.8 kB (+5 B) · 10.6 kB CSS (0) | — | — · — · — |
| SideNavItem | 61.7 kB (+5 B) · 10.4 kB CSS (0) | — | — · — · — |
| DropdownMenuSubMenu | 64.7 kB (+4 B) · 9.4 kB CSS (0) | — | — · — · — |
| MobileNav | 61.6 kB (+4 B) · 9.9 kB CSS (0) | — | — · — · — |
| TabMenu | 63.2 kB (+4 B) · 10.4 kB CSS (0) | — | — · — · — |
| TextInput | 62.8 kB (+4 B) · 10.8 kB CSS (0) | — | — · — · — |
| Thumbnail | 56.1 kB (+4 B) · 10.9 kB CSS (0) | — | — · — · — |
| ChatLayout | 55.0 kB (+3 B) · 9.8 kB CSS (0) | — | — · — · — |
| ChatSendButton | 52.2 kB (+3 B) · 9.3 kB CSS (0) | — | — · — · — |
| CheckboxListItem | 66.0 kB (+3 B) · 10.9 kB CSS (0) | — | — · — · — |
| NumberInput | 66.9 kB (+3 B) · 11.1 kB CSS (0) | — | — · — · — |
| Step | 29.8 kB (+3 B) · 5.6 kB CSS (0) | — | — · — · — |
| Stepper | 53.4 kB (+3 B) · 9.5 kB CSS (0) | — | — · — · — |
| TableSelectionToolbar | 61.3 kB (+3 B) · 12.4 kB CSS (0) | — | — · — · — |
| Avatar | 39.5 kB (+2 B) · 5.5 kB CSS (0) | — | — · — · — |
| Banner | 53.7 kB (+2 B) · 9.8 kB CSS (0) | — | — · — · — |
| Calendar | 62.7 kB (+2 B) · 9.9 kB CSS (0) | — | — · — · — |
| MobileNavToggle | 52.1 kB (+2 B) · 9.3 kB CSS (0) | — | — · — · — |
| ToggleButton | 52.6 kB (+2 B) · 9.4 kB CSS (0) | — | — · — · — |
| TopNavMenu | 66.0 kB (+2 B) · 10.5 kB CSS (0) | — | — · — · — |
| ChatLayoutScrollButton | 52.2 kB (+1 B) · 9.5 kB CSS (0) | — | — · — · — |
| DrawerHeader | 54.7 kB (+1 B) · 12.2 kB CSS (0) | — | — · — · — |
| IconButton | 51.7 kB (+1 B) · 9.3 kB CSS (0) | — | — · — · — |
| MoreMenu | 86.3 kB (+1 B) · 16.7 kB CSS (0) | — | — · — · — |
| ProgressBar | 11.5 kB (+1 B) · 6.2 kB CSS (0) | — | — · — · — |
| RadioListItem | 62.7 kB (+1 B) · 9.8 kB CSS (0) | — | — · — · — |
| SelectorOption | 19.5 kB (+1 B) · 5.8 kB CSS (0) | — | — · — · — |
| Text | 7.6 kB (+1 B) · 5.3 kB CSS (0) | — | — · — · — |
| BottomSheet | 58.6 kB (0) · 7.5 kB CSS (0) | — | — · — · — |
| BottomSheetSwitcher | 17.5 kB (0) · 3.4 kB CSS (0) | — | — · — · — |
| Carousel | 59.6 kB (0) · 10.3 kB CSS (0) | — | — · — · — |
| ChatMessageList | 23.9 kB (0) · 6.9 kB CSS (0) | — | — · — · — |
| CheckboxIndicator | 2.7 kB (0) · 813 B CSS (0) | — | — · — · — |
| CheckIndicator | 8.0 kB (0) · 681 B CSS (0) | — | — · — · — |
| ClickableCard | 16.7 kB (0) · 7.1 kB CSS (0) | — | — · — · — |
| Collapsible | 24.8 kB (0) · 4.5 kB CSS (0) | — | — · — · — |
| CommandPalette | 59.5 kB (0) · 12.4 kB CSS (0) | — | — · — · — |
| CommandPaletteItem | 11.9 kB (0) · 3.6 kB CSS (0) | — | — · — · — |
| ContextMenuItem | 26.8 kB (0) · 6.1 kB CSS (0) | — | — · — · — |
| Dialog | 22.5 kB (0) · 6.7 kB CSS (0) | — | — · — · — |
| DropdownMenuCheckboxItem | 21.3 kB (0) · 6.4 kB CSS (0) | — | — · — · — |
| DropdownMenuItem | 20.5 kB (0) · 6.0 kB CSS (0) | — | — · — · — |
| DropdownMenuRadioItem | 21.3 kB (0) · 6.4 kB CSS (0) | — | — · — · — |
| FieldLabel | 47.1 kB (0) · 5.6 kB CSS (0) | — | — · — · — |
| Item | 16.1 kB (0) · 5.3 kB CSS (0) | — | — · — · — |
| LayerProvider | 4.1 kB (0) · 20 B CSS (0) | — | — · — · — |
| Outline | 23.7 kB (0) · 4.1 kB CSS (0) | — | — · — · — |
| Pagination | 103.3 kB (0) · 17.3 kB CSS (0) | — | — · — · — |
| RadioIndicator | 2.5 kB (0) · 725 B CSS (0) | — | — · — · — |
| SegmentedControlItem | 16.2 kB (0) · 4.3 kB CSS (0) | — | — · — · — |
| Spinner | 20.5 kB (0) · 6.1 kB CSS (0) | — | — · — · — |
| StatusDot | 27.6 kB (0) · 5.3 kB CSS (0) | — | — · — · — |
| Tab | 13.3 kB (0) · 4.2 kB CSS (0) | — | — · — · — |
| Timer | 9.7 kB (0) · 5.9 kB CSS (0) | — | — · — · — |
| Timestamp | 23.2 kB (0) · 9.6 kB CSS (0) | — | — · — · — |
| Token | 27.4 kB (0) · 4.7 kB CSS (0) | — | — · — · — |
| TopNavItem | 12.6 kB (0) · 4.0 kB CSS (0) | — | — · — · — |
| TopNavMegaMenuItem | 12.7 kB (0) · 4.2 kB CSS (0) | — | — · — · — |
| ChatToolCalls | 27.8 kB (−1 B) · 8.3 kB CSS (0) | — | — · — · — |
| CheckboxList | 54.3 kB (−1 B) · 7.9 kB CSS (0) | — | — · — · — |
| CodeBlock | 66.3 kB (−1 B) · 10.6 kB CSS (0) | — | — · — · — |
| CommandPaletteInput | 27.9 kB (−1 B) · 7.5 kB CSS (0) | — | — · — · — |
| DialogHeader | 54.5 kB (−1 B) · 10.8 kB CSS (0) | — | — · — · — |
| Heading | 8.2 kB (−1 B) · 5.3 kB CSS (0) | — | — · — · — |
| HoverCard | 24.4 kB (−1 B) · 4.3 kB CSS (0) | — | — · — · — |
| SideNavCollapseButton | 52.6 kB (−1 B) · 9.4 kB CSS (0) | — | — · — · — |
| Tooltip | 25.9 kB (−1 B) · 4.3 kB CSS (0) | — | — · — · — |
| TreeList | 41.1 kB (−1 B) · 5.4 kB CSS (0) | — | — · — · — |
| DropdownMenu | 86.1 kB (−2 B) · 16.7 kB CSS (0) | — | — · — · — |
| SelectableCard | 16.9 kB (−2 B) · 7.3 kB CSS (0) | — | — · — · — |
| Selector | 93.2 kB (−2 B) · 16.9 kB CSS (0) | — | — · — · — |
| Button | 51.7 kB (−3 B) · 9.3 kB CSS (0) | — | — · — · — |
| InputGroup | 52.1 kB (−4 B) · 7.8 kB CSS (0) | — | — · — · — |
| AvatarGroup | 42.2 kB (−5 B) · 5.6 kB CSS (0) | — | — · — · — |
| Field | 50.5 kB (−5 B) · 7.2 kB CSS (0) | — | — · — · — |
| TextArea | 57.9 kB (−5 B) · 9.2 kB CSS (0) | — | — · — · — |
| RadioList | 51.9 kB (−6 B) · 7.2 kB CSS (0) | — | — · — · — |
| Popover | 78.5 kB (−9 B) · 13.4 kB CSS (0) | — | — · — · — |
| DateRangeInput | 85.8 kB (−16 B) · 12.1 kB CSS (0) | — | — · — · — |
| MultiSelector | 88.7 kB (−17 B) · 16.3 kB CSS (0) | — | — · — · — |
| charts/Chart | 176.0 kB (new) · 494 B CSS (new) | — | — · — · — |
| charts/ChartLegend | 35.8 kB (new) · 224 B CSS (new) | — | — · — · — |
| charts/ChartTooltip | 153.4 kB (new) · 423 B CSS (new) | — | — · — · — |
| lab/ChatEmojiPicker | 96.5 kB (new) · 773 B CSS (new) | — | — · — · — |
| lab/ChatReactionBar | 97.7 kB (new) · 1.1 kB CSS (new) | — | — · — · — |
| lab/CommentComposer | 138.3 kB (new) · 1.9 kB CSS (new) | — | — · — · — |
| lab/CommentPin | 52.7 kB (new) · 1.2 kB CSS (new) | — | — · — · — |
| lab/Comments | 308.5 kB (new) · 2.5 kB CSS (new) | — | — · — · — |
| lab/CommentThread | 303.4 kB (new) · 2.4 kB CSS (new) | — | — · — · — |
| lab/DiffViewer | 78.1 kB (new) · 2.1 kB CSS (new) | — | — · — · — |
| lab/EmojiPicker | 81.3 kB (new) · 751 B CSS (new) | — | — · — · — |
| lab/ListInput | 95.4 kB (new) · 1.6 kB CSS (new) | — | — · — · — |
| lab/NavigationScreen | 78.4 kB (new) · 1.5 kB CSS (new) | — | — · — · — |
| lab/NavigationStack | 78.4 kB (new) · 1.5 kB CSS (new) | — | — · — · — |
| lab/PageControl | 71.7 kB (new) · 709 B CSS (new) | — | — · — · — |
| lab/Schedule | 99.4 kB (new) · 3.9 kB CSS (new) | — | — · — · — |
| lab/SelectableCardGroup | 59.7 kB (new) · 162 B CSS (new) | — | — · — · — |
| lab/TourStep | 84.7 kB (new) · 590 B CSS (new) | — | — · — · — |
| lab/TransferList | 90.1 kB (new) · 1.7 kB CSS (new) | — | — · — · — |
| lab/TransferListSelector | 97.1 kB (new) · 1.9 kB CSS (new) | — | — · — · — |
| richtext/RichTextEditor | 290.9 kB (new) · 2.6 kB CSS (new) | — | — · — · — |
| richtext/RichTextEditorToolbar | 244.1 kB (new) · 454 B CSS (new) | — | — · — · — |
| richtext/RichTextView | 271.1 kB (new) · 2.3 kB CSS (new) | — | — · — · — |
Package size: measured; main has no measurement of the merge base to compare yet.
API changes: cli +14 · ~5 · −0 (against the merge base's committed report); core +0 · ~1 · −0
Committed API report behind this build: cli 19 rows, core 25 rows; run pnpm api:report.
20 API changes (advisory)
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| + | cli |
./api · Types · type SearchResultEntry |
deprecated?: ComponentDeprecation | undefined |
import type {SearchResultEntry} from '@astryxdesign/cli/api'; const x: SearchResultEntry = {deprecated: …} |
| + | cli |
./authoring · Types · interface ComponentDeprecation |
interface ComponentDeprecation |
import type {ComponentDeprecation} from '@astryxdesign/cli/authoring' |
| + | cli |
./authoring · Types · interface ComponentIconSlotDoc |
interface ComponentIconSlotDoc |
import type {ComponentIconSlotDoc} from '@astryxdesign/cli/authoring' |
| + | cli |
./authoring · Types · interface ComponentIconSlotStateDoc |
interface ComponentIconSlotStateDoc |
import type {ComponentIconSlotStateDoc} from '@astryxdesign/cli/authoring' |
| + | cli |
./authoring · Types · interface MultiComponentDoc |
deprecated?: ComponentDeprecation |
import type {MultiComponentDoc} from '@astryxdesign/cli/authoring'; const x: MultiComponentDoc = {deprecated: …} |
| + | cli |
./authoring · Types · interface SingleComponentDoc |
deprecated?: ComponentDeprecation |
import type {SingleComponentDoc} from '@astryxdesign/cli/authoring'; const x: SingleComponentDoc = {deprecated: …} |
| + | cli |
./doc · Types · interface ComponentBaseDoc |
deprecated?: ComponentDeprecation |
import type {ComponentBaseDoc} from '@astryxdesign/cli/doc'; const x: ComponentBaseDoc = {deprecated: …} |
| + | cli |
./doc · Types · interface SubComponentDoc |
deprecated?: ComponentDeprecation |
import type {SubComponentDoc} from '@astryxdesign/cli/doc'; const x: SubComponentDoc = {deprecated: …} |
| + | cli |
./doc · Also exported from an earlier entry point · ComponentDeprecation |
ComponentDeprecation (./authoring) |
import {ComponentDeprecation} from '@astryxdesign/cli/doc' |
| + | cli |
./doc · Also exported from an earlier entry point · ComponentIconSlotDoc |
ComponentIconSlotDoc (./authoring) |
import {ComponentIconSlotDoc} from '@astryxdesign/cli/doc' |
| + | cli |
./doc · Also exported from an earlier entry point · ComponentIconSlotStateDoc |
ComponentIconSlotStateDoc (./authoring) |
import {ComponentIconSlotStateDoc} from '@astryxdesign/cli/doc' |
| + | cli |
./template · Also exported from an earlier entry point · ComponentDeprecation |
ComponentDeprecation (./authoring) |
import {ComponentDeprecation} from '@astryxdesign/cli/template' |
| + | cli |
./template · Also exported from an earlier entry point · ComponentIconSlotDoc |
ComponentIconSlotDoc (./authoring) |
import {ComponentIconSlotDoc} from '@astryxdesign/cli/template' |
| + | cli |
./template · Also exported from an earlier entry point · ComponentIconSlotStateDoc |
ComponentIconSlotStateDoc (./authoring) |
import {ComponentIconSlotStateDoc} from '@astryxdesign/cli/template' |
| ~ | cli |
./api · Functions · function search |
function search: (query: string, options?: { cwd?: string | undefined; type?: "template" | "component" | "hook" | "theme" | "doc" | undefined; limit?: number | undefined }) => Promise<SearchResponse> → function search: (query: string, options?: { cwd?: string | undefined; type?: "template" | "component" | "doc" | "theme" | "hook" | undefined; limit?: number | undefined }) => Promise<SearchResponse> |
import {search} from '@astryxdesign/cli/api'; search(…) |
| ~ | cli |
./authoring · Types · interface MultiComponentDoc |
theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } |
import type {MultiComponentDoc} from '@astryxdesign/cli/authoring'; const x: MultiComponentDoc = {theming: …} |
| ~ | cli |
./authoring · Types · interface SingleComponentDoc |
theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } |
import type {SingleComponentDoc} from '@astryxdesign/cli/authoring'; const x: SingleComponentDoc = {theming: …} |
| ~ | cli |
./doc · Types · interface ComponentBaseDoc |
theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } |
import type {ComponentBaseDoc} from '@astryxdesign/cli/doc'; const x: ComponentBaseDoc = {theming: …} |
| ~ | cli |
./doc · Types · interface SubComponentDoc |
theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[] } → theming?: { container?: boolean; targets: ComponentThemingTarget[]; vars?: ComponentThemingVar[]; derived?: ComponentThemingDerivedVar[]; componentIcons?: ComponentIconSlotDoc[] } |
import type {SubComponentDoc} from '@astryxdesign/cli/doc'; const x: SubComponentDoc = {theming: …} |
| ~ | core |
. · Components · component ContextMenu |
prop presentation?: AdaptivePresentation = 'popover' → prop presentation?: AdaptivePresentation |
<ContextMenu presentation={…} /> |
Wide change (shared styles: packages/core/src/theme/derivedVarRegistry.test.ts, packages/core/src/theme/derivedVarRegistry.ts, packages/core/src/theme/generateThemeRules.test.ts, and 1 more): add ci:run-visual or ci:run-rtl to run them.
CI impact: +15.4 job-min per update · pool +13% · standard runners · 5 CI action or script files changed (not measured) ⚠️
| Before | After | Change | |
|---|---|---|---|
| Job-minutes per PR update | 116.8 | 132.2 | +15.4 |
| Runner pool, runner-minutes a day | 53,692 | 60,766 | +7,074 (+13%) |
| Average concurrent jobs | 37.3 | 42.2 | +4.9 |
| Time to green (median → this run) | — | — | — |
| Runs a day | 460 | 460 | 0 |
Changed jobs: pr-interaction changed: standard · pr-interaction-webkit changed: standard
Not measured (the jobs that use these keep their recent minutes above): .github/scripts/ci-interaction-webkit.test.mjs, .github/scripts/ci-interaction.test.mjs, .github/scripts/playwright-legs.mjs, .github/scripts/playwright-legs.test.mjs, .github/scripts/release-publish-workflow.test.mjs.
Flags: runner-pool usage +13%.
Assumptions: before is each job's average over the last 20 pull-request runs; after keeps that for unchanged jobs and uses this run's minutes for added or changed ones, weighted by how often each changed job ran in those runs; runs a day averages the seven whole days before today; time to green compares this run with the median of those runs that ran the same jobs.
Current run: CI concluded cancelled; parts of this report may be missing.
Generated by PR Enrichment workflow | View full report
bd95cd6 to
d01580f
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the ContextMenu touch direction, which rests only on the draft ContextMenu record and the unapproved spec:AST-094: the iOS timeline, the growing trigger, the touch scroll lock and no haptic in iOS Safari, the changes to the 500 ms press on other touch screens, and the AST-094 requirements behind them. Any engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author can do so as an engineering owner. Whatever the decision, these fixes are still needed:
- Land or retarget the predecessor pull requests so this one carries only the ContextMenu change (26,052 authored lines today).
- Keep the released
(max-width: 768px) and (pointer: coarse)query forpresentation="adaptive", which the Selector record's DEC-1 says it keeps, or move the new compact query to its own pull request with a DEC-1 amendment. - Re-attach the press recognizer when
triggerAschanges, and clear the touch-opened state in the close handlers instead of an Effect. - Let a caller's
triggerXstyletouch-action win, and restore the two #7167 regression cases on the new timeline. - Fix AST-094's citation of the nonexistent AST-090, and add the
## Writing rubric resultssection the new record needs.
[Automated review]
| // and large touch displays stay anchored, phones in either orientation get | ||
| // the sheet. The query is the shared touch presentation's | ||
| // (utils/touchPresentation.ts). | ||
| export const COMPACT_TOUCH_PRESENTATION_QUERY = COMPACT_TOUCH_QUERY; |
There was a problem hiding this comment.
This replaces the released (max-width: 768px) and (pointer: coarse) query for every presentation="adaptive" caller: a coarse-pointer viewport 744 to 768 px wide (an iPad mini in portrait) loses its bottom sheet, and a phone in landscape (wider than 768 px, under 500 px tall) gets one instead of the popover. The Selector record's DEC-1 says "adaptive keeps its width-and-pointer query", so please keep the released query here and propose the compact query in its own pull request with a DEC-1 amendment.
| useIsomorphicLayoutEffect(() => { | ||
| const element = ref.current; | ||
| if (element == null) { | ||
| return undefined; | ||
| } | ||
| return getGestureHost(element.ownerDocument).attach(element, recognizer, { | ||
| manageStyle, | ||
| }); | ||
| }, [ref, recognizer, manageStyle]); |
There was a problem hiding this comment.
The attachment is keyed on the ref object, which never changes, so when ContextMenu swaps its trigger element (triggerAs, ContextMenu.tsx:897-902) the new node never gets the recognizer, its touch-action or its touchmove claim, while the detached node keeps them; a hold on the new trigger opens nothing. Attaching through a callback ref (or keying the attachment on the element) handles replacement, and a test that toggles triggerAs and then holds would cover it.
| const wasOpenRef = useRef(isOpen); | ||
| useEffect(() => { | ||
| if (wasOpenRef.current && !isOpen) { | ||
| setIsTouchOpen(false); | ||
| pressFeedbackRef.current.release(); | ||
| } | ||
| wasOpenRef.current = isOpen; | ||
| }, [isOpen]); |
There was a problem hiding this comment.
This Effect sets isTouchOpen and starts the spring-back in reaction to isOpen, and it synchronizes no external system (react-component-runtime INV2 and INV3). The layer's onHide and updateOpenState already close the menu; clearing isTouchOpen and calling release() there keeps the work in the action that caused it and lets this Effect go.
| element.style.getPropertyValue(property), | ||
| ); | ||
| } | ||
| element.style.setProperty(property, value); |
There was a problem hiding this comment.
Writing touch-action inline here outranks a touch-action a caller sets through ContextMenu's triggerXstyle (an inline declaration beats the StyleX class), so a draggable card that sets touchAction: 'none' would start panning the page. Please let a caller's value win and add a test for it.
| it('passes touch events through to the consumer’s own handlers', () => { | ||
| const onTouchStart = vi.fn(); | ||
| const onTouchEnd = vi.fn(); | ||
| const trigger = renderMenu({onTouchStart, onTouchEnd}); | ||
| const press = finger(trigger); | ||
| press.down(); | ||
| press.up(); | ||
| expect(onTouchStart).toHaveBeenCalledTimes(1); | ||
| expect(onTouchEnd).toHaveBeenCalledTimes(1); | ||
| }); |
There was a problem hiding this comment.
This replaces the two #7167 cases ("composes consumer touch handlers with long-press detection" and "clears a pending long-press when a consumer prevents default on touch end") only in part: it checks that the handlers run, not that a hold still opens the menu beside a consumer onTouchStart, or that a consumer preventDefault on an early touchend leaves no menu. Both behaviors hold on this head, so please keep them as assertions (component-test-sufficiency INV3).
| - **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 spec:AST-090 on main or in this pull request, so owners cannot weigh a departure from its FR1 and FR10. Please cite the record that holds those requirements, or drop the departure (line 235 lists it as well).
|
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.
|
d01580f to
7825a41
Compare
|
Converged in 7825a41:
|
|
Addressed in 7825a41:
|
…eta#10263) test(cli): two agent-docs tests remove their links with unlinkSync On Node 24.13, fs.rmSync on a symbolic link to a directory throws ERR_FS_EISDIR ("Path is a directory"), even with force. Node 24.19 and later remove the link. Two tests removed such a link that way, so they failed on every run under Node 24.13, which the CLI's engines field allows. They now call fs.unlinkSync, which removes a link on every version, as the code under test does.
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #8133.
* fix(Carousel): Shift and a wheel notch scroll the row once React's wheel listener is passive, so Carousel's preventDefault() did not stop the browser's own horizontal scroll on Shift+wheel, and in Firefox each notch scrolled the row twice. A native listener stops it. It is attached only while the row overflows. * Merge main into the Carousel wheel-listener branch (meta#10248 landed; its suppression goes with the React handler) * fix(Carousel): the wheel listener reads no style or layout The listener now reads the row's direction only inside the paging's move of a held row, and it relies on the overflow state that already gates it, so a wheel over the row reads no style or layout. The changeset that announced the lint suppression this change removes is deleted. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…lease, for 0.8.0 (meta#10253) * docs(architecture): stage CLN-0029, the CLI's Core peer at its own release, for 0.8.0 * docs(architecture): format the verification table
…meta#10170) * fix(Selector): a searchable trigger's name carries the selected value With hasSearch the editable closed trigger is a button, and a button reads no value, so a screen reader heard only the label. The trigger is now named by its label followed by the rendered value text (the selection, or the placeholder), as a select-only button is. A standalone field label gets an id for that reference. The combobox shape is unchanged. Selector AR1 records it. * fix(Selector): a caller aria-label stays the first part of the searchable trigger's name; AR1 states the empty and aria-label cases Per review: with a caller aria-label, the trigger refers to itself first ("Pick a team Banana"), as the combobox shape keeps that name. AR1 now says the placeholder is the value text when nothing is selected, and a caller aria-label takes the label's place. The Chromium listbox-name spec asserts the closed searchable trigger's name in the accessibility tree, and the AR1 row names it. Plainer source comment. * fix(Selector): a caller aria-labelledby still names the searchable trigger ahead of a caller aria-label Per review: with both aria-label and aria-labelledby, the trigger keeps the field label and the caller's ids, then the value ("Team Lunch Banana"), as the combobox shape keeps aria-labelledby (input-fields FR10). AR1 says so. * Merge branch 'main' into l1/selector-search-value * test(Selector): the search-keyboard spec finds the searchable trigger by the start of its name The searchable trigger's name is now its label followed by its value text, so the browser spec matches the label at the start of the name instead of the exact name. * Merge branch 'main' into l1/selector-search-value
* [Component Audit] Kbd: key badges grow with larger text * Kbd audit spec: measure contrast on the composited badge surface * Kbd audit: self-review follow-ups * Kbd: keep only the reason in the badge height comment
…meta#10269) test(cli): remove packed-CLI fixtures once per run, not in file hooks Two node suites removed thousands of files in a 30 s per-file hook, and under a full node run that removal can outlast the hook. The packed-old-core afterAll removed a 3,123-file copy of the CLI. The global-local afterAll removed the unpacked CLI and every pooled copy, about 12k entries. The packed-old-core fixture now holds the files `npm pack` puts in the CLI's tarball, not five whole folders. That drops about 460 test files npm never ships, and adds `host`, which it does ship. Three published asset folders that theme build never reads (templates, docs and icons, 1,896 files) are one link each to the package's own folder, or a copy where links are refused. So the fixture writes about 680 files. The node project's global setup now gives each run a folder. The unpacked CLI, its pooled copies and the packed-old-core fixture are made in it, and no per-file hook removes them. The global teardown moves the folder aside, which takes no time, and a detached child removes it, so close() never waits on a delete either. The next run's setup sweeps any trash that a stopped child left. When a test writes into a file that every placed copy shares, the fixtures now unpack only the changed paths again, not all 2,581 files. That repair runs inside the test, which a busy machine could make outlast its 30 s. The fixtures suite also unpacks the CLI in its own setup, as global-local does, not inside its first test. Tests cover the pack-list fixture, a teardown that never deletes through a link, the copy fallback, the run folder, the background removal and the sweep. The shared-file test covers the repair.
… and search focusable (meta#10192) * test(TransferListSelector): a disabled selector keeps its list controls focusable and blocks changes (red) * fix(TransferListSelector): a disabled selector keeps its list controls focusable The list fieldset no longer takes native disabled, which blurred a focused control when the selector became disabled while open. It carries aria-disabled, every row's transfer and reorder controls take their own focusable disabled state, and changes are ignored (spec:AST-085 FR1-FR4). * fix(TransferList): one isDisabled state disables every control focusably, including search TransferList takes isDisabled: every option is treated as fixed without its own constraint message, and the search field is disabled but focusable. TransferListSelector passes its disabled state down instead of rewriting options. * fix(TransferList): a list disabled mid-reorder ends the grab without any change finishReorder clears the session without committing while the list is disabled, so neither a cancel restore nor a pointer drop reaches onChange, and the grip shows no pressed state while disabled. * Merge main into lane4/transfer-selector-focusable-disabled
…ta#10264) test(core): six date and time cases pin each CLDR version's words CLDR 47 (ICU 77, as in Node 24.13) and CLDR 48 (ICU 78, as in Node 24.22) write some day-period, weekday and era names differently. Six cases hard-coded CLDR 48's text, so they failed on every run under ICU 77: - timeFormat: the Assamese afternoon, "PM" in CLDR 47 and "অপৰাহ্ন" in CLDR 48. - calendarDateParser: the Vietnamese short Tuesday, "Th 3" and "Thứ 3"; the English Amete Alem era, "ERA0" and "AA". - calendarDateParser.roundtrip: the Amharic narrow Minguo eras, "R.O.C." and "ROC", "Before R.O.C." and "BROC". - DateInputTouch.calendar: the English Ethiopic era in two pane labels, "ERA1" and "AM". Each case now expects the literal for the runtime's CLDR version (process.versions.cldr). Any other version expects CLDR 48's, so new locale data fails loudly. The cases that read text also check that the formatter writes the same literal.
…s close (meta#8186) * chore(core): the layer runtime's exit copy, for a touch presentation's close placeLayerExitCopy puts a static, inert copy of a closing layer where the layer was drawn, in the top layer, for a touch presentation's close to play on: the browser hides a popover the moment it closes. The copy keeps the layer's box, transform, presented look, overflow and scroll positions; has no ids, anchor names, form names or form owner; is inert and hidden from assistive technology; and its descendants' keyframe animations and transitions do not start again. A refused show during the closing toggle waits, hidden, until the toggle is over. A layer inside other layers gets its copy after the outermost one, with what it inherited there (its color scheme included), so a close that takes the layers around it too still draws the copy. That copy is shown again above the copy the same close then places for a layer around it. A layer has one copy. The layer-runtime record states it, and that the presentation plays its exit on the copy (AST-094 FR7). Core's shared inertCopy now takes every copied native control and form-associated custom element out of any form with form="", as its documentation says, so the swap's copies join no form either. It also sets anchor-name: none on each copied element that has an anchor name, so no live anchor gets a second element. * Merge branch 'main' into uikit/c26-layer-exit-copy * Merge branch 'main' into uikit/c26-layer-exit-copy * fix(core): the exit copy keeps no closed layer The exit copy found the inner layers to raise through a module-level set of every copied layer. A layer React removed on close stayed reachable there until a later close showed a copy. The inner layers are inside the closing layer, so they are now found there, and nothing keeps a layer once it is gone. Chromium 150 and later refuse a show during another popover's toggle, so the copy can open in the next frame. The browser spec's light-dismiss case now reads the copy after a frame, as its nested-close cases do. The cap check's comment now names only the copy cap. * docs(core): the exit copy's JSDoc states the empty-canvas rule once The empty-canvas rule stays in the first paragraph, and the inert-copy paragraph no longer repeats it or `inertCopy`'s own contract. * Merge main into the layer exit copy branch (layer-runtime.md verification table keeps both rows) * Merge main into the layer exit copy branch (meta#8147 landed) * Merge main into the layer exit copy branch (placeOver's beforeNode beside keepInherited) * Merge main into the layer exit copy branch (meta#10224 landed) * fix(core): the exit copy reads its host's style before the insertion keepInherited read the outer layer's parent's computed style right after the copy was inserted there: a style read after a write. It reads only that host and writes only on the still-detached copy, so it now runs before the insertion. What the copy inherits is unchanged. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…ta#10289) docs(AST-059): anchored layers re-pick once after scrolling stops
…ed (meta#10274) perf(cli): list each folder once per read, and reuse it while unchanged Every folder a search, build or component read walks now lists through the listing cache: the docs, template, theme, icon and package walkers read it once per operation, where some read a folder twice or outside the cache. A later operation in the same process reuses a folder's listing while its stamp (size, times, inode) is the one it was kept with. A listing is kept only after its stamp has held for two seconds of this process's clock, so a change within one tick of a coarse file system clock still gets a new read. A reuse check records the folder before its stamp is taken, so a change between the two shows.
…root (meta#7931) * docs(spec): ref-first DOM styling target with explicit exemptions * Merge main into spec/ref-first-dom-style-props * docs(spec): AST-089 lists BottomSheetSwitcher as an exception, drops the bound TopNavMenu drawer-header ref, and words Icon's split BottomSheetSwitcher applies its dialog surface to every step's sheet dialog, so it joins the target-preservation exemptions. TopNavMenu and TopNavMegaMenu drawer headers bind their ref, so L10 keeps only SideNav. Icon styles its span and binds its ref to the svg. * docs(spec): AST-089 covers the Popover and PowerSearch sheet presentations, names the claims promotion amends, and collapses OQ1 E6 and E7 add the sheet paths of component:Popover DEC-8 and component:PowerSearch DEC-3. FR6 puts input-fields FR10 components outside the default. Current-state impact names public-component-api INV15 and AST-071 FR1. L7 says VisuallyHidden ignores xstyle; L8 says Icon's ref reaches the svg and its wrapper is unpromised. OQ1's choice moves into DEC-1. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…ut (meta#10281) * chore(Carousel): the wheel listener's comment names a held row's reads While a flick holds the row, the listener's move reads the row's stops as well as its direction. The comment now says so. * chore(Carousel): the wheel listener's comment says when it reads layout While a flick holds the row, the listener's move reads the row's stops, its direction and the theme's motion tempo. The comment now states only that the listener reads no style or layout unless a flick holds the row.
…nested providers correctly (meta#10283) * [Component Audit] LayerProvider: its docs reach editors and describe nested providers correctly * LayerProvider audit: self-review follow-ups
…s back into it (meta#8187) * chore(core): the layer runtime's exit copy, for a touch presentation's close placeLayerExitCopy puts a static, inert copy of a closing layer where the layer was drawn, in the top layer, for a touch presentation's close to play on: the browser hides a popover the moment it closes. The copy keeps the layer's box, transform, presented look, overflow and scroll positions; has no ids, anchor names, form names or form owner; is inert and hidden from assistive technology; and its descendants' keyframe animations and transitions do not start again. A refused show during the closing toggle waits, hidden, until the toggle is over. A layer inside other layers gets its copy after the outermost one, with what it inherited there (its color scheme included), so a close that takes the layers around it too still draws the copy. That copy is shown again above the copy the same close then places for a layer around it. A layer has one copy. The layer-runtime record states it, and that the presentation plays its exit on the copy (AST-094 FR7). Core's shared inertCopy now takes every copied native control and form-associated custom element out of any form with form="", as its documentation says, so the swap's copies join no form either. It also sets anchor-name: none on each copied element that has an anchor name, so no live anchor gets a second element. * Merge branch 'main' into uikit/c26-layer-exit-copy * feat(Popover): under a finger, it grows out of its trigger and shrinks back into it Under a coarse primary pointer, Popover grows out of its trigger on the theme duration step nearest iOS's growth (`--duration-medium-min`) and the theme's entrance easing, and shrinks back into it on the theme's exit duration and easing, on the layer runtime's exit copy. A fine primary pointer keeps the layer's own entrance. * Merge branch 'main' into uikit/c26-layer-exit-copy * Merge the updated lower stack branch into uikit/c26-popover-present (own diff unchanged) * feat(Popover): a touch during the growth is ignored While the popover grows, the layer runtime's input hold takes every touch, as UIKit ignores touches while it presents a popover. A tap outside does not dismiss it and reaches nothing under it, and a pinch still zooms the page. This folds in the hold that meta#8223 carried, so the growth and its hold land together. * fix(core): the exit copy keeps no closed layer The exit copy found the inner layers to raise through a module-level set of every copied layer. A layer React removed on close stayed reachable there until a later close showed a copy. The inner layers are inside the closing layer, so they are now found there, and nothing keeps a layer once it is gone. Chromium 150 and later refuse a show during another popover's toggle, so the copy can open in the next frame. The browser spec's light-dismiss case now reads the copy after a frame, as its nested-close cases do. The cap check's comment now names only the copy cap. * docs(core): the exit copy's JSDoc states the empty-canvas rule once The empty-canvas rule stays in the first paragraph, and the inert-copy paragraph no longer repeats it or `inertCopy`'s own contract. * Merge the updated lower stack branch into uikit/c26-popover-present (own diff unchanged) * Merge main into the layer exit copy branch (layer-runtime.md verification table keeps both rows) * Merge the updated lower stack branch into uikit/c26-popover-present (own diff unchanged) * Merge main into the layer exit copy branch (meta#8147 landed) * Merge the updated lower stack branch into the Popover branch (meta#8147 landed; uikit index header lists popover and zoomMedia) * Merge the updated lower stack branch into the Popover branch * Merge main into the layer exit copy branch (placeOver's beforeNode beside keepInherited) * Merge the updated lower stack branch into the Popover branch (meta#10224 landed) * Merge main into the layer exit copy branch (meta#10224 landed) * feat(Popover): the touch presentation starts in the next frame The growth and its input hold now start on the first frame after the open, not in the task that opens the popover, so the opening tap's task lays out no more than a mouse opening does. Folded from meta#8662, so the group's squashes apply in order: these lines are this pull request's own. * docs(Popover): record the next-frame window with the code The input hold starts on the first frame after the open, so a touch that lands within that frame reaches the page. spec:AST-094 and Popover's record state that window here, beside the code that opens it. Moved from meta#8662. * fix(core): the exit copy reads its host's style before the insertion keepInherited read the outer layer's parent's computed style right after the copy was inserted there: a style read after a write. It reads only that host and writes only on the still-detached copy, so it now runs before the insertion. What the copy inherits is unchanged. * Merge the updated lower stack branch into the Popover branch * perf(Popover): the growth reads its boxes and timing before it writes The frame that starts a touch presentation wrote the CSS entrance off and the start scale, read both boxes, wrote the origin and the start frame, then read the theme's entrance tokens. Each read after a write restyled or laid the page out again. It now reads both boxes and the entrance timing in one pass, then writes, the start frame first: the motion engine may read the element's will-change as it sets it. The box needs no reset first: the opening task already took the CSS entrance off, and a dismissal leaves the element at full size. Shown again during the dismissal, the popover reads its entrance timing before its first write too. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…rink (meta#9823) * chore(core): the layer runtime's exit copy, for a touch presentation's close placeLayerExitCopy puts a static, inert copy of a closing layer where the layer was drawn, in the top layer, for a touch presentation's close to play on: the browser hides a popover the moment it closes. The copy keeps the layer's box, transform, presented look, overflow and scroll positions; has no ids, anchor names, form names or form owner; is inert and hidden from assistive technology; and its descendants' keyframe animations and transitions do not start again. A refused show during the closing toggle waits, hidden, until the toggle is over. A layer inside other layers gets its copy after the outermost one, with what it inherited there (its color scheme included), so a close that takes the layers around it too still draws the copy. That copy is shown again above the copy the same close then places for a layer around it. A layer has one copy. The layer-runtime record states it, and that the presentation plays its exit on the copy (AST-094 FR7). Core's shared inertCopy now takes every copied native control and form-associated custom element out of any form with form="", as its documentation says, so the swap's copies join no form either. It also sets anchor-name: none on each copied element that has an anchor name, so no live anchor gets a second element. * Merge branch 'main' into uikit/c26-layer-exit-copy * feat(Popover): under a finger, it grows out of its trigger and shrinks back into it Under a coarse primary pointer, Popover grows out of its trigger on the theme duration step nearest iOS's growth (`--duration-medium-min`) and the theme's entrance easing, and shrinks back into it on the theme's exit duration and easing, on the layer runtime's exit copy. A fine primary pointer keeps the layer's own entrance. * test(Popover): browser proofs of the touch presentation's grow and shrink Real-browser proofs, in Chromium and WebKit, that under a finger Popover grows out of its trigger on the theme's entrance tokens and shrinks back on its exit tokens, keeps advancing through a blocked main thread, does no Layout or Paint per frame, and cross-fades under Reduce Motion, with storyboards of both. * Merge branch 'main' into uikit/c26-layer-exit-copy * Merge the updated lower stack branch into uikit/c26-popover-present (own diff unchanged) * Merge the updated lower stack branch into uikit/c27-popover-present-proofs (own diff unchanged) * Merge meta#8187's input hold into the Popover touch proofs * feat(Popover): a touch during the growth is ignored While the popover grows, the layer runtime's input hold takes every touch, as UIKit ignores touches while it presents a popover. A tap outside does not dismiss it and reaches nothing under it, and a pinch still zooms the page. This folds in the hold that meta#8223 carried, so the growth and its hold land together. * test(Popover): browser proofs of the hold while the popover grows A tap outside, a tap on the trigger and a pan while the popover grows do nothing, a pinch still zooms, and a storyboard shows the growth going on under a tap outside. Folded from meta#8223. * fix(core): the exit copy keeps no closed layer The exit copy found the inner layers to raise through a module-level set of every copied layer. A layer React removed on close stayed reachable there until a later close showed a copy. The inner layers are inside the closing layer, so they are now found there, and nothing keeps a layer once it is gone. Chromium 150 and later refuse a show during another popover's toggle, so the copy can open in the next frame. The browser spec's light-dismiss case now reads the copy after a frame, as its nested-close cases do. The cap check's comment now names only the copy cap. * docs(core): the exit copy's JSDoc states the empty-canvas rule once The empty-canvas rule stays in the first paragraph, and the inert-copy paragraph no longer repeats it or `inertCopy`'s own contract. * Merge the updated lower stack branch into uikit/c26-popover-present (own diff unchanged) * Merge the updated lower stack branch into uikit/c27-popover-present-proofs (own diff unchanged) * test(Popover): the blocked-main-thread proof blocks from the page and asserts the verdict The page now starts the block once the growth has run a frame, and the proof asserts the harness verdict, which fails a judged part under 4 frames. Here the growth runs 600 ms at a constant speed, so each frame moves several pixels and a block that starts late on a loaded machine still lands inside it. * Merge main into the layer exit copy branch (layer-runtime.md verification table keeps both rows) * Merge the updated lower stack branch into uikit/c26-popover-present (own diff unchanged) * Merge the updated lower stack branch into uikit/c27-popover-present-proofs (own diff unchanged) * Merge main into the layer exit copy branch (meta#8147 landed) * Merge the updated lower stack branch into the Popover branch (meta#8147 landed; uikit index header lists popover and zoomMedia) * Merge the updated lower stack branch into the Popover branch (meta#8147 landed; uikit index header lists popover and zoomMedia) * Merge the updated lower stack branch into the Popover branch * Merge main into the layer exit copy branch (placeOver's beforeNode beside keepInherited) * Merge the updated lower stack branch into the Popover branch * Merge the updated lower stack branch into the Popover branch (meta#10224 landed) * Merge main into the layer exit copy branch (meta#10224 landed) * Merge meta#8187 (next-frame start folded in) into the Popover touch proofs * feat(Popover): the touch presentation starts in the next frame The growth and its input hold now start on the first frame after the open, not in the task that opens the popover, so the opening tap's task lays out no more than a mouse opening does. Folded from meta#8662, so the group's squashes apply in order: these lines are this pull request's own. * test(Popover): browser proofs of the next-frame start The touch-motion spec's cases for the growth that starts on the first frame after the open, folded from meta#8662 with the change they prove. * docs(Popover): record the next-frame window with the code The input hold starts on the first frame after the open, so a touch that lands within that frame reaches the page. spec:AST-094 and Popover's record state that window here, beside the code that opens it. Moved from meta#8662. * Merge meta#8187 (next-frame records moved in) into the Popover touch proofs * fix(core): the exit copy reads its host's style before the insertion keepInherited read the outer layer's parent's computed style right after the copy was inserted there: a style read after a write. It reads only that host and writes only on the still-detached copy, so it now runs before the insertion. What the copy inherits is unchanged. * Merge the updated lower stack branch into the Popover branch * perf(Popover): the growth reads its boxes and timing before it writes The frame that starts a touch presentation wrote the CSS entrance off and the start scale, read both boxes, wrote the origin and the start frame, then read the theme's entrance tokens. Each read after a write restyled or laid the page out again. It now reads both boxes and the entrance timing in one pass, then writes, the start frame first: the motion engine may read the element's will-change as it sets it. The box needs no reset first: the opening task already took the CSS entrance off, and a dismissal leaves the element at full size. Shown again during the dismissal, the popover reads its entrance timing before its first write too. * Merge the updated lower stack branch into the Popover touch proofs Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…utes each (meta#10258) * ci: the Chromium interaction proofs run in as few legs as fit ten minutes each, from re-recorded times * ci: the leg count costs an unrecorded spec by its tests at the lane's mean time a test, and the recorded Chromium times add TabBar and three newer specs * ci: the Chromium legs' matrix holds six, since main's merge ref already plans four * ci: the plans cost files as their counts do, an unreadable spec stops the leg, and a test pins the Chromium count's arguments * ci: the plan reads the test counts the leg count wrote, since the head checkout has replaced the scope checkout by then * ci: a changed spec the merge commit no longer holds costs nothing in the leg count, and tests cover the tests.json hand-off
…eta#10273) test(cli): search goldens and the readme probe read the catalog once experimental-isolation's search goldens run in this process through runCli, as its docs topics do, so the seven queries share one read of the docs catalog instead of fourteen new processes. One spawned search checks that the in-process run prints what the bin prints. The icon-set project's ten searches run in one new process, through a new test helper (startCliSequence) that streams each result as it ends. Nothing read for the project without icon sets can answer for it, and its searches share one read of the catalog instead of ten new processes. One spawned search checks that process against the bin. The readme test probes one Core name of each kind of match outside an operation, instead of every eighth name and two variants of each.
…rver in another zone no longer causes a mismatch (meta#10226) * fix(Calendar, DateRangeInput): today is read after hydration, so a server in another zone no longer causes a mismatch Calendar and DateRangeInput read the local date during render. A server and a browser on different dates then render different months, today markers, or range text, and React hydration fails. A new internal useToday hook reads today through useSyncExternalStore: null on the server and in the render that hydrates server markup, the local date in every later render. Calendar marks no today and, with no focusDate or value, keeps its month grid's space hidden until the month is known. DateRangeInput shows the year until then. * chore(changeset): tag and credit the Calendar today changeset * fix(Calendar): keep the opening month once today is known, and prove hydration in a real browser After hydration a Calendar with no focusDate or value re-derived its opening month on every render, so a pick in the second month of a two-month view moved both months. The first render that knows today now stores the opening month. useToday builds on useIsHydrating instead of a second hydration gate. The layout claim is limited to the default six-row grid. New tests cover the empty month label, the unavailable month buttons, the skipped announcement and the two-month pick, and a real-browser fixture renders on one date and hydrates on the next. * test(Calendar): file the hydration fixture stories under Core Storybook files every story under a package root, so the fixture moves from Fixtures/ to Core/CalendarHydration.
…ent docs (meta#10227) test(visual-gate): regenerate the probe theme from the current component docs The probe theme covered 874 of the 892 documented theming selectors, so the visual gate did not probe the other 18. This reruns the existing generator (pnpm visual:probe-theme); its --check now passes (275 targets, 892 selectors). Fixture-only.
…OM (meta#10275) * test(core): a presentation resolves with no matchMedia, as in a test DOM A ContextMenu or a DropdownMenu with `presentation="adaptive"`, and the presentation hooks themselves, throw where the window has no matchMedia: `window.matchMedia is not a function`. An app's own jsdom tests have no matchMedia unless they stub it. These cases fail until the next commit. * fix(core): a presentation resolves with no matchMedia, as in a test DOM `usePresentation` and `useAdaptivePresentation` read the pointer and the compact size through matchMedia. Where the window has none, they now answer no match, so the component renders its default popover, as `useMediaQuery` already does. The check runs in the subscription and the snapshot, never during a server render. * test(core): an adaptive Popover renders with no matchMedia Popover resolves `presentation="adaptive"` through its own resolver, which still throws `window.matchMedia is not a function` in a DOM without matchMedia. coarsePointer's `matchesMediaQuery` reads any query and matches nothing there. The Popover case fails until the next commit. * fix(core): each presentation resolver reads matchMedia through one guard The two presentation hooks and Popover's resolver subscribe through coarsePointer's `watchMediaQuery` and read through `matchesMediaQuery`, which answer nothing where the window has no matchMedia. The check now lives in one owner, beside `isCoarsePointer`, so a later fix reaches all three.
* ci: the WebKit interaction scope covers utils/inputAge The gesture fixtures will bundle the gesture host's input-age helper, so a change to it runs the WebKit interaction proofs, as the other modules they bundle do. * Merge branch 'main' into ci/webkit-scope-input-age
The note on when the menu is asked to show reads as an instruction, so it keeps to 20 words.
There was a problem hiding this comment.
Waiting on an engineering owner's decision, with one required fix.
ContextMenu's held-finger timeline matches the touch-presentation spec, but this head adds a requirement to a current record that needs owner sign-off.
- Owner decision:
component:ContextMenuFR12 adds the 400 ms commit and the 735 ms menu display to the current ContextMenu record. An engineering owner records approval by commenting/approve-spec 260feb38264efdee2c717bfea817ae69e5439ebcon this pull request (the author may, as an engineering owner), or says how FR12 should change.
Blocking:
- The Android Chrome-with-Gboard emulator run that
spec:AST-094DEC-27 requires at the exact head (normal and removed animations, plus a slow-device profile) is reported for the previous head only, so rerun it at this head and update the description.
#9816 does not contain this head yet, so the stack line holds again once it is restacked; pr-a11y (browser specs) hit its 15-minute limit with no failing test and needs a re-run.
[Automated review]
| * recognizer claims the finger. The page stops scrolling under it, and its | ||
| * lift sends no click and no compatibility mouse events. The menu opens where | ||
| * the finger landed: at `menuDisplayMs` if it is still down, or at the lift. | ||
| * A lift before the commit, a second finger or a scroll gets no menu. So does |
There was a problem hiding this comment.
Non-blocking (a miss in earlier rounds): a second finger after the commit still gets the menu (contextMenuPress.test.ts "ignores a second finger after the commit"; the changeset says so too), so this reads as if any second finger cancels. Suggest: "Before the commit, a lift, a second finger or a scroll gets no menu."
| // moves onto one, as with DropdownMenu's held finger. The menu is an | ||
| // internal focus target, not a control, so it shows no browser ring. | ||
| // Keyboard focus moves to an item instead. | ||
| outline: 'none', |
There was a problem hiding this comment.
Non-blocking (spec gap): after a touch open the menu itself holds focus with no outline, so someone who then reaches for a keyboard sees no focus indicator until an arrow key moves to a row. spec:AST-094 FR33 settles that no row is lit, but no record names an indicator for the menu itself; one sentence in FR5 or FR12 would settle it.
|
/approve-spec 260feb3 |
Stack: #9814 → #8125 → #9816 → #8133
Merge group: #9814, #8125, #9816, #8133
Behavior module: ContextMenu's touch hold.
Supersedes #8065, whose measured timeline is the first commit here.
User need
A held finger should open a context menu on UIKit's sequence: the menu at about 0.74 s, and no click at the lift. On main a finger gets a menu at ~500 ms, then a click.
Problem and solution fit
presentation="popover", a finger keeps today's 500 ms long press.spec:AST-094FR4, FR45); FR12 records them.Current specification
spec:AST-094FR13, FR25, FR32, FR33 anddesign:reduced-motiongovern; the hold stays internal until createContextMenu takes it (spec:AST-107FR8).Public delta and caller burden
An omitted
presentationmeans the touch model under a finger;presentation="popover"keeps today's menu. No API change. ContextMenu's import grows from 85,043 to 110,004 B gzip, mostly core's shared touch modules (Testing).Design and alternatives
UIKit's growth during the hold is separable, so #9816 adds it, on UIKit's lift spring. The watch also judges a mouse or a pen at its bubbling mousedown, as main's Effect did. So that Effect goes (
architecture:react-component-runtimeINV16), and desktop dismissal is unchanged.Evidence
Android (
spec:AST-094DEC-27): Code Quality's Chrome-with-Gboard emulator run at this head passes; its takes are under Testing. iPhone, iOS 26.2 Simulator at the stack's top: hold, menu, tap outside. Real-device iOS: the owner's card 33c pass.Scope
Writing rubric results
Over
component:ContextMenu: the pan gap goes; FR12 records the hold's internal moments. Owner-gated.Testing
Android, Code Quality's Chrome-with-Gboard emulator at 543d05f, this pull request's same own diff, six takes at normal speed, under a 4× CPU slowdown and with animations removed: the menu opens at the finger, an early lift and a pan open nothing, and an outside press closes it for touch, mouse and pen.
Restacked on main 4598223, which has #10275's guarded presentation read: this head merges #9814's head 57f4e92, ContextMenu's
usePresentationread goes through that guard, anduse-table-sortable-state-hook-usage.test.tsxpasses 3 of 3 here. The long-press spec's note keeps to 20 words. Local checks at 260feb3: core typecheck and that test. At the stack's top on the same base: the Layer, ContextMenu, Indicator, utils, motion, gestures, DropdownMenu and hooks units (4,997 passed);ContextMenuLongPressin Chromium (22 passed, 1 skipped) and in WebKit under the CI profile (19 passed, 4 skipped); the three ContextMenu a11y specs (27 passed).Earlier, restacked for the chain landing: that head merged #9814's head on main 79cc4ac and pins
boxHold.test.tsx's CheckboxList-row case topresentation="popover", since the touch model opens at 735 ms, past that case's 600 ms hold. Local checks at that head, 543d05f: core typecheck; the Layer, ContextMenu, Indicator, utils, motion and gestures units (3,550 passed); eslint on the changed files; check:knowledge; check:changesets.On #9814 at 5aff040, the merge worker's head, which merges main 3731ec3. FR5 merges row-wise with #9915's: the row is main's, minus the pan gap this pull request closes.
CodeBunny's review at 279fe10, addressed:
TouchLongPressPopover, which now ships here.watchOutsidePressjudges a mouse or a pen at its bubblingmousedown, as main's Effect did, so ContextMenu's own Effect goes. Red first: 5 of the 29outsidePress.test.tscases fail before the change; the two new guards pass either way. The shadow-root ContextMenu case now fires a mouse press'spointerdownbefore itsmousedown, as a browser does.Main's #9988 dismissal count: the one watch counts every outside press. Main's #9008: the record's FR8 rows and compatibility row merge row by row with this pull request's.
Local checks at c4f8964: core typecheck and the Layer, ContextMenu, DropdownMenu, utils and motion units (2,865 passed). At #9816's head on the same base, 4efcbc4:
CI=true pnpm lint(0 errors), core typecheck, the ContextMenu, Layer, DropdownMenu, motion, hooks, gestures, SegmentedControl and utils units (4,809 passed), check:knowledge, check:changesets, and the Chromium interaction specs for ContextMenu and Layer (34 passed, 1 skipped). On the previous base (main 9e559e9), the stack's top with this content (a4a1a46, apart from two comments) passed the full ui (21,276) and node (16,672) suites, the Chromium interaction specs for ContextMenu, Layer, DropdownMenu, Dialog and Drawer (60 passed, 1 skipped), the same specs in WebKit (51 passed, 10 skipped), and their a11y contracts (330 passed). Size (INV6), gzip, the import with its transitive modules: ContextMenu 85,043 → 110,004 B, DropdownMenu unchanged (79,793 B), CSS unchanged. ContextMenu's own touch code is about 2.4 KB of it for the whole stack (see #8133); the rest is core's shared gesture host, arena and long-press recognizer, which a page carries once for every touch component.Earlier local checks at 1af10a0, on #9814 at ed150e2 (main f10bc91): core typecheck; the ContextMenu, Layer, motion, hooks, gestures and DropdownMenu units with the press-coverage guard (3,408 passed); strict eslint and prettier on the changed files; check:knowledge; check:changesets; the comment-length check; and
pnpm review:pre(no findings). Browser:ContextMenuLongPress.interaction.browser.spec.ts(the 735 ms menu, and the outside pan that closes it while the page scrolls) in Chromium and WebKit. Red first: the 735 ms menu, the lift after the commit, an early lift, a second finger, a drift past the tolerance on one axis, the claimed finger's click and scroll, the outside pan and tap, the popover and fine-pointer long press, a touch open's focus,triggerAsswaps, consumer touch handlers, the commit haptic, and the recognizer's states each fail before the change. The timeline's other browser proofs come with #9816, held back for size. Code Quality's Android emulator run at 653a5cd (Chrome; default and Remove animations), the unsplit touch model this stack rebuilds, passes: the commit lands at 399–460 ms, and the menu opens at 734–746 ms at the finger. A pan gives no menu, and the menu stays open after the lift. The full gates run before this leaves draft. It changes gesture feel, so it also waits for a human check on its preview.Test names against main: none dropped. Main's five "a menu that was never opened" tests stay exactly as main has them. Six main tests are renamed where the touch model changed what they check: "opens on touch long-press" is "opens for a finger still down at 735 ms (iOS 26.2), not a millisecond before"; "composes consumer touch handlers with long-press detection" is "passes touch events through to the consumer's own handlers" and "still opens at 735 ms beside a consumer onTouchStart, which runs (#7167)"; "clears a pending long-press when a consumer prevents default on touch end" is "opens nothing for an early lift whose consumer onTouchEnd prevents default (#7167)"; "cancels the long-press when the finger moves past the threshold" is "cancels when the finger drifts past the tolerance on one axis (11.5 px)" and the popover "a drift past 10 px" case; the compatibility-mousedown case is "outside the touch model, a tap outside closes the menu through its compatibility mousedown, and a scroll that starts outside leaves it open"; and "presses that send no mousedown leave the menu open, as before" is "a mouse press that sends no mousedown leaves the menu open, as before".
Review notes
Follow-ups after merge, from non-blocking review notes: the touch layer's header reads as if any second finger cancels, where only one before the commit does; and no record names a visible focus indicator for the menu after a touch open.