Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
33cc736 to
ecd4287
Compare
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 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.
Approval of the bottomAccessory addition to the lab TabBar waits on an engineering owner: no current spec covers the new prop, its placement and width (including two- and three-tab bars), or the draft AST-094 requirements this diff carries, so an engineering owner needs to provide or identify the governing spec and record the decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so if they are an engineering owner). Independently of that decision, three changes are needed:
- Rebase onto the landed stack so this diff carries only the accessory commit; against
mainit is about 30,000 authored lines. - Render the accessory only when its content is something React renders:
bottomAccessory={false}or''currently draws an empty capsule that blocks taps (TabBar.tsx:711). - Move the adaptive menu and selector breakpoint change into its own pull request and name the migration its Changeset needs under AST-017 FR3.
[Automated review]
| styles.bar, | ||
| styles.barSize(px(tabBar.barHeight), px(naturalWidth(count))), | ||
| )}> | ||
| {bottomAccessory != null ? ( |
There was a problem hiding this comment.
bottomAccessory != null lets false and '' through, so bottomAccessory={nowPlaying && <MiniPlayer />} draws an empty 360 × 48 capsule that takes taps while nothing plays; a function that returns null does the same. Resolve the content first and render only when it is something React renders:
const accessoryContent =
typeof bottomAccessory === 'function' ? bottomAccessory(placement) : bottomAccessory;
const hasAccessory =
accessoryContent != null && typeof accessoryContent !== 'boolean' && accessoryContent !== '';| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] `presentation="adaptive"` on DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector chooses the bottom sheet on a compact touch screen: a coarse primary pointer and a viewport narrower than 744px or shorter than 500px (it was 768px wide and below). An iPad mini in portrait keeps the anchored popover; a phone in landscape now gets the sheet. |
There was a problem hiding this comment.
This changes documented presentation="adaptive" behavior on five released components: coarse-pointer viewports 744-768 px wide lose the bottom sheet, and wider ones under 500 px tall gain it. AST-017 FR3 asks the Changeset to name the prop or option that restores the old default, and none exists here. It belongs in its own pull request with that migration, or classified under FR1 for a declared minor.
ecd4287 to
46b331b
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: no current spec covers Lab TabBar's new bottomAccessory prop and TabBarAccessoryPlacement type or their placement, and the draft AST-094 record adds requirements (FR1–FR23, DEC-1/DEC-2) that are not yet approved. An engineering owner can record that decision by commenting /approve-spec 46b331b965a2307730ee86667b04d5078b616e2d on this pull request, or by adding or identifying the record that states this direction; the author may do so if they are an engineering owner. Whatever that decision, these changes are still needed:
- Retarget or split the branch so this pull request carries only the accessory commit; it currently brings about 30,700 authored lines from the stack beneath it.
- Render nothing when
bottomAccessoryisfalse,''or a function returningnull(today an empty glass capsule appears,TabBar.tsx:711), and add a regression test. - Move the
presentation="adaptive"breakpoint change to its own pull request with the migration AST-017 FR3 asks the changeset to name, and run the interaction specs inside the existing test lane rather than a newpr-interactionjob (AST-030 FR11).
[Automated review]
| styles.bar, | ||
| styles.barSize(px(tabBar.barHeight), px(naturalWidth(count))), | ||
| )}> | ||
| {bottomAccessory != null ? ( |
There was a problem hiding this comment.
bottomAccessory != null lets false, '' and a function returning null through, so bottomAccessory={isPlaying && <Player />} with nothing playing draws an empty 48 px glass capsule above the bar (and inline when minimized) that takes taps. Resolving the content for the current placement first and rendering the accessory only when it is renderable fixes it; a test for false keeps it fixed.
| // 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 moves presentation="adaptive" from bottom sheet to popover on coarse-pointer viewports 744–768 px wide, and from popover to bottom sheet on landscape phones wider than 768 px, for DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector. AST-017 FR3 asks the changeset of a functional default change to name the prop or option that restores the old default; .changeset/touch-presentation.md names none.
| # them, so a change to an instrument is proven by its negative controls | ||
| # before a component spec relies on it. Chromium only on these runners; | ||
| # see the harness README for running WebKit. | ||
| pr-interaction: |
There was a problem hiding this comment.
pr-interaction routes on its own needs and if. AST-030 FR11: "New coverage inside an existing surface MUST join those owners and MUST NOT create a separately routed lane or required context." Running these specs inside the existing test owner would meet it.
…-touch-model restack)
…-touch-model restack)
46b331b to
2281a36
Compare
github-actions
Bot
added
the
needs:design-review
2281a36 to
aa16e7a
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner: no current spec covers Lab TabBar's new bottomAccessory prop and TabBarAccessoryPlacement type or their placement, the draft AST-094 adds requirements (FR1–FR23, DEC-1/DEC-2) that are not yet approved, including an FR4 that the theme motion roles in this branch contradict, and the isRtlElement and motion/reducedMotion.stylex exports are marked pending review. An engineering owner can record that decision by commenting /approve-spec aa16e7a3adadce9e3edd5fe867401a3f4fcbe9d6 on this pull request, or by adding or identifying the record that states this direction; the author may do so if they are an engineering owner. Whatever that decision, these changes are still needed:
- Retarget or split the branch so this pull request carries only the accessory commit; it currently brings about 40,000 authored lines from the stack beneath it.
- Keep the press controller's new selection lock out of this change, or classify it as a stable behavior change with pen evidence: it applies to every
usePressFeedbackcomponent and removes the iOS long-press callout fromLink(AST-017 FR22, FR59). - Tell accessory callers to declare the capsule on
LayerProvider(blockEnd: 139), not only the bar's 83 px (AST-059 FR6), and bring AST-094 FR8's Reduce Motion fades within the 200 ms the current reduced-motion record allows. - Run the interaction specs inside the existing test lane rather than a new
pr-interactionjob (AST-030 FR11). - Deliver a finger lift's
onChangefrom its handler rather than after the compositor hand-off or from an Effect, or record that exception (react-component-runtime INV2).
[Automated review]
| name: 'bottomAccessory', | ||
| type: "ReactNode | ((placement: 'expanded' | 'inline') => ReactNode)", | ||
| description: | ||
| "A bottom accessory, such as a music player's now-playing bar: a 48 px capsule as wide as the bar, 8 px above it. When the bar minimizes it moves down inline beside the circle and gets narrower; pass a function to adapt its content to where it is (placement is 'inline' there). Leave 56 px more room at the bottom of the scrolling content.", |
There was a problem hiding this comment.
With an accessory the floating stack is 139 px tall (83 + 56), but this only asks for more scroll room, and line 30 has callers declare inset={{blockEnd: 83}}. Under AST-059 FR6 the app declares the persistent bar's extent on LayerProvider, so menus, popovers and a bottom toast viewport would clear the bar and still land on the capsule. Suggest telling accessory callers to declare blockEnd: 139, and doing so in the BottomAccessory story.
| @@ -283,6 +301,9 @@ function onPointerDown(event: PointerEvent): void { | |||
| gesture: idlePressGesture(), | |||
| pointerId: event.pointerId, | |||
| timer: null, | |||
| // A held finger never starts text selection on the surface (UIKit's | |||
| // controls and cells never do), on any coarse pointer. | |||
| unlock: lockSelection([element], event.target), | |||
There was a problem hiding this comment.
This lock reaches every usePressFeedback surface on a touch or pen press, not only rows: Button, Item, Tab, Switch and Link, whose <a href> would lose the iOS long-press callout (preview, Open in New Tab, Copy Link) to -webkit-touch-callout: none. AST-017 FR22 keeps stable runtime behavior out of an [experimental] Changeset, and FR59 asks for pen before-and-after evidence. Keeping links out of the callout lock and classifying the rest as a stable behavior change, in its own pull request, would settle both.
| # them, so a change to an instrument is proven by its negative controls | ||
| # before a component spec relies on it. Chromium only on these runners; | ||
| # see the harness README for running WebKit. | ||
| pr-interaction: |
There was a problem hiding this comment.
pr-interaction routes on its own needs and if. AST-030 FR11: "New coverage inside an existing surface MUST join those owners and MUST NOT create a separately routed lane or required context." Running these specs inside the existing test owner would meet it.
| on: a transition that travels becomes a cross-fade carrying the same change | ||
| of state. An entrance lands in place and fades in, an exit fades out in | ||
| place, and any other move fades through. A navigation push or pop | ||
| cross-fades over 0.5 s on the ease-in-out curve; a sheet, a full-screen |
There was a problem hiding this comment.
A 0.5 s push cross-fade and a sheet fading on its presentation spring exceed the current reduced-motion record's DR2 ("MUST change opacity only and MUST finish in 200 ms or less"), which this record neither cites nor proposes to amend; the animator in this branch already uses 200 ms. Conforming FR8 to DR2, or naming the DR2 amendment for its owners, would close the gap.
| // input took over): its choice still reaches the app, at once. | ||
| useEffect(() => { | ||
| if (!isTouch || isMinimized) { | ||
| pendingReportRef.current?.(); |
There was a problem hiding this comment.
This Effect delivers a waiting lift's choice, and with it the app's onChange, in reaction to isTouch or isMinimized changing; normally the choice arrives three frames and a timeout after the lift (afterHandOff). react-component-runtime INV2 has the callback fire "from the handler that caused the change, never from an Effect reacting to the resulting state" and says deferral "MUST NOT delay" the action. Delivering from the lift handler, or recording the Mobile Safari hand-off as an exception with that record's owners, would settle it.
The comments it rewrote say, at this layer's wording too, that the tab bar's springs play as measured; the lens's selection Effect says what it synchronizes.
…nges under it
Red first: four tabs shrinking to three under a finger held on the last one
throw at the lift ("Cannot read properties of undefined (reading 'value')"),
since the lift resolves its index against the new list.
…ges under it A press measured the tab centres of the list it began on. When the list changes under a held finger, the press now ends there as a cancel and the lens settles back to rest, so no move chases the old centres and the lift never hands onLift an index into the new list.
Its tab-list naming props sit beside this layer's minimizeBehavior, and the comments it rewrote say, at this layer's wording too, that the tab bar's springs play as measured.
Red first: expanded by a tap, the bar minimizes again on the next pixel down; where the browser sends no scrollend, a script's scroll after the person's counts; collapsed, the tab list stays in the accessibility tree and the circle is no button; and the circle and the split glass mount even with minimizing off.
… restarts on expand - Expanding starts the downward scroll run over, so an expanded bar takes another minimizeDistance down before it minimizes again (before, the next pixel down minimized it). - Where the browser sends no scrollend, the scroll watch ends when the gesture host's reader says the person's scroll ended (its own fallback), so a script's scroll after it no longer counts. - Collapsed, the circle is the bar's one element for assistive technology, as on iOS: a button beside the tab list, named after the selected tab and collapsed (aria-expanded), with the tabs inert until it expands; from the keys, focus lands on the selected tab. The capsule sits in the bar's box, which places the circle. - The circle and the glass's three pieces mount only when minimizing is on; otherwise the glass is one piece.
Its collapsed circle (a button beside the tab list, in the bar's box this layer added for the accessory), its tab-list naming props beside this layer's bottomAccessory, and its comments saying the tab bar's springs play as measured.
…urn keeps its speed Red first: expanding, "Next" shows past the glass's trailing cap while the glass grows; turned around mid-move, the accessory starts back from rest.
…rn keeps its speed - Expanding, the accessory's content is laid out at once for its new box while the glass grows into it. The content now shows through a window whose trailing edge rides the glass's trailing cap (a translate, with the content counter-moved so it stays put), so "Next" no longer shows past the glass for its first ~140 ms. A shrinking glass already holds the narrower content, so no window moves then. - Turned around mid-move, each of the accessory's springs starts from the velocity it had (spec:AST-094 FR6) instead of from rest.
…object too On the global object the onscrollend handler is an own property, so the browser without scrollend the test models still detected one.
The no-scrollend case removes the handler from the global object too.
The merge of meta#8129's fixes ran Prettier over every file it staged, which reformatted 17 files this stack does not change (docs, scripts and a test from main and the gesture foundation). They are back to the foundation's text.
The tap and the second finger's landing went out 20 ms apart on the test's wall clock, which a loaded box stretched until the lens had reached Profile before the finger landed. They now go back to back, and the hold is a wait after the landing.
… spring from rest The turn used to come after the page settled, when the accessory had almost stopped, so a carried and a dropped velocity looked alike. The drag now ends at rest and the circle is tapped at once, and the first frame back is compared with the expand spring from rest.
…rt times The expand's first frame is checked against the expand spring started from the offset the accessory turns from, with the speed the collapse had at the turn. The turn's moment is the two animations' start times apart, so a late tap (a loaded box) moves the prediction with it instead of failing the case.
There was a problem hiding this comment.
Approval waits on an engineering owner: no current spec covers Lab TabBar's new bottomAccessory prop and TabBarAccessoryPlacement type, including the slot's place in the bar's anatomy, its width at every tab count (118 px inline with two tabs), and its two springs, which are not motion roles and on expand settle about 80 ms ahead of the bar. An engineering owner can record that decision by commenting /approve-spec 646c1500c2153a7164bc0413a39b16df54f5ef7f on this pull request, or by adding or identifying the record that states this direction; the author may do so if they are an engineering owner. Whatever that decision, two changes are still needed:
- Reduce this pull request to the accessory layer, since against
mainit carries about 34,500 authored lines from the stack beneath it. - Record before-and-after element counts for the seven elements the accessory adds, as
component-style-authoringINV7 asks, or draw the glass caps on pseudo-elements.
[Automated review]
| styles.bar, | ||
| styles.barSize(px(tabBar.barHeight), px(naturalWidth(count))), | ||
| )}> | ||
| {hasAccessory ? ( |
There was a problem hiding this comment.
With an accessory the bar renders seven more elements here: the wrapper, an aria-hidden glass box holding three glass pieces, the content's window and the content box. component-style-authoring INV7 asks that "any added element is justified against this invariant with measured element counts before and after" and puts decoration with no semantic role on a pseudo-element. Please record the BottomAccessory story's counts at rest and minimized under a fine and a coarse pointer, or draw the glass caps on pseudo-elements, which this branch's animator can drive.
The Effect that ends a press when the recognizer turns off, and the one that ends it when the tab list changes under a finger, now say what they synchronize, as the selection Effect does.
While the bar is collapsed its tabs are inert, so focus never lands on a tab: keyboard focus reaches the circle, whose activation expands the bar. The prop doc, the component doc and the hook said focus on a tab expands it.
|
Thanks, fixed in this push:
Element counts for INV7: a bar with an accessory adds 7 elements, and a bar without one adds none. The stack below lands first; the review link shows only this layer. |
There was a problem hiding this comment.
Approval waits on an engineering owner: no current spec covers Lab TabBar's new bottomAccessory prop and TabBarAccessoryPlacement type, including the slot's place in the bar's anatomy, its width at every tab count (118 px inline with two tabs), and its two springs, which are not motion roles and on expand settle about 80 ms ahead of the bar. An engineering owner can record that decision by commenting /approve-spec 675212270577981fd572123796e480cc0d086a93 on this pull request, or by adding or identifying the record that states this direction; the author may do so if they are an engineering owner. Whatever that decision, two changes are still needed:
- Reduce this pull request to the accessory layer, since against
mainit carries about 34,500 authored lines from the stack beneath it. - Thanks for the element counts; the four decorative glass elements (
TabBar.tsx:842-863) still need to move to pseudo-elements, or each needs a reason undercomponent-style-authoringINV7 why it must be an element.
[Automated review]
| <div aria-hidden {...stylex.props(styles.accessoryGlass)}> | ||
| <div | ||
| {...stylex.props( | ||
| styles.glass, | ||
| styles.platterStart(HALF_ACCESSORY), | ||
| )} | ||
| /> | ||
| <div | ||
| ref={minimize.accessoryMiddle} | ||
| {...stylex.props( | ||
| styles.glass, | ||
| styles.platterMiddle(HALF_ACCESSORY), | ||
| )} | ||
| /> | ||
| <div | ||
| ref={minimize.accessoryEnd} | ||
| {...stylex.props( | ||
| styles.glass, | ||
| styles.platterEnd(HALF_ACCESSORY), | ||
| )} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Thanks for recording the counts: with an accessory the bar adds seven elements and without one none, which the stories bear out. Four of the seven are this glass box and its three pieces, decoration with no role, which component-style-authoring INV7 puts on pseudo-elements ("A pseudo-element carries decoration that has no semantic role"). Drawing the caps on this box's ::before and ::after, which this branch's animator can drive as the lens's lifted look does, or giving each remaining element its INV5 reason, would settle it.
|
Thanks: the description now records the BottomAccessory story's element counts at rest and minimized, under a mouse and under a finger. The stack below lands first; the review link shows only this layer. |
Depends on #8154.
Review only this PR's change: vjeux/astryx@0056d05...646c150
User need
Part of matching Astryx's touch components to iOS 26, so apps built with it feel native on phones. iOS apps keep one app-wide control, like Music's now-playing bar, in a capsule above the tab bar that moves down beside the minimized bar. Astryx apps have nowhere to put it. This adds
bottomAccessory.Problem and solution fit
'expanded' | 'inline'), so content can drop controls inline.Current specification
Needs an engineering owner's decision (no record governs it): this API, and the accessory spanning the bar with two or three tabs.
spec:AST-094FR13, FR8.Public delta and caller burden
New
bottomAccessory?: ReactNode | ((placement) => ReactNode)andTabBarAccessoryPlacement. Leave 56 px more room below the content;uikit.tabBargains the accessory values.Size, gzip against #8154's head: TabBar's import 63.9 → 64.7 KB JavaScript, 1,923 → 1,946 B CSS.
Design and alternatives
Evidence
iOS 26.2 simulator (minimize and expand; a 600 ms block; Reduce Motion), then Android 15 Chrome.
Scope
Stack: #8360 → #8129 → #8141 → #8154 → this PR.
Testing
Try it:
pnpm storybook, Lab › TabBar › Bottom Accessory, on a phone or Chrome's device mode.Browser tests: the move follows UIKit's springs, keeps going through a 600 ms block, and page content clears both bars. The busy-page tests now block as the motion starts, so a slow machine can't make them flaky.
Passing:
pnpm test:interaction TabBar,pnpm vitest run packages/lab/src/TabBar,pnpm typecheck, lint.