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
d7ace22 to
caff703
Compare
caff703 to
ef764f2
Compare
ef764f2 to
ab3bc68
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval of TabBar waits on an engineering-owner decision about its direction: a new lab component beside TabList, its API, tab roles without tab panels, and geometry and motion fixed to iOS 26.2, all resting on the draft spec:AST-094, which no owner has approved yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) or by asking for changes to that direction. Whatever the decision, these still need fixing:
- Land the stacked predecessors on their own so this diff is the TabBar commit; the range here is about 27,600 authored lines, and the TabBar commit alone is 2,022.
- Add a Changeset naming
@astryxdesign/clifor the newastryx discoverentries; the lint job's Changeset coverage check fails without it. - Remove
zIndex: 2from the fixed root inTabBar.tsx, which sets a page-level stacking rank against app content (spec:AST-027FR1/FR3). - Build the badge suffix (", 3") from one translated ICU message instead of a literal joined in code.
- Tell callers to declare the bar as
insetonLayerProviderso toasts and menus clear it, and correct the changeset and motion README, which list lens valuesuikit.tabBardoes not contain.
[Automated review]
| position: 'fixed', | ||
| insetInline: 0, | ||
| bottom: 0, | ||
| zIndex: 2, |
There was a problem hiding this comment.
zIndex: 2 on this position: fixed root sets a page-level stacking rank against app content, which spec:AST-027 FR1 rules out ("MUST NOT establish an ordering contract against unrelated components, application content"), and FR3 keeps the outer owner at its parent's paint level. Drop it here and keep the lens and badge ordering inside an isolated capsule. Whether the bar must escape page content through the Layer host is part of the placement decision.
| </span> | ||
| {hasBadge(item.badge) ? ( | ||
| <VisuallyHidden> | ||
| {`, ${item.badgeLabel ?? String(item.badge)}`} |
There was a problem hiding this comment.
This joins a literal , to the badge inside the tab's accessible name ("Library, 3"). The internationalization architecture's INV1a asks for joined labels to be one ICU message resolved by the translator. A key with a {badge} placeholder, through useTranslator from @astryxdesign/core/i18n (as lab's Chat and Comments do), keeps the separator localized.
| name: 'TabBar', | ||
| displayName: 'Tab Bar', | ||
| description: | ||
| "The bar. It sits fixed at the bottom of the viewport, over the page, so leave 83 px of room at the bottom of the scrolling content. You own the selection: pass the selected tab's value and update it in onChange.", |
There was a problem hiding this comment.
Besides reserving 83 px under the content, apps need to declare the bar as inset={{blockEnd: 83}} on LayerProvider (spec:AST-059 FR6), or toasts and menus anchored near the bottom render under it. Worth saying here and showing in the stories.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [experimental] The UIKit constants table read through `@astryxdesign/core/motion/experimental` gains `uikit.tabBar`: the iOS 26 tab bar's measured geometry (capsule, items, badge) and its selection lens's springs, lift, flex and edge rubber band, each with its source. Nothing else changes. |
There was a problem hiding this comment.
uikit.tabBar has no lift, flex or edge rubber-band entries: only the geometry, the badge, lensTravel and reducedMotion. The same wording is in the uikit/tabBar.ts row of packages/core/src/motion/README.md. Also, packages/cli/assets/discover/official-packages.json changes in this commit, and CI's Changeset coverage check needs a Changeset naming @astryxdesign/cli.
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 |
ab3bc68 to
0909a22
Compare
There was a problem hiding this comment.
Approval of TabBar waits on an engineering-owner decision about its direction (a new lab component beside TabList, its API, tab roles without tab panels, and geometry and lens motion fixed to iOS 26.2 and kept under Reduce Motion), which rests on the draft spec:AST-094 that no owner has approved yet. 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. Whatever the decision, these still need fixing:
- Land the stacked predecessors on their own so this diff is the TabBar commit; the range is about 27,600 authored lines and the TabBar commit alone is 2,029.
- Remove
zIndex: 2from the fixed root inTabBar.tsx, which sets a page-level stacking rank against app content (spec:AST-027FR1/FR3). - Build the badge suffix (", 3") from one translated ICU message instead of a literal joined in code.
- Tell callers to declare the bar as
insetonLayerProviderso menus and toasts clear it. - Correct the changeset and motion README, which list lens values
uikit.tabBardoes not contain, and move the releasedadaptivethreshold change into its own pull request.
[Automated review]
| position: 'fixed', | ||
| insetInline: 0, | ||
| bottom: 0, | ||
| zIndex: 2, |
There was a problem hiding this comment.
zIndex: 2 on this position: fixed root, with no isolation owner, sets a page-level stacking rank against application content, which spec:AST-027 FR1 rules out ("It MUST NOT establish an ordering contract against unrelated components, application content, AppShell chrome, or another page region"); drop it and keep any local values inside an isolation: isolate owner.
| </span> | ||
| {hasBadge(item.badge) ? ( | ||
| <VisuallyHidden> | ||
| {`, ${item.badgeLabel ?? String(item.badge)}`} |
There was a problem hiding this comment.
This joins the badge into the tab’s name with a literal English ", " ("Browse, !", "Library, 3"); internationalization INV1a requires one ICU message with placeholders resolved by the translator instead of call-site concatenation.
| name: 'TabBar', | ||
| displayName: 'Tab Bar', | ||
| description: | ||
| "The bar. It sits fixed at the bottom of the viewport, over the page, so leave 83 px of room at the bottom of the scrolling content. You own the selection: pass the selected tab's value and update it in onChange.", |
There was a problem hiding this comment.
Besides the 83 px of padding, spec:AST-059 FR6 has an app with a bar floating over a viewport edge declare it on LayerProvider’s inset (here blockEnd: 83), so menus, popovers and toasts clear the bar; the doc should say so.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [experimental] The UIKit constants table read through `@astryxdesign/core/motion/experimental` gains `uikit.tabBar`: the iOS 26 tab bar's measured geometry (capsule, items, badge) and its selection lens's springs, lift, flex and edge rubber band, each with its source. Nothing else changes. |
There was a problem hiding this comment.
uikit.tabBar holds lensTravel and reducedMotion for the lens, not lift, flex or edge rubber-band values (the PR puts press-and-drag and minimize out of scope); please describe what ships, here and in the motion README row.
0909a22 to
c3db081
Compare
There was a problem hiding this comment.
Approval of TabBar waits on an engineering-owner decision about its direction (a new lab component beside TabList, its API, tab roles without tab panels, and geometry and lens motion fixed to iOS 26.2 and kept under Reduce Motion), which rests on the draft spec:AST-094 that no owner has approved yet. 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. Whatever the decision, these still need fixing:
- Land the stacked predecessors on their own so this diff is the TabBar commit; the range is about 28,000 authored lines and the TabBar commit alone is 2,044.
- Remove
zIndex: 2from the fixed root inTabBar.tsx, which sets a page-level stacking rank against app content (spec:AST-027FR1/FR3). - Build the badge suffix (", 3") from one translated ICU message instead of a literal joined in code.
- Tell callers to declare the bar as
insetonLayerProviderso menus and toasts clear it. - Describe what ships in the
uikit.tabBarchangeset and the motion README, which list lift, flex and rubber-band values the table does not contain, and rebase ontomain, wherepr-a11yruns a spec this branch predates.
[Automated review]
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
…changed) bbc0a4a is the merge of the heads this pull request builds on. With it as an ancestor, GitHub's compare from it shows exactly this pull request's own files.
…ps reach it Red first: with no label the tab list has no name, and aria-label or aria-labelledby passed to TabBar land on its root, where they name nothing.
As TabList does, the tab list is named by aria-labelledby, else aria-label,
else label, else the translated `@astryx.tabList.label` ("Tabs"), and the
aria naming props go to the tab list instead of the root.
spec:AST-094 FR4 names none of the tab bar's values, and Core's roles test pins the registered roles to exactly FR4's list, so registerTabBarTable() lists no motion role and each spring plays as measured, as the scroll and navigation tables do. The comments and the changeset no longer call them motion roles.
|
Thanks, fixed in this push:
The stack below lands first; the review link in the description shows only this pull request's files. |
There was a problem hiding this comment.
Approval of TabBar waits on an engineering owner providing its governing spec: no current record owns the component, and family:action-groups counts a tab bar as a member, so that decision must settle whether TabBar follows the family's press model (FR1-FR3, through the shared tracker) or carries the UIKit exception proposed in the comments, together with its API beside TabList, tablist roles without tab panels (spec:AST-020 FR15), the item copy the lens carries under a finger (spec:AST-017 FR60), the new mouse presentation (FR59) and a motion role for the lens spring (spec:AST-094 FR4, DEC-5). 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. Whatever the decision, two changes remain:
- Land the dependency stack through its own pull requests so this diff carries TabBar (30,468 authored lines today, 27,845 of them dependency work).
- Register the lens spring as the motion role the approved record names instead of playing it as a literal (
packages/core/src/motion/uikit/tabBar.ts:184-186).
[Automated review]
| export function registerTabBarTable(): void { | ||
| registerMotionTable('tabBar', tabBar); | ||
| } |
There was a problem hiding this comment.
This registers no motion role, so the lens plays tabBar.lensTravel as a literal spring. spec:AST-094 FR4: "A component MUST read touch motion through these roles and MUST NOT play a literal"; a component's own measured spring joins the role set through its record (DEC-5). Once an approved record names it, register it here with {roles: ['lensTravel']} and correct the comments at lines 24-26 and 178-180 and the changeset.
| {lens.isCopyShown ? ( | ||
| <div | ||
| ref={lens.copy} | ||
| data-tab-bar-copy="" | ||
| {...stylex.props(styles.copyInner)}> | ||
| <div | ||
| {...stylex.props( | ||
| styles.copyOuter, | ||
| styles.copyOuterGeometry( | ||
| capsuleInset, | ||
| px(tabBar.barHeight), | ||
| ), | ||
| styles.copyRest(lensOffset(restIndex, count)), | ||
| )}> | ||
| {items.map((item, index) => ( | ||
| <span | ||
| key={item.value} | ||
| {...stylex.props( | ||
| styles.copyItem, | ||
| styles.box( | ||
| `calc(${center(index, count)} - ${size(count)} / 2)`, | ||
| ITEM_TOP, | ||
| itemSize, | ||
| ITEM_HEIGHT, | ||
| ), | ||
| )}> | ||
| <ItemFace item={item} isSelectedLook isCopy /> | ||
| </span> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ) : null} |
There was a problem hiding this comment.
Under a finger every slide mounts a second copy of all the items here. spec:AST-017 FR60 lets a component render more elements under touch than under a fine pointer only for a layer while it is open, so this copy needs an owner exception recorded with its element counts, which the open direction question asks for.
|
@cixzhang |
Depends on #8360, #8340 and #8562.
Review only this PR's change: vjeux/astryx@bbc0a4a...94bba42
User need
Part of matching Astryx's touch components to iOS 26, so apps built with it feel native on phones; this is TabBar's first PR. Phone apps switch sections with a bottom tab bar. Astryx has none, so apps hand-roll one without iOS's capsule, sliding lens and badges.
Problem and solution fit
onReselect.tablistnamed bylabel,aria-labeloraria-labelledby("Tabs" otherwise), one tab stop, arrow keys, Enter or Space.Current specification
spec:AST-094FR13, FR8, FR24 and FR4 (FR4 names none of the tab bar's values, so its springs play as measured). An engineering owner decides this new lab component and its manually activatedtablist.Public delta and caller burden
New lab
TabBar,TabBarItem; coreuikit.tabBar; message@astryx.tabBar.badgedItem; names added tointernal/motionandinternal/touch. Leave 83 px below content.Size, gzip against the base (no TabBar): TabBar's import 0 → 40.3 KB JavaScript, 0 → 1,524 B CSS.
Design and alternatives
Evidence
iOS 26.2 simulator (taps, a 600 ms block, Reduce Motion), then Android 15 Chrome.
Scope
Stack: #8360 (which carries #8320 and the PRs below it), with #8340 and #8562 merged in → this PR.
Testing
Try it:
pnpm storybook, Lab › TabBar › Default.label, the tab list reads as "Tabs".Browser tests: the slide follows UIKit's spring, even through a 600 ms block; each tab is drawn once at rest; a mouse runs no touch code.
Passing:
pnpm test:interaction TabBar,pnpm vitest run packages/lab/src/TabBar,pnpm typecheck, lint.