Repository navigation
spec(AST-012): compact is width below md or height below 500 on every platform - #8274
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report
|
|
/approve-spec 92df424 |
github-actions
Bot
removed
the
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.
Changes requested: the owner approval at this head settles the compact definition, but the records still disagree with current native and Selector contracts.
spec:AST-012FR9 binds@astryxdesign/nativeto the effectivemdand rules out platform size classes, whilespec:AST-082FR18 keepsadaptationsweb-only and AST-082 FR11 andspec:AST-093FR3 leave native presentation to platform controls, so amend those records here or scope FR9 to the web.Selector.spec.mdstill says its requirements describe shipped behavior, yet FR4 now describes the FR9 resolution that no code implements, so mark it pending and classify theadaptivechange under Compatibility.- Give FR9 and INV17 a boundary that puts AppShell's FR8 layout clearly in or out, and name ContextMenu, MultiSelector and the "768px and below" consumer docs in the Current-state impact paragraph.
[Automated review]
| while its viewport width is below the effective `md` point (FR1) or its | ||
| viewport height is below 500. The numbers are the same on every platform: CSS | ||
| pixels on the web, points on iOS, and density-independent pixels on Android; | ||
| native presentations do not map compact to platform size classes. The theme |
There was a problem hiding this comment.
This native sentence (and @astryxdesign/native on line 173) conflicts with current records this PR leaves unchanged: spec:AST-082 FR18 puts adaptations, where widthBreakpoints and so md live, in the web part that native themes report as having no native meaning, and AST-082 FR11 with spec:AST-093 FR3 leave native presentation to platform controls and conventions. Amend those records in this PR, or scope FR9 to the web.
| | FR2 | Without explicit placement, a non-search popover aligns the selected row over the trigger and clamps it to the viewport. Search popovers and explicit placement use normal Layer positioning. | Consumer docs, implementation, and geometry tests | | ||
| | FR3 | An option whose resolved selection indicator draws no content reserves no mark-column space. Visible selected or themed replacement indicators remain in layout at `indicatorPosition`; the resulting state-dependent label position or available width is intentional, and row content keeps its existing overflow behavior. | `itemMarkColumn`, focused indicator tests, and Chromium evidence stories | | ||
| | FR4 | `popover` uses an anchored Popover. `bottom-sheet` uses a modal BottomSheet. `adaptive` resolves to the modal bottom sheet on compact coarse-pointer screens and Popover otherwise. | Presentation controller and adaptive-presentation tests | | ||
| | FR4 | `popover` uses an anchored Popover. `bottom-sheet` uses a modal BottomSheet. `adaptive` resolves to the modal bottom sheet on coarse-pointer screens that are compact under `spec:AST-012` FR9 and Popover otherwise. | Presentation controller and adaptive-presentation tests | |
There was a problem hiding this comment.
This row sits under "These requirements describe shipped behavior on current main" (line 141), but the shipped resolver still uses (max-width: 768px) and (pointer: coarse) and the cited adaptive-presentation test checks that query. Mark the FR9 resolution as pending with its implementation state, and classify the adaptive edge change under Compatibility.
| `<part>Style` together, reaching that element and composing as INV6 describes. | ||
| No public prop accepts StyleX styles for an element that cannot also take a | ||
| class name and inline styles, following `spec:AST-071/DEC-1`. | ||
| - **INV17 — Compact has one shared definition.** A component that chooses a |
There was a problem hiding this comment.
"A component that chooses a presentation by screen size" has no boundary: AppShell picks its mobile layout by a caller-chosen width point with no height edge under spec:AST-012 FR8. Say whether that layout is in scope, for example by limiting INV17 and FR9 to components that resolve a popover, bottom-sheet or drill-in presentation.
| extension metadata remain useful implementation evidence. | ||
|
|
||
| The theme exports no compact definition, and `when` has no `height` field. | ||
| Selector's `adaptive` presentation and DropdownMenu's compact-touch decisions use |
There was a problem hiding this comment.
ContextMenu and MultiSelector resolve through the same query, and the ContextMenu and MoreMenu docs promise "768px and below"; name them here so the release note and doc updates cover every adaptive surface.
|
/approve-spec 0f7b094 |
There was a problem hiding this comment.
Changes requested: an engineering owner approved the compact definition at this head, and one record still contradicts it. spec:AST-082 FR18 now puts the compact definition in PortableTheme, and so in DefinedTheme, but the same record's Current-state impact (lines 420-422), DEC-11 and FR18 verification row (line 457) still say DefinedTheme is unchanged for web callers. Please say there that DefinedTheme gains the compact member additively, derived from the effective md, and add a verification state in which a theme overrides md; the new commit then needs a fresh exact-head approval.
[Automated review]
| portable entry exports `PortableTheme`: a theme's name, tokens, and theme-local | ||
| tokens, each resolvable without CSS to a concrete value per color mode, such as | ||
| portable entry exports `PortableTheme`: a theme's name, tokens, theme-local | ||
| tokens, and the compact definition of `spec:AST-012` FR9 (the effective `md` |
There was a problem hiding this comment.
Adding the compact definition here also adds it to DefinedTheme ("PortableTheme plus the web part", line 362), and it has to be there: today md lives only in the internal __adaptations, which native reports as web-only, so a theme that overrides md would otherwise reach native with different numbers. Lines 420-422 ("DefinedTheme [is] unchanged for web callers"), DEC-11 ("changes nothing for web callers") and the FR18 verification failure "DefinedTheme changes for a web caller" (line 457) still say the opposite. Please say there that DefinedTheme gains this member additively, derived from the effective md, and add a verification state where a theme overrides md.
| defaults. `defaultIsMobile` remains an SSR hint; the client query uses the | ||
| effective map after hydration. | ||
| - **FR9 — Compact has one definition on every platform.** A screen is compact | ||
| while its viewport width is below the effective `md` point (FR1) or its |
There was a problem hiding this comment.
Not blocking: "the effective md point" does not say which theme supplies it. FR8 gives AppShell the nearest provider, the root-theme fallback and the FR1 defaults, and its verification row tests no, nearest, root and nested themes; the same sentence here and those states in the FR2 (height), FR9 row would keep two resolvers from reading different themes.
|
/approve-spec 3df14ae |
There was a problem hiding this comment.
Changes requested: an engineering owner approved the compact definition at this head, and one verification cell still contradicts it. In spec:AST-082, the FR18 verification row (line 459) still lists "DefinedTheme changes for a web caller" as a failure, while FR18 and its Current-state impact now say DefinedTheme gains the compact member additively. Please narrow that signal to a removed or changed member, and add a failure for the new md-override state (the compact definition not following the theme's effective md); optionally, add "and at least 500 tall" where the Selector entry and AST-012's Current-state impact say a 768-wide screen opens the popover. The new commit then needs a fresh exact-head approval.
[Automated review]
| | FR16 | Package-boundary check, plus a native fixture app installed and bundled without `react-dom` or StyleX | an import of the portable entry; an import of a core component or the web theme entry from native; a listed build-time read from a native build script; an unlisted one; a core import of native; a core contract change | Native resolves a DOM type, `react-dom`, StyleX, or a web module; either direction of the boundary is crossed, an unlisted build-time read passes; or a core contract change does not reach native contracts on the next build | | ||
| | FR17 | Icon map coverage check per native platform, plus override resolution tests | every built-in name; a namespaced name; a theme platform icon with and without a version range; an app-registered icon; an icon slot given an unknown name in a development and a release build | A built-in name has no platform default, a theme or app icon fails to replace the default, a scoped icon applies outside its range, a web icon is substituted, or an unknown name draws something in a release build or does not stop a development build with its name | | ||
| | FR18 | Type tests over `PortableTheme` and `DefinedTheme`, token resolution tests per color mode, and `defineNativeTheme` reporting tests | each maintained theme; a theme with component overrides, icons, indicators, media surfaces, and adaptations; light and dark; a length, a color, and a font token | `DefinedTheme` changes for a web caller, a portable token needs CSS to resolve, `PortableTheme` references a React or DOM type, or a web part with content is dropped without a report | | ||
| | FR18 | Type tests over `PortableTheme` and `DefinedTheme`, token resolution tests per color mode, and `defineNativeTheme` reporting tests | each maintained theme; a theme with component overrides, icons, indicators, media surfaces, and adaptations; light and dark; a length, a color, and a font token; a theme that overrides `md`, whose compact definition follows the override | `DefinedTheme` changes for a web caller, a portable token needs CSS to resolve, `PortableTheme` references a React or DOM type, or a web part with content is dropped without a report | |
There was a problem hiding this comment.
FR18 (lines 355-363) and Current-state impact (lines 420-424) now say DefinedTheme gains the compact definition additively, but this row still fails when "DefinedTheme changes for a web caller". The new state "a theme that overrides md, whose compact definition follows the override" also has no failure signal. Suggested failure cell: "DefinedTheme removes or changes a member a web caller uses, the compact definition does not follow the theme's effective md and the 500 edge, a portable token needs CSS to resolve, …".
| `popover`. | ||
| - `adaptive`'s compact test becomes `spec:AST-012` FR9 (width below the | ||
| effective `md` point or height below 500) in place of the shipped | ||
| `(max-width: 768px)` query: a coarse-pointer screen exactly 768 pixels wide |
There was a problem hiding this comment.
Not blocking: under FR9, a coarse-pointer screen exactly 768 wide opens the popover only when it is at least 500 tall (with the default md); a 768 × 499 screen opens the sheet, as the next clause implies. Consider adding "and at least 500 tall" here and in docs/specs/AST-012/spec.md lines 246-248.
3df14ae to
d01df17
Compare
|
/approve-spec d01df17 |
…e on every platform
AST-012 FR9 and DEC-6: one compact definition the theme exports, with the same
numbers in CSS px, iOS points, and Android dp, and no mapping to platform size
classes; every presentation resolver uses it. FR2 adds the height condition
{below: 500}. architecture:public-component-api INV17 makes components use it,
and Selector FR4 names it.
…and AST-093 name it; compact excludes layout widths; Selector change pending
…verify an md override
d01df17 to
4676a94
Compare
|
/approve-spec 4676a94 |
There was a problem hiding this comment.
Changes requested: the compact definition is approved at this head, and two sentences in the records still need a one-phrase fix each.
docs/specs/AST-012/spec.mdlines 246-247: the shipped(max-width: 768px)query already counts a 768-wide screen under 500 tall as compact, so write "wider than 768 pixels" instead of "at least 768 pixels wide".packages/core/src/Selector/Selector.spec.mdlines 50-51: the opening compatibility line should also name theadaptivecompact-test change, for example "except thepresentationdefault (DEC-1) andadaptive's compact test (below)".
The new commit then needs a fresh exact-head approval, which an engineering owner records by commenting /approve-spec <full head SHA>.
[Automated review]
| ContextMenu and MoreMenu consumer docs describe it as "768px and below": a coarse-pointer screen exactly 768 pixels wide and at least 500 tall counts as compact, and a landscape phone at least 768 pixels wide | ||
| but under 500 tall does not. Under FR9 the first opens the popover and the |
There was a problem hiding this comment.
The shipped (max-width: 768px) and (pointer: coarse) query matches a viewport exactly 768 px wide, so a 768 × 450 coarse-pointer screen already counts as compact and opens the bottom sheet; FR9 changes nothing there. Suggest "a landscape phone wider than 768 pixels but under 500 tall does not", so the release note lists only the screens that change.
| stable-default change, so it ships as `[breaking]` in a scheduled minor under | ||
| `spec:AST-017`; callers that need the anchored popover on every device pass | ||
| `popover`. | ||
| - `adaptive`'s compact test becomes `spec:AST-012` FR9 (width below the |
There was a problem hiding this comment.
This entry adds a second pending change to released behavior, but the section's opening line (line 50) still says released defaults and behavior are unchanged except the presentation default. Suggest "except the presentation default (DEC-1) and adaptive's compact test (below)".
…names the adaptive change up front
|
/approve-spec e760193 |
There was a problem hiding this comment.
Approval recommended: an engineering owner approved the compact definition at this head, and the five records now agree with each other and with the shipped (max-width: 768px) and (pointer: coarse) behavior they describe. The two wording fixes requested earlier are in place, in AST-012's Current-state paragraph and in Selector's opening compatibility line. Nothing more is needed in this pull request; adopting FR9 in code belongs to the follow-up implementation pull requests.
[Automated review]
Exact decision needed
What "compact" means when a component picks a presentation by screen size, and whether it differs by platform.
Existing authority searched
spec:AST-012FR1–FR2: width points (mdis 768) and a closedwhenvocabulary with no height field.architecture:public-component-apiINV14 and change coupling: components keep no parallel hardcoded breakpoint table.component:SelectorFR4:adaptiveuses the sheet on "compact coarse-pointer screens", with no number.design:mobile-typographyDR3 uses(width < 1024px) and (pointer: coarse)for text sizing, a separate concern this PR leaves alone.(max-width: 768px) and (pointer: coarse).Decision
spec:AST-012FR9 / DEC-6: a screen is compact while its width is below the effectivemdpoint or its height is below 500, with the same numbers on every platform (CSS px on the web, pt on iOS, dp on Android); native does not map compact to platform size classes. The theme exports this one definition; the web touch presentation resolver, Selector'sadaptivepresentation, and@astryxdesign/nativeall use it.spec:AST-012FR2:whengainsheight, accepting only{below: 500}.architecture:public-component-apiINV17: a component that chooses a presentation by screen size uses that definition and keeps no numbers of its own.spec:AST-082FR18:PortableThemecarries the compact definition, so native reads the same numbers; FR11 andspec:AST-093FR3: a native choice between presentations by screen size uses it, not size classes, while the platform's own control still presents the surface.mobileNav.breakpoint, is not compact.component:Selectorrecords theadaptiveedge change as pending and compatible underspec:AST-017FR1.Rejected alternatives
Claim scope and non-goals
The compact definition and the height condition only. Text sizing under
design:mobile-typography, which surfaces adopt a touch presentation, and resolver code are outside this PR.Evidence and implementation relationship
The touch presentation resolver contract lives only in the open
spec:AST-094revision (#8052), which this PR does not edit; it must use FR9. Native adopts FR9 when it gains a compact resolver. Behavior delta when the code conforms: a coarse-pointer screen exactly 768 wide and at least 500 tall opens the popover, and a landscape phone wider than 768 but under 500 tall opens the bottom sheet. Code follow-up belongs to separate implementation PRs. INV16 is the caller-control invariant from #8273 (merged); this PR adds INV17. No implementation is included.Writing rubric results
Scope
Validation
pnpm check:knowledge(CI lint)git diff --check