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 PreviewNew ComponentsSearchField (@astryxdesign/lab) · View in Storybook
Component 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.
This needs an engineering owner's decision before it can be approved. Nothing on main governs the TabBar's active search behavior (the field riding above the keyboard and the search circle becoming a Cancel button), and the draft touch-presentations spec this PR adds is not yet approved. An engineering owner can provide or identify the governing spec for that behavior 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. These changes are needed whatever that decision is:
- Land the stacked predecessors first, so this PR carries only its own commit instead of about 39,000 changed lines.
- Run the new interaction proofs under an existing CI owner, or amend the CI routing spec, instead of adding a separately routed
pr-interactionjob. - Give SearchField's default clear and close button names catalog messages instead of English literals.
- Move the active Cancel button out of the tab list, which otherwise has no available tab while search is active.
[Automated review]
| <button | ||
| type="button" | ||
| // Active, the circle is the field's cancel button, not a tab. | ||
| role={isSearchActive ? undefined : 'tab'} |
There was a problem hiding this comment.
While search is active, this button stays inside the role="tablist" capsule but drops role="tab", and every item tab is inert while searching. The tab list then exposes no tab and a stray "Cancel" button. Rendering the active Cancel button outside the tablist, beside the minimized circle, would avoid that.
| } | ||
| tabIndex={isSearchSelected ? 0 : -1} | ||
| ref={searchMotion.circle} | ||
| inert={isSearchSelected && !isSearchActive} |
There was a problem hiding this comment.
When onActiveChange(false) leaves the search tab selected (for example onActiveChange={setIsActive}, as in the component's test), this focused button turns inert in the same commit, and keyboard or VoiceOver focus falls to the document. Moving focus to the inline field or the collapsed circle first would keep it.
| cancelLabel = 'Cancel', | ||
| clearLabel = 'Clear text', |
There was a problem hiding this comment.
These English defaults become the clear and close buttons' accessible names. Astryx-owned assistive strings should come from the message catalog through t(), as TabBar's Cancel does; the props can stay as overrides.
| if git fetch --quiet --no-tags --depth=1 origin "$BASE_SHA" \ | ||
| && git show "$BASE_SHA:.github/scripts/pr-scope-touches.sh" > "$helper"; then | ||
| scope=0 | ||
| bash "$helper" "$BASE_SHA" '^(internal/interaction-harness/|playwright\.interaction\.config\.ts$|packages/[^/]+/src/|apps/storybook/)' || scope=$? |
There was a problem hiding this comment.
This job routes itself with its own path regex and adds a separate pr-interaction check. The CI routing spec (AST-030 FR11) asks new coverage inside an existing surface to join that surface's existing owners instead, or to amend the spec to admit a new owner.
| const range = doc.caretRangeFromPoint?.(x, y); | ||
| if ( | ||
| range != null && | ||
| editable.isContentEditable && |
There was a problem hiding this comment.
Only a contenteditable target gets the caret placed under the finger. For a claimed textarea or input, the browser's own tap is cancelled, so the first tap focuses without moving the caret.
| '@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.
An iPad mini in portrait (744 px wide) got the sheet under the old (max-width: 768px) query, so it flips to the popover here rather than keeping it; 768 px tablets flip the same way.
|
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.
|
9207607 to
c589f05
Compare
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved. Nothing on main governs the TabBar's active-search Cancel button (the search circle becoming a button named "Cancel", and the collapsed bar no longer being a tab list while it shows), while the field's rest above the keyboard already follows spec:AST-099. An engineering owner needs to provide or identify the governing spec for that behavior 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. These changes are needed whatever that decision is:
- Land the stacked predecessors first, so this PR carries only its own commit instead of about 49,500 authored lines.
- Run the interaction proofs inside an existing CI owner, or amend
spec:AST-030, instead of adding the separately routedpr-interactionjob. - Keep focus when Cancel ends search but the search tab stays selected; today the circle turns inert and focus falls to the page.
- Blur the field when Cancel falls back to
onActiveChange(false), asspec:AST-099FR5 requires, and add the Firefox for Android evidence that AST-099 makes a merge gate.
[Automated review]
| tabIndex={isSearchSelected ? 0 : -1} | ||
| // Under a finger, while the field waits inline, it is faded | ||
| // out; under a mouse it stays, selected, in place. | ||
| inert={isSearchSelected && isTouch && !isSearchCancel} |
There was a problem hiding this comment.
When Cancel ends search but the caller keeps the search tab selected (as lab-tabbar--more-tab-with-search does with onActiveChange={setIsSearchActive}), this circle turns inert in the same commit while it holds focus, and focus falls to <body>. Activating Cancel from the keyboard in that story leaves document.activeElement on the body (WCAG 2.2 SC 2.4.3). Moving focus to the leading circle, or to the field's search landmark as SearchField's own keyboard cancel does, before the circle goes inert would keep it.
There was a problem hiding this comment.
Fixed in df384bf: Cancel is now its own button over the circle, outside the tab list. Activated from the keys, it hands focus to the collapsed bar's circle (or to the selected tab once the bar expands) before it goes. A test focuses Cancel, activates it, and checks focus lands on the circle.
| } else { | ||
| search.onActiveChange?.(false); | ||
| } |
There was a problem hiding this comment.
Without a cancel() on the handle (the doc's "An input element works" case), nothing blurs the field here. The glass plays its deactivation back while the input keeps focus and the keyboard stays up, and the field stays lifted because keyboardHeight > 0. spec:AST-099 FR5 asks that a moving surface blur its focused control, and architecture:react-component-runtime INV11 applies that to Lab. Calling field?.blur() before onActiveChange(false) would do it.
There was a problem hiding this comment.
Fixed in df384bf: without a cancel() on the handle, Cancel blurs the field before onActiveChange(false), so the keyboard goes as the field moves. A test checks the field loses focus.
| if git fetch --quiet --no-tags --depth=1 origin "$BASE_SHA" \ | ||
| && git show "$BASE_SHA:.github/scripts/pr-scope-touches.sh" > "$helper"; then | ||
| scope=0 | ||
| bash "$helper" "$BASE_SHA" '^(internal/interaction-harness/|playwright\.interaction\.config\.ts$|packages/[^/]+/src/|apps/storybook/)' || scope=$? |
There was a problem hiding this comment.
This job routes itself with its own path regex and adds a separate pr-interaction check. spec:AST-030 FR11 says new coverage inside an existing surface "MUST join those owners and MUST NOT create a separately routed lane or required context", and FR10 says a new test type is not a new surface. Running these proofs as steps of an existing routed owner that already consumes the Storybook artifact, or amending AST-030 under FR10, would resolve it.
|
Posted by Cindy's agent on her behalf. The open spec question on this PR is the contract is settled by Still needed: i18n for the literals, Cancel inside the tablist, the |
c589f05 to
df384bf
Compare
|
The four items from the last review are in df384bf.
On the hold: there is no hold from vjeux on this PR — no draft state, no label, no line in the body or the threads. The body's old "Proposed; pending owner review" line was the only thing that read like one, and it now says plainly what it is: the Cancel button's role is the owner decision this PR asks for. |
…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.
At this layer's text too: the tab list's name, the circle and the split glass mounted only when the bar can collapse, leaving search from a disabled tab, the field held above the keyboard after Cancel, and the comments saying the tab bar's springs play as measured.
The no-scrollend case removes the handler from the global object too.
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: no spec on main governs the TabBar's active-search Cancel button (its own button over the search circle, outside the tab list, that ends the search), so an engineering owner needs to provide or identify the governing spec for it 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. The field's hold above the keyboard after Cancel now follows spec:AST-099 FR5, including a Cancel that returns to the previous tab. Whatever that decision, these changes are needed:
- Take the keyboard height from the visual viewport in Chrome on Android too, and stop setting
overlaysContent, inuseKeyboardHeight.ts, asspec:AST-099FR6, FR8 and DEC-4 require. - Land the stacked predecessors so this pull request carries only its own layer instead of about 44,600 authored lines.
[Automated review]
| name: 'TabBarSearchTab', | ||
| displayName: 'Tab Bar Search Tab', | ||
| description: | ||
| 'The search tab, as on iOS 26: a circle as tall as the bar at its trailing end, apart from the other tabs, which share a narrower capsule. Choosing it selects it like any tab, collapses the bar to a circle holding the tab it was chosen from (a tap on that circle returns there) and shows its field in the bar; by default the same tap activates the field and focuses it, which raises the keyboard on a phone. Active, the field spans the screen but for the search circle, which becomes its cancel button; on a phone both sit above the keyboard. Under a mouse it selects in place: the bar stays expanded and the field shows at once above it, focused, where the bottom accessory sits; Escape returns to the tab it was chosen from. TabBar reads its props and draws it; it renders nothing on its own.', |
There was a problem hiding this comment.
On this head the search circle stays an inert tab while a separate Cancel button lies over it, outside the tab list, and cancels through fieldRef.cancel() or blurs the field and calls onActiveChange(false) (TabBar.tsx:1200-1230, :1742-1758). Describing that button here, rather than the circle becoming the cancel button, keeps the consumer docs matching what renders.
…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.
…tree (AST-085 FR5) Reads Chromium's accessibility tree: the disabled tab keeps its name and is announced disabled on the tab itself; no other tab is.
The component doc said the search circle becomes the cancel button; the circle stays a tab, out of reach while the field is active, and a separate Cancel button lies over it.
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: no spec on main governs the TabBar's active-search Cancel button (its own button over the search circle, outside the tab list, named by @astryx.tabBar.cancelSearch), so an engineering owner needs to provide or identify the governing spec, or record the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. Independently of that decision, two changes are still required, and two small fixes remain (Lab's second isRtlElement, and docs that still call the search circle the cancel button):
- Lab's keyboard module takes Chrome's
navigator.virtualKeyboardreport as the keyboard height and turns onoverlaysContent(packages/lab/src/keyboard/useKeyboardHeight.ts:124-203), whilespec:AST-099FR6, FR8 and DEC-4 require the visual-viewport measurement in every browser and forbid overriding the app's own viewport setting. - The branch carries about 44,600 authored lines of stacked work; landing the predecessors so this pull request holds only its own layer is needed to review and revert it as one change.
[Automated review]
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: no spec on main governs the TabBar's active-search Cancel button (its own button over the search circle, outside the tab list, named by @astryx.tabBar.cancelSearch), so an engineering owner needs to provide or identify the governing spec, or record the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. The consumer docs now describe that Cancel button correctly; independently of the decision, two changes are still required, and two small fixes remain (Lab's second isRtlElement, and the fieldRef JSDoc and code comments in TabBar.tsx and useTabBarSearch.ts that still call the search circle the cancel button):
- Lab's keyboard module takes Chrome's
navigator.virtualKeyboardreport as the keyboard height and turns onoverlaysContent(packages/lab/src/keyboard/useKeyboardHeight.ts:124-203), whilespec:AST-099FR6, FR8 and DEC-4 require the visual-viewport measurement in every browser and forbid overriding the app's own viewport setting. - The branch carries about 44,700 authored lines of stacked work; landing the predecessors so this pull request holds only its own layer is needed to review and revert it as one change.
[Automated review]
| /** | ||
| * The field's handle: focused by the tap that activates it, and cancelled | ||
| * (`cancel`, when it has one) by the search circle, which is the active | ||
| * field's cancel button under a finger. Without `cancel`, the circle | ||
| * reports `onActiveChange(false)`. | ||
| */ |
There was a problem hiding this comment.
This JSDoc still says the field is cancelled by the search circle and that, without cancel, the circle reports onActiveChange(false). On this head a separate Cancel button over the circle does both, blurring the field first when the handle has no cancel (lines 1205-1230), and the consumer docs now say so (TabBar.doc.mjs:172). Worth wording it the same way, along with the header at lines 87-89 and useTabBarSearch.ts lines 26-31 and 79-80.
Depends on #8222 and #8224.
Review only this PR's change: vjeux/astryx@6793453...c79ed7f
User need
Part of matching Astryx's touch components to iOS 26, so apps built with it feel native on phones. On iOS, activating the search tab lifts its field above the keyboard, with Cancel beside it; in #8222 it stays under the keyboard, hiding what people type. This adds both.
Problem and solution fit
{preventScroll: true}.Current specification
spec:AST-099FR2–FR5 and FR8;spec:AST-094FR13, FR24. An engineering owner decides (no record governs it): Cancel as its own button over the search circle, named by@astryx.tabBar.cancelSearch.Public delta and caller burden
fieldRefmay carrycancel()and itsfocustakes options; core gains the keyboard gap, the cancel delay,@astryx.tabBar.cancelSearch, and #8224'ssearchininternal/motion.Size, gzip against #8222's head: TabBar's import 66.7 → 67.9 KB JavaScript, 1,968 → 1,980 B CSS.
Design and alternatives
Evidence
iOS 26.2 simulator before (#8222) and after; Android 15 Chrome with Gboard; Firefox for Android.
Scope
Stack: #8076 → … → #8320 → #8129 → … → #8222 → this PR, with #8224 merged in.
Testing
Try it:
pnpm storybook, Lab › TabBar › Search Tab, on a phone.Browser tests, with a simulated keyboard: the field lands 8 px above it and Cancel beside the circle, unanimated; nothing listens to the keyboard outside search; Cancel hands focus to the bar. On Firefox for Android the field rises over the real keyboard without panning the page.
Passing:
pnpm test:interaction TabBar,pnpm vitest run packages/lab/src/TabBar packages/lab/src/SearchField,pnpm typecheck, lint.