Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
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 waits on an engineering-owner decision. No current spec covers the TabBarSearchTab and TabBar API (described here as "proposed; pending owner review"), the draft spec:AST-094 requirements, the module-level motion/experimental and gestures/experimental subpaths, or the new 744/500 px threshold for presentation="adaptive". TabBar motion also departs from the current architecture:theme-tokens record, which would need updating if touch presentations are to take motion from the UIKit table. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Separately from that decision, these need changes:
- Land the predecessor stack or retarget the base, so that this pull request carries only the search-tab commit instead of 29 commits and about 31,600 authored lines.
- Keep keyboard focus visible while searching, since Shift+Tab from the field lands on the Search tab at opacity 0 and the arrow keys then move through invisible tabs.
- Hold the tab that search was chosen from in state rather than in a ref written during render (
TabBar.tsxlines 824–833). - Name the option that restores the old adaptive default in the
touch-presentationChangeset, and fixspec:AST-094FR7's citation of aspec:AST-090that is not in the repository.
[Automated review]
| // searching, and choosing it leaves the search tab. | ||
| const previousValueRef = useRef<string | null>(null); | ||
| if (!isSearchSelected && selectedIndex >= 0) { | ||
| previousValueRef.current = value; |
There was a problem hiding this comment.
This writes the tab that search returns to into a ref during render, and shownIndex then places the lens and the leading circle from it. Current architecture:react-component-runtime INV4 says a ref must not be the sole source of rendered output. Keeping the previous tab in state, set where selection changes (for example in chooseSearch), stops a discarded render from leaving a tab that never committed.
| id={`${baseId}-tab-search`} | ||
| aria-selected={isSearchSelected} | ||
| aria-label={search.label} | ||
| tabIndex={isSearchSelected ? 0 : -1} |
There was a problem hiding this comment.
While search is selected, this button holds the tablist's only tab stop, but useTabBarSearch fades it to opacity 0 through searchMotion.circle. Shift+Tab from the field therefore focuses an invisible tab, and ArrowLeft moves on to invisible tabs. Keep the focused control visible, or move the tab stop to a visible one and make the faded tabs inert.
3942d57 to
2d706cc
Compare
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: no current spec governs the TabBarSearchTab and TabBar API (described here as "proposed; pending owner review"), the draft spec:AST-094 requirements, the module-level motion/experimental and gestures/experimental subpaths, or the new 744/500 px compact-touch threshold, and FR4 would also need the current architecture:theme-tokens record (INV3) updated before touch motion leaves the theme's motion tokens. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:
- Land the predecessor stack or retarget the base, so this pull request carries only the search-tab commit instead of about 32,000 authored lines.
- Keep keyboard focus visible while searching: Shift+Tab from the field lands on the Search tab at opacity 0, and the arrow keys then move through invisible tabs.
- Say in the
touch-presentationChangeset that the new query also flips the defaultDropdownMenuSubMenudrill-in on landscape phones and 744–768 px touch screens, and give the migration for both surfaces. - Keep the tab that search returns to in state rather than in a ref written during render, cancel the lift's pending report on unmount, and fix FR7's citation of a
spec:AST-090that is not in the repository.
[Automated review]
| export const COMPACT_TOUCH_QUERY = | ||
| '(width < 744px) and (pointer: coarse), (height < 500px) and (pointer: coarse)'; |
There was a problem hiding this comment.
This query also feeds useMenuDrillIn.tsx, so the default adaptive presentation of every DropdownMenuSubMenu changes with it: on an 852 × 393 touch phone a sub-menu now drills in where it flew out, and on a 744–768 px touch screen it flies out where it drilled in. The touch-presentation Changeset names only presentation="adaptive" on five components; please name the sub-menu change too and give the migration (presentation="flyout" or "drill-in" on the sub-menu), as spec:AST-017 FR3 asks for a changed default.
| '@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.
spec:AST-017 FR3 asks a Changeset that changes a default's behaviour to name the old and new default and the option that restores the old one. This names the old and new rule but no restoring option, and it leaves out the default sub-menu drill-in, which reads the same query.
| function afterHandOff(callback: () => void): void { | ||
| let frames = 0; | ||
| const step = () => { | ||
| frames += 1; | ||
| if (frames < HAND_OFF_FRAMES) { | ||
| requestAnimationFrame(step); | ||
| } else { | ||
| // After this frame's rendering update, not inside it. | ||
| setTimeout(callback, 0); | ||
| } | ||
| }; | ||
| requestAnimationFrame(step); | ||
| } |
There was a problem hiding this comment.
These animation frames and the timeout are never cancelled, and the report they lead to calls flushSync(() => onLiftRef.current(chosen)), which reaches the app's onChange. A bar that unmounts (or switches to a mouse) within those frames still reports the lift. Keeping the ids and cancelling them, and clearing pendingReportRef, on unmount and when the recognizer is disabled would follow architecture:react-component-runtime INV5.
| 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 in the repository at this revision or on main, so owners cannot check this departure. Could this cite the record that holds those claims, or drop the departure?
|
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.
|
…-touch-model restack)
…-touch-model restack; the before clip is meta#8192's build)
…-touch-model restack)
…-touch-model restack); the before clip is meta#8192's build
2d706cc to
0993496
Compare
github-actions
Bot
added
the
needs:design-review
|
Done in 0993496: direction comes from |
…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.
At this layer's text: the tab list's name and the aria naming props (the collapsed circle falls back to the same name), the circle and the glass's three pieces mounted only when the bar can collapse under a finger (minimizing is on, or a search tab is there), the scroll run and scroll-end fixes, the accessory content's window, a turned-around accessory keeping its speed, and the comments saying the tab bar's springs play as measured.
…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 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.
Holding every animation made the earliest one the sampling origin; when the bar's other layers started a frame before the field, the samples ran ahead of the spring by about half a pixel in WebKit.
…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.
Changes are requested: the search tab's direction is settled, but it and the stack under it conflict with current records, and this pull request still carries that whole stack.
- Keep the active search field above the phone keyboard without panning the page (
spec:AST-099FR3 and FR4), for example by landing #8243's handling with or before this tab and lettingfieldReffocus withpreventScroll. - Make a press that slides across tabs act on the tab it lifts on under a mouse too, through Core's action-group tracker as
family:action-groupsFR1 and FR5 ask of a tab bar, or have an owner record an exception. - Let the gesture host attach for every pointer, as
architecture:react-component-runtimeINV13 andspec:AST-094FR19 and FR43 now require, and keep the leading circle out of the tab order while the active field covers it. - Land or rebase past #8172 so this pull request carries only the search-tab layer (about 2,000 of its 36,300 authored lines).
[Automated review]
| } | ||
| }); | ||
| if (activates) { | ||
| search.fieldRef?.current?.focus(); |
There was a problem hiding this comment.
This focus raises the phone keyboard, but nothing places the active field from the keyboard height (it keeps the bar's row, :1456-1466) and the call passes no preventScroll, which the fieldRef type at line 239 cannot carry. Current spec:AST-099 FR3 says tab bar search positions from the keyboard height and FR4 that programmatic focus never scrolls the document. Landing #8243's keyboard handling with or before this tab, with fieldRef taking FocusOptions, would conform.
| } | ||
| aria-expanded={isMinimized && !isSearchSelected ? false : undefined} | ||
| aria-hidden={isMinimized ? undefined : true} | ||
| tabIndex={isMinimized ? 0 : -1} |
There was a problem hiding this comment.
While the search field is active under a finger it covers this circle: in Pixel 7 emulation the hit test at the circle's centre finds the search input and a tap there leaves the search tab selected, yet the circle keeps tabIndex={0} and the name "Home". Making it inert and unnamed while the field is active would keep hidden controls out of the tab order.
| } | ||
| }, | ||
| { | ||
| isEnabled: isTouch && !isMinimized, |
There was a problem hiding this comment.
The bar tracks a held press only under a coarse pointer, with its own recognizer: with a mouse, a press on Home released on Browse leaves Home selected. Current family:action-groups makes a tab bar a member ("joins by the rule, including … a tab bar"), FR1 and FR5 ask for the slide-and-lift under every pointer, and architecture:interaction-modality INV13 gives that tracking to actionGroupTracker.
| * - Recognizers attach, and the host registers listeners, only while the | ||
| * primary pointer is coarse (`spec:AST-094` FR19, FR24; coarsePointer.ts). | ||
| * Where it is fine, an attach is recorded but nothing is installed: no |
There was a problem hiding this comment.
Current architecture:react-component-runtime INV13 says "The gesture host attaches for every pointer, with no touch-only query", and spec:AST-094 FR19 now reads "Recognizers attach for every pointer, with FR43's exceptions" (DEC-21 rejects keeping gestures touch-only). This coarse-only gate cites FR19 for a rule it no longer states.
…arch field covers it Under a finger the active field spans the bar's row over the leading circle, which kept its tab stop and its name. It is inert while the field is active there, with a browser test (red first: the covered circle took focus).
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.
There was a problem hiding this comment.
Changes are requested: the leading circle now stays out of reach while the active search field covers it, but the search tab and the stack under it still conflict with current records, and this pull request still carries that whole stack.
- Keep the active search field above the phone keyboard without panning the page (
spec:AST-099FR3 and FR4), for example by landing #8243's handling with or before this tab and lettingfieldReffocus withpreventScroll. - Make a press that slides across tabs act on the tab it lifts on under a mouse too, through Core's action-group tracker as
family:action-groupsFR1 and FR5 ask of a tab bar, or have an owner record an exception. - Let the gesture host attach for every pointer, as
architecture:react-component-runtimeINV13 andspec:AST-094FR19 and FR43 require. - Point the two jsdom tests that leave search through the circle ("leaving search from the circle…" in
TabBar.test.tsx) at an inline field, since they now drive a circle that browsers keep inert. - Land or rebase past #8172 so this pull request carries only the search-tab layer (about 2,000 of its 36,300 authored lines).
[Automated review]
| ); | ||
| }); | ||
|
|
||
| it('leaving search from the circle with the keys moves focus on to the tab it returns to', () => { |
There was a problem hiding this comment.
With the default automaticallyActivates, choosing the search tab here makes the field active, so the circle this case focuses now carries inert (TabBar.tsx:1387): a browser refuses that focus, as the new browser case asserts, and jsdom 27 accepts it only because it implements no inert. The case at line 682 has the same premise; its click reaches the circle only because fireEvent skips hit testing. Rendering both with automaticallyActivates={false}, as the case at line 773 does, keeps them on the path people still have: the circle beside an inline field.
There was a problem hiding this comment.
Changes are requested: the newest commits only touch comments and docs, so the search tab and the stack under it still conflict with current records, and this pull request still carries that whole stack.
- Keep the active search field above the phone keyboard without panning the page (
spec:AST-099FR3 and FR4), for example by landing #8243's handling with or before this tab and lettingfieldReffocus withpreventScroll. - Make a press that slides across tabs act on the tab it lifts on under a mouse too, through Core's action-group tracker as
family:action-groupsFR1 and FR5 ask of a tab bar, or have an owner record an exception. - Let the gesture host attach for every pointer, as
architecture:react-component-runtimeINV13 andspec:AST-094FR19 and FR43 require. - Point the two jsdom tests that leave search through the circle at an inline field, and have the
minimizeBehaviordocs say that a minimized bar's search tab stays reachable and expands the bar when focused. - Land or rebase past #8172 so this pull request carries only the search-tab layer (about 2,000 of its 36,300 authored lines).
[Automated review]
| name: 'minimizeBehavior', | ||
| type: "'never' | 'onScrollDown'", | ||
| description: | ||
| "With 'onScrollDown', scrolling the page down collapses the bar into a small circle at its leading end that holds the selected tab's icon, keeping the content in view, as iOS's tab bar does. Scrolling back up leaves it collapsed; reaching the top of the page or activating the circle expands it. Collapsed, the circle is the bar's one element for assistive technology, named after the selected tab and collapsed, as on iOS. Only under a finger: a mouse-driven bar stays expanded.", |
There was a problem hiding this comment.
With a search tab (the Search Tab story's own setup), a minimized bar keeps its tab list un-inert, so the Search tab stays reachable beside the circle (TabBar.tsx:1034-1037, :1131), and focusing that tab expands the bar through the list's focus handler (TabBar.tsx:1134-1139). This doc now says the circle is the bar's one element for assistive technology and that only the page top or the circle expands it; naming the search-tab case here and in useTabBarMinimize.ts:95 would keep the docs true.
Depends on #8172.
Review only this PR's change: vjeux/astryx@43efaf0...9574764
User need
Part of matching Astryx's touch components to iOS 26, so apps built with it feel native on phones. iOS apps give search its own tab: a circle at the bar's end whose tap brings up the field and keyboard. Astryx has none; this adds
TabBarSearchTab. Land it with #8243, which keeps the active field above a phone's keyboard.Problem and solution fit
automaticallyActivates={false}keeps it inline).aria-expanded.onActiveChange(false).Current specification
spec:AST-094FR13 and FR24;spec:AST-012FR9;spec:AST-017FR21 and DEC-27;spec:AST-099DEC-3 (keyboard shown in the simulator and emulator to merge).Public delta and caller burden
New
TabBarSearchTab(value, label, icon, children, isActive, onActiveChange, fieldRef, automaticallyActivates);uikit.tabBargains the search values.Size, gzip against #8172's head: TabBar's import 61.7 → 63.5 KB JavaScript, 1,934 → 1,955 B CSS.
Design and alternatives
Evidence
iOS 26.2 simulator (a tap; Reduce Motion), a Chromium mouse, Android 15 Chrome.
Scope
Stack: #8076 → … → #8320 → #8129 → … → #8172 → this PR.
Testing
Try it:
pnpm storybook, Lab › TabBar › Search Tab.Browser tests: the field is focused inside the choosing tap; the morph follows UIKit's spring; leaving search moves focus to the returned-to tab, never a hidden one.
Passing:
pnpm test:interaction TabBar,pnpm vitest run packages/lab/src/TabBar,pnpm typecheck, lint.