Repository navigation
test(build): give the PostCSS fixture's Astryx package a manifest - #7797
Merged
Merged
Conversation
The PostCSS plugin now scans what each installed Astryx package publishes, which it reads from the package's manifest. The Babel-config test's fixture installed an Astryx module without one, so the plugin treated that module as app code and the test failed on main once both changes landed.
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Visual Regression40 of 486 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 20 more. Generated by PR Enrichment workflow | Storybook | Sandbox | 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 looks ready to merge: the new package.json puts the fixture's @astryxdesign/core back into the PostCSS plugin's library scan, so the test passes again and once more fails if the library pass picks up the app's Babel config. One optional fix: the new comment says a package without a manifest would count as app code, but the plugin skips such a package, so its module is never compiled (see the inline note).
[Automated review]
Comment on lines
+50
to
+51
| // An installed Astryx package has a manifest; the plugin scans what each | ||
| // one publishes, so without it the module would count as app code. |
There was a problem hiding this comment.
Small accuracy fix: without a manifest the plugin skips this package (libraryGlobs stops at index.js:60), so the module is never compiled at all. It cannot count as app code either, since any discovered node_modules/@astryxdesign/ path is classified as library (index.js:247); on main the emitted CSS holds only the app's blue rule. Suggested wording:
// An installed Astryx package has a manifest; the plugin skips a package
// without one, so this module would never be compiled.
4 tasks done
cixzhang
pushed a commit
that referenced
this pull request
Oct 21, 2026
…#7530) > Data items' `header` and `footer`, and the bottom sheet's level regions, follow once this merges. ## User need A sub-menu picking from a long list needs a filter field above its rows, often a "Create" button below. #7344 gave the root menu `header` and `footer`; a sub-menu has neither, so the field goes among the rows (DEC-8 forbids it) or the picker leaves the menu. ## Problem and solution fit `DropdownMenuSubMenu` takes `header` and `footer` under the root's rules (DEC-8, DEC-10). With either, the sub-menu's surface (the flyout, or the drilled-in view on a phone) is a modal dialog named by its row, holding the header, the sub-menu's `role="menu"` with rows only, and the footer. A flyout renders inside its parent menu, so `DropdownMenu` renders a host beside its menu for the dialog. A drilled-in view with regions replaces the root's menu and regions until popped, by a style rule that reads the host. ```tsx <DropdownMenuSubMenu label="Labels" header={<TextInput label="Filter labels" … />} footer={<Button label="Create label" … />}> {visibleLabelRows} </DropdownMenuSubMenu> ``` ## Current specification `component:DropdownMenu` (draft) gains a concept row, FR20, AR7 and DEC-13, all proposed and pending owner review. Other proposals in open pull requests: #7319 FR18/DEC-11, #7352 FR19/DEC-12, #7532 FR21/DEC-14, #7541 FR22/DEC-15. `module:DropdownMenu/useMenuPress` (current) gains a proposed ORD1a: a press released on a row of that dialog's menu acts, as FR1 requires for any flyout; a picker placed in its header or footer stays its own press root. ## Public delta and caller burden - `header` and `footer` on `DropdownMenuSubMenu`. - With either, the row says `aria-haspopup="dialog"`; Tab walks header, rows as one stop, footer, and wraps; an Escape nothing inside handled returns to the row; a pointer leaving the flyout keeps it open until focus leaves. Rendered beside the root's menu, such a flyout closes with the flyout holding its row. - Phone drill-in: a shown view follows its regions and caps at the root's `menuMaxHeight`. - `ContextMenu` renders no host: there the sub-menu shows its rows alone and warns in development; parity follows once this merges. - Without them, nothing changes. ## Design and alternatives Rejected: regions inside the flyout (inside the parent menu); a separate `DropdownMenu` per level; closing on pointer-leave, losing typed text. A drilled-in view's header sits above Back. ## Writing rubric results - R1 narration: none. - R2 research in a record: none. - R3 evidence naming a change: none. - R4 rejected alternatives over one line or outside a decision: none. - R5 pull request or issue references: none. - R6 ruled open questions listing options: none. ## Evidence See Testing. ## Scope - [x] One capability is added. - [x] Unrelated fixes, cleanup, and policy changes were removed or split. - [x] Consumer docs, focused tests, a story and a changeset ship with the feature. - [x] Public text and artifacts contain no internal Meta context. ## Testing Review round 8 (rebased onto `main` b74cf21): `pr-a11y` gets a "Run DropdownMenu sub-menu header and footer contract" step beside "Run DropdownMenu header and footer contract". It runs `SubMenuHeaderFooter.a11y.chromium.spec.ts` with `--project chromium`, so FR20's real-engine checks gate every head. That exact command passes locally against a Storybook built from this head (6 cases). 15 files, +1867/−121 (1,988 lines). DropdownMenu, ContextMenu, Layer, MoreMenu, RadioList, `useMenuPress` and doc-prop suites: 739 tests; header and footer Chromium specs: 16; `.github/scripts` workflow tests: 100; typecheck, `lint:strict`, `check:knowledge`, Storybook build: clean. Review round 7 (rebased onto `main` 863e167): `surfaceHost`, `surfaceOwnerId` and `menuMaxHeight` leave the exported `DropdownMenuContextValue` for `DropdownMenuSurfaceContext`, which the package does not export, layer-scoped and re-provided by each flyout as the menu context is; the public type matches `main`. A new `DropdownMenuContext.public.test.ts` pins its four members (it fails typecheck with the three back) and checks the internal context is not exported. Data-mode parity (data items' `header` and `footer`, `renderDropdownItems`, the bottom sheet's warning and their tests) moves to the follow-up, and the three pointer-leave cases share one setup: 14 files, +1856/−121 (1,977 lines). DropdownMenu, ContextMenu, Layer, MoreMenu, RadioList, `useMenuPress` and doc-prop suites: 739 tests; header and footer Chromium specs: 16; typecheck, `lint:strict` (no warnings), `check:knowledge`, Storybook build: clean. Rebased onto `main` d0f9afe, no conflicts or changes. The previous head's `pr-a11y` and `smoke-test` reds came from CI: main's #7802 added a `pr-a11y` step whose spec file the older head lacked, and jsdelivr was still propagating core@0.7.2. DropdownMenu, ContextMenu, RadioList, docProp* suites: 477 tests; typecheck, `check:knowledge`: clean. Review round 6: a flyout the pointer has left now decides that focus left once the move has completed, so an arrow key in a header radio group, where the browser moves focus itself, no longer closes it. A jsdom case replays the browser's order (focusout, a microtask checkpoint, focusin) and fails on the previous head; a Chromium case on the new "Submenu / header with a choice" story presses ArrowDown after the pointer leaves, and fails against a Storybook built from the previous head, where the flyout closes. DropdownMenu, ContextMenu, Layer, MoreMenu, RadioList, `useMenuPress` and doc-prop suites: 741 tests; header and footer Chromium specs: 16; typecheck, `lint:strict`, `check:knowledge`, Storybook build: clean. Review round 5 (rebased onto `main` 2c3ed99; the earlier `test` and `test-node` reds were `main`'s own, a PostCSS fixture that #7797 fixed): an Escape a control in a region already handled (a header menu closing, a field clearing itself) closes nothing more, and the new case fails on the previous head. The contract takes FR20, AR7 and DEC-13 so open proposals in this record stay distinct. DropdownMenu, ContextMenu, Layer, MoreMenu, `useMenuPress` and doc-prop suites: 646 tests; header and footer Chromium specs: 15; typecheck, `lint:strict`, `check:knowledge`, Storybook build: clean. Rebased onto `main` 577a62e after #7737 merged: one commit, 15 files, +1733/−109. DropdownMenu, ContextMenu, Layer, MoreMenu, `useMenuPress` and doc-prop suites: 645 tests; header and footer Chromium specs: 15; typecheck, `lint:strict`, `check:knowledge`, Storybook build: clean. Review round 4 (amended into the last commit): the surface host is owned through a ref, with only a key in state that changes when a host attaches (it moves when the root's header or footer comes or goes); a drilled-in view with regions caps at the root's `menuMaxHeight`. New cases: the view cap (fails on the previous head) and the host move (a guard). Gates below are for this head. Round 3, then stacked on #7737: the root steps aside for a view by a style rule over its surface host instead of a drill-in record kept in an Effect; a flyout with regions closes from the `onHide` of the flyout holding its row instead of an Effect; the bottom sheet's level regions moved to a follow-up. Each change has a case that fails with it reverted. - `DropdownMenuSubMenu.test.tsx` "header and footer" suite: 20 cases, plus one in `ContextMenu.test.tsx` (rows alone, with the warning). They cover the dialog's role, modality, name and row wiring; regions outside every menu; the Tab walk and wrap from a field, a row and a deeper flyout; field keys; Escape, an IME Escape and Escapes a region handled; a press from a menu row released on a dialog row (acts) or its field (outside); a menu placed in a region keeping its own press; pointer-leave with focus inside, focus moving within (a header radio's arrow included), then focus leaving; a nested flyout closing with its parent; the phone drill-in, the root stepping aside, a shown view following its regions and its height cap. - `__tests__/SubMenuHeaderFooter.a11y.chromium.spec.ts`, real Chromium: six cases (laptop structure, keyboard walk, picks, press-drag pick, a header radio after the pointer leaves; phone drill-in with the root stepped aside). - iOS 26.2 simulator (Mobile Safari), earlier head: phone drill-in as described above; on `main` the level has no field or button.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maintainer impact
mainis red:test-nodefails inpackages/build/src/postcss.test.mjs("compiles Astryx source without the app's Babel config"). Every pull request based on currentmain, and any release that gates on a greenmain, is blocked.Intended invariant
The test keeps proving what #7774 added: the PostCSS plugin's library pass compiles Astryx source without the app's Babel config, while the app's own files still get it. It should run against an Astryx package installed the way a real one is.
Change
#7775 and #7774 each passed on their own but conflict once both are on
main:@astryxdesign/*package publishes, read from that package'spackage.json; a directory without one is not treated as an Astryx package.node_modules/@astryxdesign/core/src/Box/Box.tsxbut nopackage.json.So, on
main, the fixture's Astryx module is compiled as app code (@layer product) and the assertion on@layer astryx-basefails. The fix gives the fixture's package a minimal manifest ({"name": "@astryxdesign/core"}, nofiles, so the whole package is scanned). Test-only; no plugin change.Evidence
main,pnpm exec vitest run --project node packages/build/src/postcss.test.mjsfails withexpected '@layer product {…' to match /@layer astryx-base \{[^]*color:\s*red/, the same failure asmain'stest-nodejob.packages/buildsuite.Scope
Testing
pnpm exec vitest run --project node packages/build: 7 files, 85 tests pass (the target test fails without this change).pnpm check:changesets: pass. No Changeset: test-only.