Skip to content

feat(core): PowerSearch touch sheet flow behind experimental.presentation - #8001

Open
imdreamrunner wants to merge 1 commit into
mainfrom
feat/powersearch-experimental-bottom-sheet
Open

imdreamrunner wants to merge 1 commit into
mainfrom
feat/powersearch-experimental-bottom-sheet

Conversation

@imdreamrunner

@imdreamrunner imdreamrunner commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

One commit on main (692aa2647). This pull request now ships the touch flow the Lab prototype MobilePowerSearch ships today, including its refined touch filter flow: stacked steps, trailing remove actions, Clear all behind a confirmation that keeps read-only filters, an empty state with one centered Add filter, an unfocused Add filter search, fixed sheet chrome, and muted editor sections. The record it implements is component:PowerSearch DEC-3 as revised for that flow (FR10–FR21, FR15a, AR4–AR6, PR5). This pull request carries that record text and lifts its implementation-gap notes, except the date rows. The confirmation and the search-drag handling are Core's shared internal touch-sheet modules on main, which the Tokenizer's sheet also uses; this pull request imports them and adds no copy.

User need

On a compact touch screen, the PowerSearch field fights the software keyboard. When the tokenizer takes focus, the keyboard covers the half of the screen where the field menu and the editor popover open. The editor's Selector and Tokenizer then open more anchored layers inside a small popover, and each new token makes the field taller under the thumb. Builders who ship PowerSearch on phones have no supported touch surface. Today each product keeps the popover on touch or builds the flow itself.

Problem and solution fit

  • experimental.presentation on PowerSearch takes 'popover', 'bottom-sheet' or 'adaptive':
    • 'popover' is the default: the tokenizer field and the editor popover, unchanged.
    • 'bottom-sheet' shows the touch surface on every device.
    • 'adaptive' shows the sheet on compact coarse-pointer screens and the popover otherwise. It resolves through the shared useAdaptivePresentation policy, so a PowerSearch and a Tokenizer on one page switch together. On the server and while hydrating, it is the popover.
  • Trigger (PowerSearch.tsx): the field's box with display-only tokens and one button, which is the only tab stop. Its name is the label plus the visible content (WCAG 2.5.3). Disabled uses aria-disabled and stays focusable.
  • Flow (PowerSearchBottomSheet.tsx, loaded on demand). The steps stack: each opens above the one that led to it, which stays open beneath, and a dismissal (Escape, scrim, swipe) closes only the top sheet and reveals the step beneath. No sheet has a Back action.
    • Filters: one row per filter. A row opens the value step on its filter, and a trailing "Remove {field} filter" button removes it in one tap; a one-tap (empty-valued) filter's row removes it. A fixed header holds Add filter (an icon button) and a fixed footer holds Clear all and Done, with only the rows scrolling between them. With no filter, the body is one empty state ("No filters applied") whose centered primary Add filter is the only action.
    • Clear all opens a confirmation ("Clear all filters?", "Clear filters" / "Keep filters"). Confirming removes the editable filters, keeps read-only ones (onChange(kept, 'remove', firstRemovedIndex)), leaves Filters open with focus on Add filter, and announces "All filters removed" there, or "Editable filters removed" while a read-only filter remains, joined with a changed result count. This matches the popover's Clear all on main (FR23, DEC-4). The handler closes the confirmation with flushSync, then moves focus and announces, as the Tokenizer's sheet does; Keep filters returns focus to Clear all.
    • Add filter: grouped field rows under "General" and each group's name, or the content search with inline suggestions. Its search takes no focus on open. A vertical touch drag on the rows moves focus off a focused search, so the keyboard leaves. ArrowDown and ArrowUp move between the content search and its suggestions through Core's shared list focus (useInternalListFocus), without wrapping. A field whose only operator takes no value adds at once and shows a plus, not a chevron. When Add filter is the entry point, its fixed footer holds Done.
    • Value step: an info sheet that stages operator, then value, and confirms from its footer (Cancel, and Add filter or Apply). It has no Remove. A lone editor sits on the sheet surface; with the Operator group above it, each section is a muted container. Choice rows have no dividers. The radio rows' one tab stop and arrow, Home and End moves come from Core's shared list focus (useInternalListFocus), and a row that takes focus from those keys is checked.
    • Commit from a direct entry: when Add filter was the entry point, Filters is not open beneath, so a commit opens it in the same update that closes the steps (FR15a). From Add filter, one sheet closes as one opens: BottomSheet's replace (component:BottomSheet FR10). From the value step, two sheets close: the stack's ordinary closes and open (FR5–FR9). Focus lands on Add filter and is still there after every motion has settled (FR16); a test asserts it after the sheets settle.
    • string_list and entity_list values edit through the public Tokenizer with experimental={{presentation: 'bottom-sheet'}}, whose sheet opens above the value step, saves with Save, and asks before it discards changes.
  • Kept Core-only pieces: the value step edits a nested value with the popover's built-in NestedEditor; formatTokenValueText is the single source of a token's value text, so the Filters rows read as the token does; catalog keys are under @astryx.powersearch.sheet.*; the adaptive swap and hasAutoFocus handling; every visible field lists, including a nested-only one.
  • Touch targets: the header's Add filter and each row's remove button keep their drawn medium size. Under a coarse pointer, an invisible hit layer grows them to 44 px, using the coarse-only ::after pattern Core's clear button uses.

Current specification

  • component:PowerSearch DEC-3, FR10–FR21, FR15a, AR4–AR6, PR5, as on main for the Lab flow. This pull request lifts the record's implementation-gap notes that the port makes true and records Core's element counts in DEC-3.
  • component:PowerSearch FR23 and DEC-4 (on main): the popover's Clear all keeps read-only filters. The sheet's Clear all has the same semantics and the same catalog key.
  • spec:AST-017 FR21 and FR27: the experimental boundary, the [experimental] changeset, and no new theming target.
  • component:Selector FR4: the shared adaptive resolution.
  • component:Tokenizer DEC-3: the list-value sheet.
  • component:BottomSheet FR5–FR9 (the stack) and family:overlay-dismissal. The deprecated BottomSheetSwitcher is not used.

Public delta and caller burden

/** @experimental */
experimental?: {presentation?: 'popover' | 'bottom-sheet' | 'adaptive'};
  • No new exports. No stable prop, default or type changes. packages/core/api-report.md gains exactly that one prop experimental? line.
  • A caller that omits experimental gets the same render path, DOM and callbacks as main. The evidence is below.
  • In the sheet presentation, menuWidth and tokenOverflowBehavior have no surface. Each warns once in development.
  • @astryx.powersearch.sheet.* catalog keys are added. The sheet's Clear all reuses main's @astryx.powersearch.allFiltersRemoved and @astryx.powersearch.editableFiltersRemoved. No new theming target; the touch hit layer adds two private custom properties, documented in PowerSearch.doc.mjs.
  • fieldInputFamily.test.tsx lists PowerSearch as pending family:input-fields FR10: its sheet trigger composes the Field box, and its stable ref and root styling props stay where they are until their own decision.
// No opt-in: unchanged
<PowerSearch config={config} filters={filters} onChange={setFilters} />

// Opt in: the sheet flow on compact touch screens, the field and popover elsewhere
<PowerSearch config={config} filters={filters} onChange={setFilters}
  experimental={{presentation: 'adaptive'}} />

Design and alternatives

DEC-3 records the chosen model and the rejected alternatives: steps that replace one another with Back actions, Remove in the value step's footer, and focusing Add filter's search on open.

What changed from this pull request's earlier flow, item by item against the Lab flow:

  1. Steps stack and a dismissal reveals the step beneath (was: replace in place).
  2. No Back on Add filter.
  3. A trailing remove button per row, and tap-to-remove for empty filters (was: Remove in the value step's footer).
  4. Rows carry the remove button, not a chevron.
  5. empty-only fields add directly from their row, with a plus.
  6. Clear all asks first, keeps read-only filters, keeps Filters open and announces there (was: removed everything and closed).
  7. The empty state is an EmptyState with a centered primary Add filter, and the header action hides.
  8. The content search is unfocused on arrival, with drag-to-dismiss of a focused search.
  9. Fixed header and footer around a scroll viewport.
  10. Muted containers only with more than one section.
  11. Not done: dates. The value step keeps main's date editors in their sheet presentation, which read and write the UTC day, and tokens and rows show the token's date. The record keeps an implementation-gap note on FR11, FR12 and FR17 until the date-zone fixes land.
  12. The value step is purpose="info" (was form).

Where Core differs from Lab on purpose: the empty-result title wording stays in Core's catalog ("No filters match your search"); a nested-only field lists; the icon actions meet 44 px through a hit layer. Lab's Clear all says "Editable filters removed" every time; Core says it only while a read-only filter remains.

Tokenizer: the two list-value editors render the public Tokenizer with its experimental.presentation, now on main; nothing internal to the Tokenizer is imported.

Evidence

Desktop does not regress. The popover presentation, adaptive on a mouse or a wide screen, and an absent experimental all behave as on main:

  • Parity harness (not committed): it renders the popover presentation on main (692aa2647, this head's base) and on this head and records every DOM snapshot, the active element, and the onChange/onFocus/onBlur log at each step. It covers 9 prop sets, each with no filters and with 15 filters spanning every value type (including a stale and a read-only filter), plus scripted typeahead, pick, Escape, the token edit popover, remove, Backspace, Clear all, blur, content-search free text and the nested editor. The variants are experimental absent, {presentation: 'popover'}, and {presentation: 'adaptive'} with matchMedia matching nothing. Each is compared with main's render without experimental.
    • Result, on this head: 96 of 96 snapshots identical (branch vs main (absent) diffs: []), and the spoken announcements are the same messages in the same order for every variant. main's self-variance between two runs is empty.
    • The run covers the presentation swap. Its focus hand-off lives in the roots' callback refs (each presentation keys its root, so a swap remounts it), not in Effects: the detaching root notes held focus, the attaching root gives it to its primary control, and the sheet root's attach hides an editor popover left open and preloads the flow's chunk. The popover's editing state returns to idle in the render that sees the swap.
  • Existing popover tests pass and none is changed or removed. PowerSearch.test.tsx gains one popover test (hasAutoFocus stays on the combobox across re-renders). PowerSearchEditPopover.test.tsx, PowerSearchValueEditor.test.tsx, PowerSearchFilterEditor.test.tsx, and the Tokenizer and BottomSheet suites are not edited.
  • Real Chromium, fine pointer: the Experimental adaptive story at 1280×800 and at 390×844 without touch renders the tokenizer combobox and no sheet trigger. A token click opens the editor popover. 166 elements at rest and 334 with the editor open, the same as before this change.

Touch flow: real Chromium, static Storybook builds of main (Lab) and this head (Core), a 390×844 iPhone touch profile. Each pair shows the same state, Lab on the left and Core on the right:

State Top dialog Open dialogs Lab vs Core
Field at rest with three filters none 0 Same tokens and trigger; date text differs (item 11)
Filters Filters 1 Same: Add filter icon button, rows with trailing remove, Clear all and Done at full width
Add filter from Filters Add filter 2 (Filters inert) Same: unfocused search, "General" and group headings, chevrons, a plus on the one-tap field
Value step, one operator "Priority is" 3 Same: the choice rows on the sheet surface, Cancel and Add filter
Value step, several operators Status 3 Same: Operator radios and the choice rows each in a muted section
Clear all confirmation "Clear all filters?" 2 Same: destructive "Clear filters" above "Keep filters"
Filters with no filter Filters 1 Same: "No filters applied" with one centered primary Add filter
Add filter entered directly Add filter 1 Same: Done in the fixed footer, no Back
Content search, query with no match Add filter 1 Same: the free-text suggestion lists
Field search, query with no match Add filter 2 Same compact empty state; Core keeps its catalog title "No filters match your search"
  • Lower sheets of the stack are inert and aria-hidden. Under the confirmation, Filters is blocked by the open modal.
  • The remaining visible differences are the date text (item 11) and story data: Core's story config has no field groups and other field names.

Element counts (DEC-3, document.body.querySelectorAll('*'), coarse pointer, read when settled, on this head): 123 at rest, 313 Filters, 314 with Add filter above it, 332 with a choice-row value step above both, 349 with a list-value step, 420 with the Tokenizer's sheet above that, 373 with an edit step above Filters, 333 with the Clear all confirmation. Fine pointer: 166 at rest, 334 with the editor popover open. The record states them.

Bundle: PowerSearch.js loads PowerSearchBottomSheet.js only through import(), so a field that does not opt in does not carry the flow (PR5).

  • Lazy graph imports Core directly (scripts/core-single-bundle.test.mjs). The value step reuses the popover's editors (FR17). The sheet gets PowerSearchValueEditor, NestedEditor and buildRelativeDateOptions from PowerSearch.tsx, whose graph already loads them for the popover, so the editors and the components they reach stay out of the lazily imported graph. The modules the sheet itself reaches import Core modules directly: TextInput, Field, RadioList, RadioListItem and useInputStatusIcon change only their import paths.
  • Touch-only cost (architecture:component-style-authoring INV6), flagged and justified. Importing PowerSearch costs about 10 KB minified (about 3 KB gzip) more than on main. That is over the 2 KB gzip budget. Per module, minified: PowerSearch.tsx +3.2 KB, the shared English catalog +2.4 KB, PowerSearchValueEditor.tsx +1.5 KB with its catalog +0.7 KB, formatTokenValue.ts +1.5 KB with its catalog +0.3 KB, focus-ring styles +0.5 KB.
    • The trigger and its styles must render synchronously: they are the server markup for bottom-sheet and the first client render on a compact touch screen for adaptive. Loading them on demand would show no field until the chunk arrives. The swap hand-off and the adaptive latch belong with them.
    • The English catalog is one shared module that every component's fallback reads; the sheet's strings are entries in it.
    • PowerSearchValueEditor and formatTokenValue are the popover's own modules, which the value step and the Filters rows reuse so one value reads and edits the same way in both presentations (FR11, FR12, FR17). Their sheet branches cannot move into the chunk without forking the editor.
    • Nothing here is a touch primitive the shared internal touch modules provide: the sheet's drag and confirmation modules are those shared modules, and they load only with the flow.

Writing rubric results

Run over this pull request's change to docs/specs/components/PowerSearch/PowerSearch.spec.md. The record text is main's. On top of it:

  • The implementation-gap notes are lifted from every sheet row except the date parts of FR11, FR12 and FR17, which now say what Core does instead.

  • The verification row no longer points at the Lab suite.

  • DEC-3 records Core's element counts in place of its implementation gap.

  • PowerSearchBottomSheet.test.tsx and PowerSearch.ssr.test.tsx join verified_by, with frontmatter links for claims that source comments used to cite.

  • R1 narration: none. The record describes the shipped behavior in the present tense.

  • R2 research in the record: none. Measurements and runs stay in this description.

  • R3 evidence naming a change: none.

  • R4 rejected alternatives: unchanged one-line entries in DEC-3.

  • R5 pull request or issue references: none.

  • R6 ruled questions still listing options: none.

Scope

  • One capability is added.
  • Unrelated fixes, cleanup, and policy changes were removed or split.
  • Consumer docs (PowerSearch.doc.mjs: the prop with stability: 'experimental', an example, and the zh and dense variants), four stories, focused tests and the [experimental] changeset ship with the feature.
  • Public text and artifacts contain no internal Meta context.

Testing

  • vitest, scoped:
    • packages/core/src/PowerSearch, internal, RadioList, TextInput, Field, hooks, Tokenizer, BottomSheet, Dialog, Tooltip, the derived-var registry test and Lab's MobilePowerSearch: 107 files, 2,788 tests passed on main 4f452f927; on 692aa2647 (this head's base), the PowerSearch, Dialog, internal, hooks, Tokenizer and BottomSheet suites: 84 files, 2,028 tests passed, and Core typecheck passes. PowerSearchBottomSheet.test.tsx has 169 tests.
    • scripts/core-single-bundle.test.mjs: 3 of 3 pass against a fresh Core build.
  • Core pnpm build and pnpm typecheck: exit 0.
  • ASTRYX_STRICT_LINT=1 eslint over every changed file: 0 errors. prettier --check over every changed file: clean.
  • pnpm check:repo (includes check:knowledge, check:changesets, check:i18n-runtime, check:i18n-catalog, check:sync): exit 0. node scripts/sync-exports.js --check: up to date.
  • pnpm check:knowledge: aligned.
  • git diff --check: clean.

@vercel

vercel Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 3, 2026 10:50am UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟡 Reviewed (for maintainers only) Nov 3, 2026, 11:00 AM UTC

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 23, 2026
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from b154a0f to c07a91a Compare October 23, 2026 19:53
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from 07d619c to 0efc86f Compare October 23, 2026 19:56
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from c07a91a to 36f8999 Compare October 23, 2026 20:03
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from 0efc86f to 7e07d10 Compare October 23, 2026 20:03
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from 36f8999 to a017ec1 Compare October 23, 2026 20:17
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from 7e07d10 to 8b9c8d8 Compare October 23, 2026 20:17
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from a017ec1 to 13206a2 Compare October 23, 2026 20:25
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from 8b9c8d8 to 11fb87a Compare October 23, 2026 20:26

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch flow for PowerSearch, but it cannot be approved yet: the head writes DEC-3 and FR10–FR21 into the current component:PowerSearch record, and an engineering owner still has to decide those requirements, including FR17/FR21's reliance on the Tokenizer sheet requirements from #7977 that are not on main yet. The approval on #8000 covers that pull request's head, not this one; an engineering owner records the decision here by commenting /approve-spec 11fb87a50dafe3d00fb7f54d4151c577eaa3ace9 on this pull request, and the author may do so as an engineering owner. Whatever that decision is, these fixes are needed:

  • Put data-testid on the sheet trigger (today it moves from the combobox input to a div), and drop the family:input-fields membership and FR10 citation, since that family's record excludes PowerSearch.
  • Show date-only values without a time of day in the Filters rows (the forced story shows Mar 9, 2024, 7:00 PM).
  • Align FR16 (focus after a value suggestion commits) and FR20 (the relative-date row labels) with the code, and declare the stable result-count fix in its own [fix] Changeset.
  • Split the 4,800-line change, for example by dropping the carried spec commit once #8000 lands and moving the stable fix out, or explain why it cannot be split.

[Automated review]

onClick={handleTriggerWrapperClick}
onFocus={handleTriggerFocusWithin}
onBlur={handleTriggerBlurWithin}
data-testid={testId}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the popover presentation data-testid reaches the combobox input (through Tokenizer); here it lands on the control-box div, so a test id changes element when an adaptive field switches. Putting it on the trigger <button> keeps it on the field's primary control in both presentations.

config,
operator.value,
filter.value,
Number.MAX_SAFE_INTEGER,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatFilterValue formats every date_absolute with hour and minute, so a date-only filter (isDateOnly: true) reads Mar 9, 2024, 7:00 PM in this row while its token says Mar 9, 2024. Could the row format date-only values without the time?

value: filterValue,
};
pendingSheetFocusRef.current = {
type: 'manager-filter',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a controlled parent this moves focus to the new filter's row after a value suggestion commits, while FR16 says a creation commit focuses Add filter. Either the code or FR16 should change so they agree.

]
modules: []
families: [family:overlay-dismissal]
families: [family:input-fields, family:overlay-dismissal]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

family:input-fields lists PowerSearch as excluded ("higher-level compositions that consume member behavior"), and its FR10 sends data-* to the primary semantic control, which this record's prop-target paragraph does not follow. Drop the membership and the FR10 citation, or amend that family first.

'@astryxdesign/core': patch
---

[experimental] PowerSearch gains a touch surface behind `experimental={{presentation: 'bottom-sheet' | 'adaptive'}}`. `bottom-sheet` makes the whole field one trigger showing display-only filter tokens, and moves filter editing into a flow of tall modal sheets: a Filters sheet listing the active filters as rows with Add filter, Done, and Clear all; an Add filter sheet with the fields grouped (searchable from eight fields), or the content search with the field typeahead's suggestions listed inline; and a value step that stages field → operator → value and confirms from its footer, with Remove while editing. Closed vocabularies are full-width rows, numbers stage as typed, and `string_list` / `entity_list` values edit through the Tokenizer's own sheet presentation titled with the field label. `adaptive` uses the sheet on compact coarse-pointer screens and the popover otherwise, resolved the same way Selector resolves it. The sheet flow loads on demand, so a PowerSearch that does not opt in does not carry it. New `@astryx.powersearch.sheet.*` catalog keys for the sheets' strings; no new theming target. The result-count announcement is now StrictMode-safe in both presentations (no announcement on mount). Anything inside PowerSearch's `experimental` prop may change or be removed in a patch release; omitting it keeps the stable behavior.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The StrictMode-safe result count changes the stable presentation too; spec:AST-017 FR22 keeps that out of an [experimental] entry, so it needs its own [fix] Changeset.

@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from 13206a2 to 6f02169 Compare October 23, 2026 21:25
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from 11fb87a to bf9ee1b Compare October 23, 2026 21:39
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from 6f02169 to 1dc081e Compare October 23, 2026 22:10
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from bf9ee1b to 3cbfa4c Compare October 23, 2026 22:12
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from 3cbfa4c to c3bbf19 Compare October 23, 2026 22:21
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from 1dc081e to c1d1852 Compare October 23, 2026 22:33
@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from c3bbf19 to fc16c1d Compare October 23, 2026 22:34

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch flow for PowerSearch, but it cannot be approved yet: the head writes DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6 into the current component:PowerSearch record, and an engineering owner has to decide those requirements, including FR17/FR21's reliance on the Tokenizer sheet requirements from #7977 that are not on main yet. An engineering owner records that decision by commenting /approve-spec followed by this head's full commit SHA on this pull request, and the author may do so as an engineering owner. Whatever that decision is, these still need changes:

  • Make a date-only filter show the same day on the trigger and in the Filters row; today the token formats in the device's time zone and the row in timezoneID.
  • Announce the sheets' empty-result messages as AR6 says, or drop that clause from AR6.
  • Split the 5,181-line change, for example by dropping the carried record change once #8000 lands and moving the stable result-count fix out, or explain why it cannot be split.

[Automated review]

operator.value.type === 'date_absolute' &&
filter.value.type === 'date_absolute' &&
operator.value.isDateOnly !== false
? formatDateAbsoluteCompact(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The row formats a date-only value in timezoneID, but the trigger's token (PowerSearch.tsx:430) formats the same instant with no time zone, so it follows the device. With timezoneID America/Los_Angeles on a Tokyo device, the forced story's token says Mar 10, 2024 while this row says Mar 9, 2024. Formatting both the same way (here, or in the token as its own [fix]) keeps one filter on one day, and the boundary test could assert the token too.

"Search filters"; every sheet action is at least 44px tall.
- **AR6 — Announcements and focus.** While a sheet is open, every announcement
the flow makes — `Added {label}` for a committed suggestion, the result count
of FR19, and the sheets' empty-result messages — is delivered by a persistent

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing announces the empty-result messages: "No filters match your search" and "No results found" render as plain Text with no live region (PowerSearchBottomSheet.tsx:1320–1334). Either deliver them through the flow's polite region or drop this clause from AR6.

@imdreamrunner
imdreamrunner force-pushed the feat/powersearch-experimental-bottom-sheet branch from fc16c1d to 5e77abd Compare October 23, 2026 23:19
@imdreamrunner
imdreamrunner force-pushed the feat/tokenizer-experimental-bottom-sheet branch from c1d1852 to 86731bf Compare October 23, 2026 23:33

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request is on hold for a spec decision: the proposed component:PowerSearch record (DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6, with FR13's keyboard exception and DEC-3's surface-budget exception) and the sheet trigger's departure from family:input-fields FR10 need an engineering owner to approve them or name the requirements to change. An engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; FR17 and FR21 also depend on the Tokenizer record in #7977. Two changes are needed either way:

  • hasAutoFocus now applies on the first mount only (PowerSearch.tsx:611-616), which removes data-autofocus from the stable field after its first re-render, so a PowerSearch inside a Dialog that opens later no longer takes focus; keep it as passed until a presentation swap actually happens, and add a test with a Dialog that opens after mount.
  • The change is 8,668 authored lines; split out the separable pieces (the carried spec commit, the stable StrictMode fix, the formatter extraction) or land the size exception under current authority.

[Automated review]

Comment on lines +611 to +616
const hasMountedRef = useRef(false);
const isFirstMount = !hasMountedRef.current;
useEffect(() => {
hasMountedRef.current = true;
}, []);
const autoFocusOnMount = hasAutoFocus && isFirstMount;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autoFocusOnMount comes from a ref an Effect flips after the first commit, so it is false on every later render — including in the stable popover presentation, where it reaches the Tokenizer's hasAutoFocus and removes data-autofocus from the combobox. An Astryx Dialog keeps its children mounted while closed and focuses only [data-autofocus] after showModal(), so a <PowerSearch hasAutoFocus> in a Dialog that opens later no longer takes focus over the controls before it (in Chromium, with a button before the field: base focuses the combobox, head leaves focus on the button). Track the swap in state adjusted during render, as wasDisabled is, and keep hasAutoFocus constant until a swap actually happens.

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch presentation to PowerSearch, and approval waits on a spec decision: the head writes DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6 into the current component:PowerSearch record and keeps a recorded departure from family:input-fields FR10, so an engineering owner needs to approve or amend those requirements by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) and decide whether FR10 is amended for this trigger or the trigger stops composing Field. FR17 and FR21 also depend on the Tokenizer requirements in #7977, which are not on main yet. Independently of that decision:

  • Derive the swap-render hasAutoFocus rule from state adjusted during render instead of the Effect-written renderedPresentationRef (PowerSearch.tsx:617-622, react-component-runtime INV4 and INV3).
  • Show "No filters match your search" only for a non-empty query, as FR18 says (PowerSearchBottomSheet.tsx:1852).
  • Split the 8,697-line change: land the record through its own spec pull request and move the StrictMode fix and the formatTokenValue.ts extraction into their own pull requests.

[Automated review]

Comment on lines +617 to +622
const renderedPresentationRef = useRef(isSheetPresentation);
const isSwapRender = renderedPresentationRef.current !== isSheetPresentation;
useEffect(() => {
renderedPresentationRef.current = isSheetPresentation;
}, [isSheetPresentation]);
const autoFocusOnMount = hasAutoFocus && !isSwapRender;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads renderedPresentationRef, written by an Effect, during render to decide the trigger's autoFocus and the Tokenizer's hasAutoFocus. architecture:react-component-runtime INV4 says a ref mutation must not be the sole source for a value later read to produce rendered output, and INV3 asks a new Effect to name the external system it synchronizes. Deriving the swap from state adjusted during render (as wasDisabled is), for example a hasSwapped flag, keeps the intent without the ref.

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch flow for PowerSearch, and approval waits on an engineering owner: the head writes DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6 (with two INV7 owner exceptions) into the current component:PowerSearch record, keeps a recorded departure from family:input-fields FR10, and hosts the steps in BottomSheetSwitcher, which main now deprecates, so an engineering owner needs to approve or amend those requirements by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) and decide the FR10 and switcher questions. Independently of that decision, these need the author:

  • Split the 8,720-line change: let the record land with #8000, move the StrictMode fix, the formatTokenValue.ts extraction and the story decorator out, and stage the flow.
  • Describe the stacked list-value sheet in DEC-3 and AV5 as an owner exception beyond the 5-element open-layer allowance, and drop "as it displays it" from AR6.
  • Show "No filters match your search" only for a non-empty query, and keep a target for the content search's aria-controls when nothing matches.
  • Add the writing-rubric results to the description and update the passages that still describe removed behavior.

[Automated review]

list value (its Tokenizer trigger), and 401 with the Tokenizer's list-value
sheet open above the step (FR21) — every single-sheet state stays under the
open popover's 334; the stacked state does not, and is recorded under the
open-layer allowance (`spec:AST-017/FR60`, `DEC-29`) as this record's INV7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-017 FR60 and DEC-29 allow an open layer at most 5 extra elements under touch ("Anything beyond this needs an owner exception with before and after counts"). By this record's own counts the stacked state is 67 over the open popover (401 against 334), so it is an owner exception beyond the allowance rather than something recorded under it; could DEC-3 and AV5 say so?

for a filter removed from the value step (the label is the Filters row's
text — the same "{label} {value}" catalog message the row renders, its
summaries and relative-date values resolved from the catalog as FR12
states; each presentation announces a removed filter as it displays it: the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The popover presentation announces the Tokenizer item's label, which formatFilterValue builds (PowerSearch.tsx:841: a date with its time of day in timezoneID, a number with its units), not the token's visible text, and the first known gap below says the two differ in four places. "As it displays it" holds for the sheet only; could this say the popover announces its item label?

// root styling props is a stable API change for existing callers, so it waits
// for its own decision rather than riding the experimental option
// (`component:PowerSearch` Open questions).
const PENDING_FR10: ReadonlySet<string> = new Set<string>(['PowerSearch']);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps the family coverage test green while the trigger composes <Field> with ref on the root and root xstyle / className / style in the props type, which current family:input-fields FR10 does not allow for any component that composes Field. The exemption should follow an engineering owner's decision on FR10 rather than precede it.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch flow for PowerSearch, and approval waits on an engineering owner: the head writes DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6 (with two INV7 owner exceptions) into the current component:PowerSearch record and keeps a recorded departure from family:input-fields FR10, so an engineering owner needs to approve or amend those requirements by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) and decide the FR10 question. Independently of that decision, these need the author:

  • Split the 8,800-line change: let the record land with #8000, move the StrictMode fix and the formatTokenValue.ts extraction out, and stage the flow.
  • Update the description and the test file header where they still say nested-only fields are left out and nested rows are plain, and where the i18n list names the removed editFilter and selectedChoice keys.

[Automated review]

* sheet, Add filter, the operator step, each value type (list values
* through the Tokenizer's sheet), content search, focus after each step,
* disabled while open, the StrictMode-safe result count, onChange metadata,
* nested exclusion

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header still lists "nested exclusion", which this head removed: nested-only fields now list in Add filter and their steps use the built-in nested editor. The description has the same outdated passages.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch flow for PowerSearch, and approval waits on an engineering owner. The head writes DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6, with two INV7 owner exceptions, into the current component:PowerSearch record, and keeps recorded departures from family:input-fields FR10 and from the deprecated BottomSheetSwitcher. An engineering owner needs to approve or amend those requirements by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision, these need the author:

  • Order Add filter's field groups with the ungrouped fields first, as FR13 says and the popover's field list already does (PowerSearchBottomSheet.tsx:1012-1023), and add a test whose first field is grouped.
  • Split the 8,800-line change: let the record land with #8000, move the StrictMode fix and the formatTokenValue.ts extraction into their own pull requests, and stage the flow.
  • Update the description where its Evidence list still names a nested-only exclusion test and where its catalog-key counts disagree with the 24 keys en.json adds.

[Automated review]

Comment on lines +1012 to +1023
const groupedFields = useMemo(() => {
const groups = new Map<string, PowerSearchField[]>();
for (const field of matchingFields) {
const key = field.group ?? '';
const existing = groups.get(key);
if (existing) {
existing.push(field);
} else {
groups.set(key, [field]);
}
}
return [...groups.entries()];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add filter's groups follow insertion order here, so a config whose first field is grouped lists that group above the ungrouped fields. FR13 at this head says "ungrouped first", and the popover's field list already orders them that way through groupItems(items, {ungroupedFirst: true}) in usePowerSearchSource.ts. Reusing that ordering (or putting the '' group first) keeps the two presentations of one config consistent; a test with a grouped first field would cover it.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an opt-in touch flow for PowerSearch, and approval waits on an engineering owner: the head writes DEC-3, FR10–FR21, AR4–AR6, PR5 and AV5–AV6 into the current component:PowerSearch record, with two INV7 owner exceptions and a recorded departure from family:input-fields FR10, so an engineering owner needs to approve or amend them by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) and decide whether FR10 is amended for this trigger or the trigger stops composing Field. The previous review's Add filter group-order finding is fixed, and FR10 now records the layer runtime's missing presentation hold. Independently of that decision, the 8,856-line change still needs splitting: let the record land through #8000, and move the StrictMode fix and the formatTokenValue.ts extraction into their own pull requests.

[Automated review]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The touch sheet flow now matches the component:PowerSearch DEC-3 contract that merged on main, and its tests, typecheck and browser checks hold up; the remaining blocker is size. At 8,480 authored lines the change needs to be broken up, and parts of it are separable: please land the StrictMode result-count [fix] (with its test and Changeset) and the formatTokenValue.ts extraction ahead of the flow, then put the sheet presentation up as a stack that lands together, for example the trigger, latch and Filters sheet, then Add filter with the content search, then the value step with list and nested values.

[Automated review]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The touch sheet flow now matches the component:PowerSearch DEC-3 contract on main, with standalone step sheets, and its tests, typecheck and browser checks hold up; the remaining blocker is size. At 8,521 authored lines the change needs to be broken up: please land the StrictMode result-count [fix] (with its test and Changeset) and the formatTokenValue.ts extraction ahead of the flow, then put the sheet presentation up as a stack that lands together. While the record is being edited, please also refresh the AV5 and DEC-3 element counts, which were measured with the switcher host.

[Automated review]

@@ -218,8 +216,7 @@ FR10–FR21 govern.
standalone BottomSheets driven by one step state — a step that closes in the
update another opens is replaced by it in place (`component:BottomSheet`
FR10), the pattern `component:BottomSheetSwitcher` DEP-0015 names as the
deprecated switcher's replacement — or, until that replace is shipped, in a
BottomSheetSwitcher; it may compose List, RadioList, TextInput, Button, Heading, and Text in any
deprecated switcher's replacement; it may compose List, RadioList, TextInput, Button, Heading, and Text in any

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the switcher allowance gone, the counts a few lines below (296 Filters, 296 Add filter, 318, 328 and 401 stacked) and DEC-3's "adds 73 above it" still describe the switcher host. On this head Chromium measures 290, 290, 307, 322 and 393 (+71) on the Experimental Adaptive story at 390 × 844, so every relation holds; please re-measure and update the figures in this upkeep commit, as DEC-3 asks ("The counts are re-measured when either surface changes").

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The touch sheet flow follows the current component:PowerSearch record and holds up in the tests, the typecheck and a phone-sized Chromium run, but two things need changes before it can be approved. At 8,555 authored lines it needs splitting: please land the StrictMode result-count [fix] (with its test and Changeset) and the formatTokenValue.ts extraction first, then put the sheet presentation up as a stack that lands together. Separately, the Filters rows are keyed by position (PowerSearchBottomSheet.tsx:1519), so when the parent removes a filter above the focused row, focus jumps to the next filter or the focused row silently shows a different one; keying rows by filter identity, with a test, fixes it.

[Automated review]

'@astryxdesign/core': patch
---

[fix] PowerSearch no longer announces its initial `resultCount` on mount under React StrictMode. The result-count live-region effect guarded its first run with a mounted flag that StrictMode's effect replay reset, so the count present on mount was spoken unprompted; the guard now compares against the last announced text, so only a change after mount is announced in every render mode.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This [fix] and its popover StrictMode test change the stable presentation and stand on their own (the record says it "ships as a [fix] patch release of its own"); landing them ahead of the sheet flow is the first step of the split this review asks for.

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 1920c3f

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
PowerSearch · Storybook 🔴 190.9 kB (+6.1 kB) · 21.5 kB CSS (+964 B) 6,730 (+2,724) — · — · —
lab/MobilePowerSearch 215.9 kB (+1.2 kB) · 1.2 kB CSS (0) — — · — · —
lab/TransferList 74.8 kB (+411 B) · 1.7 kB CSS (0) — — · — · —
33 more components
Component Size LoC Visual · A11y · RTL
richtext/RichTextEditor 278.1 kB (+407 B) · 2.6 kB CSS (0) — — · — · —
lab/ListInput 81.9 kB (+402 B) · 1.6 kB CSS (0) — — · — · —
lab/EmojiPicker 70.6 kB (+401 B) · 722 B CSS (0) — — · — · —
lab/Comments 233.3 kB (+400 B) · 2.5 kB CSS (0) — — · — · —
lab/MobileTokenizer 101.6 kB (+397 B) · 880 B CSS (0) — — · — · —
richtext/RichTextEditorToolbar 223.0 kB (+392 B) · 454 B CSS (0) — — · — · —
lab/TransferListSelector 81.2 kB (+385 B) · 1.9 kB CSS (0) — — · — · —
lab/CommentComposer 103.1 kB (+384 B) · 1.9 kB CSS (0) — — · — · —
lab/CommentThread 227.9 kB (+367 B) · 2.4 kB CSS (0) — — · — · —
DateTimeInput 159.2 kB (+122 B) · 17.0 kB CSS (0) — — · — · —
DateInput 103.4 kB (+109 B) · 15.3 kB CSS (0) — — · — · —
lab/ChatEmojiPicker 65.2 kB (+46 B) · 773 B CSS (0) — — · — · —
lab/ChatReactionBar 65.7 kB (+44 B) · 1.1 kB CSS (0) — — · — · —
MultiSelector 73.1 kB (+40 B) · 15.2 kB CSS (0) — — · — · —
CheckboxList 43.4 kB (+39 B) · 8.6 kB CSS (0) — — · — · —
ComplexSelector 55.6 kB (+33 B) · 11.1 kB CSS (0) — — · — · —
InputGroup 42.1 kB (+25 B) · 8.4 kB CSS (0) — — · — · —
DateRangeInput 69.9 kB (+24 B) · 11.8 kB CSS (0) — — · — · —
NumberInput 55.0 kB (+24 B) · 11.2 kB CSS (0) — — · — · —
Selector 77.5 kB (+17 B) · 15.6 kB CSS (0) — — · — · —
FileInput 53.7 kB (+14 B) · 10.7 kB CSS (0) — — · — · —
TimeInput 90.1 kB (+8 B) · 14.3 kB CSS (0) — — · — · —
Markdown 145.3 kB (+1 B) · 18.3 kB CSS (0) — — · — · —
FieldLabel 36.0 kB (0) · 5.9 kB CSS (0) — — · — · —
Tokenizer 70.6 kB (0) · 17.1 kB CSS (0) — — · — · —
Typeahead 63.1 kB (0) · 11.8 kB CSS (0) — — · — · —
Pagination 86.6 kB (−1 B) · 16.0 kB CSS (0) — — · — · —
RadioListItem 51.3 kB (−9 B) · 10.1 kB CSS (0) — — · — · —
TextInput · Storybook 52.8 kB (−17 B) · 10.9 kB CSS (0) 420 (+1) — · — · —
Slider 88.9 kB (−21 B) · 9.4 kB CSS (0) — — · — · —
TextArea 47.2 kB (−21 B) · 9.8 kB CSS (0) — — · — · —
Field 39.9 kB (−1.6 kB) · 7.4 kB CSS (−955 B) — — · — · —
RadioList 41.3 kB (−1.6 kB) · 7.4 kB CSS (−956 B) — — · — · —

Package size: @astryxdesign/core 6.17 MB packed (+70.9 kB); no new dependencies.

API changes: core +1 · ~0 · −0

Committed API report behind this build: core 50 rows; run pnpm api:report.

1 API change (advisory)
Package Where Signature Usage
+ core . · Components · component PowerSearch prop experimental?: { presentation?: 'popover' | 'bottom-sheet' | 'adaptive' } @experimental <PowerSearch experimental="popover" />

Wide change (shared styles: packages/core/src/theme/derivedVarRegistry.test.ts): add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The touch sheet flow follows the current component:PowerSearch DEC-3 contract, but two things need changes before it can be approved. At 8,512 authored lines it needs splitting: land the formatTokenValue.ts extraction on its own, then put the sheet flow up as a stack that lands together (for example the trigger, latch and Filters sheet; Add filter with the content search; the value step), each with its tests. Separately, the Filters rows are keyed by position (PowerSearchBottomSheet.tsx:1519), so when the parent reorders the filters or removes one above the focused row, focus moves to a different filter; keying rows by filter identity, with tests for both cases, fixes it.

[Automated review]

filter,
index,
isEditable,
key: `${index}-${filter.field}-${filter.operator}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keying rows by ${index}-${filter.field}-${filter.operator} remounts the focused row when the parent reorders the filters or removes one above it, and the recovery effect then focuses the row at the old index: with focus on "Author is Ada" in [Status, Author, Priority], dropping Status lands focus on "Priority is P1". Keying each row by filter identity (a per-object id from a WeakMap, falling back to filterSignature for clones), with tests for a reorder and a removal above the focused row, keeps focus on the filter the person was on.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommending approval: the touch sheet flow matches the current component:PowerSearch contract (DEC-3, FR10–FR21), the stable popover presentation is unchanged, and the focused tests, typecheck and a phone-sized Chromium run all hold up. The size is accepted here because the record defines the sheet presentation as one complete flow, so it cannot land in parts without shipping an option that breaks its own requirements. The two earlier blockers are resolved: the StrictMode result-count fix is already on main, and Filters rows are now keyed by filter identity, with tests. What remains is the repository's spec-owner approval for the small record update.

[Automated review]

@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec f8c4a15

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. The touch flow follows the current component:PowerSearch contract (DEC-3, FR10–FR21), and the PowerSearch and Tokenizer tests, the Core typecheck and lint all pass on this head. Three things need fixing first:

  • Move the presentation-swap focus hand-off in PowerSearch.tsx (the per-commit useLayoutEffect at line 651 and the swap Effects at lines 678 and 793) into callback refs, as Tokenizer.tsx now does with handOffFocusRef and wrapperCallbackRef (architecture:react-component-runtime INV2, INV6).
  • Replace formatSharedList in formatFilterValue.ts with Core's shared useListFormatter, which already owns the same narrow-conjunction Intl.ListFormat policy, and delete the copy.
  • Importing PowerSearch now loads about 3.1 KB more gzip JavaScript for the touch presentation, over the 2 KB touch-only budget in architecture:component-style-authoring INV6, so either move that code into the lazy chunk or the shared touch modules, or justify the cost in the description.

The size is fine as one implementation of DEC-3, and the record edits are owner-approved on this head.

[Automated review]

// commit of the closed field, so the swap below knows the input was focused
// when `adaptive` resolves to the sheet on a touch screen right after
// hydration.
useLayoutEffect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This layout effect reads document.activeElement after every commit of every PowerSearch, and the Effect at line 678 then moves focus when the presentation flips. On main, Tokenizer runs the same hand-off from callback refs (handOffFocusRef and wrapperCallbackRef in Tokenizer.tsx). The detaching control notes focus while it is still in the document, which covers the pre-hydration autofocus case, and the attaching control takes it. Giving PowerSearch's root wrapper, trigger and Tokenizer handle that pattern removes both Effects (architecture:react-component-runtime INV2, INV6).

* type runs Chinese values together and gives Japanese and Korean a bare
* space).
*/
export function formatSharedList(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packages/core/src/i18n/useListFormatter.ts already builds this exact narrow-conjunction Intl.ListFormat and keeps raw list formatting in one place (architecture:internationalization INV8). Passing useListFormatter()'s function from the sheet into formatTokenValueText lets this copy go.

* Whether the tree root offers the group-operator picker when the field has
* several operators. The popover shows it; the sheet presentation's value
* step owns the operator choice in its own Operator group and hides it
*. @default true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: a stray *. is left here from a removed citation. @default true reads best on its own JSDoc line (the same leftover appears at powerSearchSheetUtils.ts:167 and :194).

@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec 796dd9d

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. The record edits for the stacked touch flow are owner-approved on this head, so what remains is implementation and evidence work:

  • Keep Core renderable from a single esbuild bundle: the lazily imported sheet graph now reaches modules that import Core barrels, so scripts/core-single-bundle.test.mjs fails and an app that bundles Core without code splitting throws before rendering List and Item; import those modules directly instead.
  • Make the other two failing tests pass: document or restructure the new --_power-search-sheet-hit-* custom properties that fail derivedVarRegistry.test.ts, and rebase onto main so the Clear all confirmation test drives a full backdrop press, since a bare click no longer dismisses an info Dialog there.
  • FR15a relies on component:BottomSheet FR10's same-update replace when a direct-entry creation commits from the value step, but two sheets close on that path, which FR10 excludes, so amend FR15a (or close Add filter in an earlier update) and assert focus after the sheets settle.
  • Move the presentation-swap focus hand-off in PowerSearch.tsx (the per-commit useLayoutEffect and the swap Effects) into callback refs, as Tokenizer.tsx does with handOffFocusRef and wrapperCallbackRef.
  • Announce and place focus after Clear all's confirmation in its action handler, as TokenizerBottomSheet.tsx does in closeClearConfirmation, instead of an Effect that waits for the dialog to close.
  • Replace formatSharedList in formatFilterValue.ts with Core's shared useListFormatter and delete the copy.
  • Importing PowerSearch now loads about 3.1 KB more gzip JavaScript that only the sheet presentation runs, over the 2 KB touch-only budget, so move it into the lazy chunk or the shared touch modules, or justify the cost in the description.
  • Rewrite the description: it still says the flow has no new behavior, that DEC-3 is unchanged and that every existing test passes, while this head stacks the steps, adds a confirmed Clear all, reverses two rejected alternatives and has three failing test cases.

A few comments in PowerSearchBottomSheet.tsx and the prop's JSDoc still describe the replaced flow; worth refreshing while you are there.

[Automated review]

// carry the media condition because StyleX allows plain values inside a
// pseudo-element. On a fine pointer the overlay is not generated, so it
// never takes hover from the action's own content.
touchHitArea: {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--_power-search-sheet-hit-inset and --_power-search-sheet-hit-content fail derivedVarRegistry.test.ts ("PowerSearch: all source vars are in doc file"): document them in PowerSearch.doc.mjs or reach the coarse-pointer hit area without new custom properties.

| FR13 | Add filter — from the Filters sheet's Add filter action, or directly when an interactive field with no filter opens — is a tall modal sheet named "Add filter" opened above the Filters sheet, which stays open beneath it (FR15a), so the sheet has no Back action: dismissing it — Escape, a scrim tap, a swipe — reveals Filters again, or closes the flow when Add filter was the entry point, in which case its fixed footer holds Done. Its header — the heading and, when one shows, the search field — stays fixed while the rows scroll. Without a content-search field it lists the visible fields grouped by `group`, each group a named list: the ungrouped fields first, under the heading "General", then each group under its name in order of first appearance, each a row button with the field's icon, a trailing chevron — or, for a field whose only operator takes an `empty` value, a trailing plus, since choosing it adds the filter at once (FR15) — and, once the field count reaches eight, a "Search filters" field above the list that matches labels and `typeaheadAliases` case-insensitively, caps a non-empty query's matches at `maxSearchResults`, and shows the empty state "No filters match your search" when nothing matches. With a `contentSearchFieldKey` whose default operator takes a `string` value it shows instead a "Search" searchbox using `placeholder`, with the same suggestions the popover presentation's typeahead offers listed inline as row buttons ("Search results"): fields, field-and-operator pairs, value matches, and a leading free-text item for the content field; Enter commits the first suggestion, ArrowDown and ArrowUp move between the search and the rows — arrow movement across independently focusable controls, recorded here as this record's exception to `spec:AST-020/FR15`, which adopts arrow keys for a single composite only: the searchbox-to-list continuity is what the popover presentation's combobox gives the same suggestions, and the rows stay in the Tab order as well — an IME-composing Enter does nothing, and a query with no suggestion shows the empty state "No results found" — the searchbox's `aria-controls` names the results list, or that message while nothing lists, so it always names an element; the field list's "No filters match your search" likewise shows only for a non-empty query that matches nothing, never for a config with no field. Each of the two is a compact empty state with a search icon, that title, and the description "Try a different search.", mounted only while the Add filter sheet is the open one. Neither search field takes focus when the sheet opens: the sheet opens with focus on its panel and the software keyboard stays down until the person taps the search; a direct vertical touch drag on the rows while a search field has focus blurs it, so the keyboard leaves and the rows can be read. A suggestion that carries a value commits `onChange([...filters, filter], 'add', filters.length)`, announces `Added {label}` inside the Filters sheet (AR6), and shows Filters as FR15a states; any other suggestion or field row opens the value step with that field and operator, above Add filter. Every visible field lists, a field whose every operator is `nested` included: the value step edits every operator's value (FR14). | DEC-3; Lab touch-flow evidence; current typeahead source | Current contract; verified by focused sheet tests and real-engine touch evidence. |
| FR14 | The value step is a tall modal sheet that edits every operator's value, opened above the sheet that led to it — Filters for an edit, Add filter for a creation — which stays open beneath it (FR15a); its title is the dialog's name (AR5), and for a filter whose field or operator the config no longer has it reads as the Filters row does (FR12): the field's label when the config still has the field, the raw key for whatever is gone. Its header and footer stay fixed while the body scrolls. When the field offers more than one operator the body starts with an "Operator" radio group whose options read "{field label} {operator label}" and list the field's operators, with the operator the step opened with checked — the filter's operator while editing, and while creating the operator the suggestion named (FR13) or else the field's default operator, else its first; otherwise there is no group and the title is "{field label} {operator label}" naming that operator. When no operator label applies the title is the field label. The body below the group is the editor for the checked operator's value type: the sheet-native controls of FR17, or for a `nested` value the popover presentation's built-in nested editor — the sub-filter tree with its Add action, each sub-filter's field, operator, and value controls, and removal — rendered as the popover renders it, with one difference: the step's Operator group takes the place of the editor's root group-operator picker, which is not shown, and the tree root reads the checked operator's label; a sub-filter's controls are the popover editor's own, in their default presentation (a Selector's menu, the Tokenizer's inline field), not the sheet's choice rows (FR17) or Tokenizer sheet (FR21), which govern the step's top-level value only, because the sheet passes no `experimental` option down to them; the tree is its own width container, so below 400 CSS px its rows wrap instead of overflowing the sheet, as they wrap in a narrow popover. A group is created and edited the same way in both presentations; switching the operator swaps the body. A value chosen in the body is staged, not committed: no `onChange` fires until the footer confirms it. Changing the operator keeps a staged value only when the new operator reads the same value type; an `empty` operator stages `{type: 'empty'}`. The body's sections sit on the sheet's surface when the editor is the body's only section; when the body has more than one section — the Operator group above the editor — each section is a muted container, so the grouping reads without a frame around a lone editor. The footer holds Cancel and the confirm action — `popoverSaveButtonLabel` when given, otherwise "Add filter" while creating and "Apply" while editing — disabled until the staged value is complete; the step has no Remove action: a filter is removed from its Filters row (FR12). When `components[type].Editor` is set for the checked operator's value type it replaces the body below the Operator group and the footer — the group stays outside it, so switching operators still swaps the body — receiving the props the popover presentation hands it, with the same meaning: the caller's `config`, the partial `filter`, `mode`, `saveButtonLabel`, `isReadOnly`, `onSave`, and `onCancel`, and nothing the popover does not pass; `onSave(filter)` commits it, `onSave(null)` removes the filter while editing and returns to Filters unchanged while creating. A controlled update that removes the field or operator being edited replaces the body with "This filter is no longer available", announced from its rendered element inside the sheet while the step is the open one (AR6), and keeps Cancel as the way out, the filter's trailing remove action on its Filters row being the removal path (FR12); when focus was on a control of the replaced body it moves to Cancel, so it never falls to the page body inside the open sheet (FR16). | DEC-3; Lab touch-flow evidence | Current contract; verified by focused sheet tests and real-engine touch evidence. |
| FR15 | Confirming a created filter commits `onChange([...filters, filter], 'add', filters.length)`; choosing a field whose only operator takes an `empty` value commits that filter the same way from the Add filter row, with no value step. Tapping an editable Filters row opens the value step on that filter in edit mode; Apply commits `onChange(updated, 'edit', index)`; the row's trailing remove action, or a tap on an `empty`-valued filter's row, commits `onChange(rest, 'remove', index)`, where `index` is the filter's position at commit time: a controlled parent that reorders or clones its filters while the step is open is followed by object identity, then by a structurally equal filter in the original slot, then by a unique structural match; when the target is gone or ambiguous nothing is committed and the step closes back to Filters, focus landing as FR16 states for a row that no longer exists. Every commit shows the Filters sheet as FR15a states. While the field is read-only, or disabled, no commit happens. | DEC-3; Lab touch-flow evidence | Current contract; verified by focused sheet tests and real-engine touch evidence. |
| FR15a | The steps stack: each step opens above the sheet that led to it, which stays open — receded and blocked as a covered sheet of the stack (`component:BottomSheet` FR5, FR6) — so the stack is Filters alone; Filters under Add filter; Filters under the value step for an edit; Filters under Add filter under the value step for a creation; or, when Add filter was the entry point, Add filter alone and then Add filter under the value step. No sheet of the flow carries a Back action: a dismissal — Escape, a scrim tap, a swipe, a platform close request — reaches only the top sheet (`component:BottomSheet` FR7), closes it, and reveals the sheet beneath, which takes focus back as FR16 states; dismissing the bottom sheet of the stack closes the flow. Cancel on the value step is that dismissal. A commit closes the step that made it: an edit's commit reveals Filters beneath the value step; a creation's commit, from the value step or from an Add filter row or suggestion that commits directly, closes Add filter, and the value step above it, together and shows Filters — revealed when it is open beneath them, else opened in the same update that closes them, BottomSheet's same-update replace (`component:BottomSheet` FR10), so the person sees the filter just added. The flow therefore has one direction forward (open a step above) and one back (close the top step): no dismissal replaces a step, and a creation's commit from a direct entry is the one place a sheet opens as others close. | DEC-3; `component:BottomSheet` FR5–FR9 | Current contract; verified by focused sheet tests and real-engine touch evidence. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a creation committed from the value step after a direct-entry Add filter, two sheets close (the value step and the covered Add filter) while Filters opens. component:BottomSheet FR10 replaces only when exactly one sheet closes and one opens, and keeps the ordinary push and pop motion and focus return otherwise, so this sentence promises a replace that does not apply on that path.

// commit of the closed field, so the swap below knows the input was focused
// when `adaptive` resolves to the sheet on a touch screen right after
// hydration.
useLayoutEffect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This layout effect reads document.activeElement after every commit of every PowerSearch, and the swap Effects below move focus when the presentation flips. Tokenizer.tsx does the same hand-off from callback refs (handOffFocusRef, wrapperCallbackRef); the same shape here removes the per-commit read (architecture:react-component-runtime INV2, INV6).

// Focus then moves to the sheet's first remaining control: a kept
// read-only row has none, so Add filter. The Effect synchronizes the live
// region and the document's focus, and acquires nothing.
useEffect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear all's announcement and focus are one event's work. TokenizerBottomSheet.tsx closeClearConfirmation does it in the handler: flushSync the close, then focus and announce. handleClearAll can do the same, with no Effect waiting on isClearConfirmationOpen.

* type runs Chinese values together and gives Japanese and Korean a bare
* space).
*/
export function formatSharedList(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the narrow-conjunction Intl.ListFormat that i18n/useListFormatter.ts owns ("Raw Intl.ListFormat construction stays centralized in this hook"). Pass useListFormatter()'s function into formatTokenValueText and delete this copy.

openSheet();
fireEvent.click(within(sheet()).getByRole('button', {name: 'Clear all'}));
// A scrim press dismisses the confirmation.
fireEvent.click(clearConfirmation());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the merge with main this click no longer dismisses the confirmation: main dismisses an info Dialog only on a press that starts and ends on the backdrop (family:overlay-dismissal FR10), and CI's test-ui (3/4) fails at line 1620. Drive a full pointer press on the backdrop after rebasing.

@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec fd60b07

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. The stacked touch flow matches the component:PowerSearch record edits an engineering owner approved on this head, and most earlier findings are fixed; two things remain:

  • CI lint fails because Prettier reformats the dependency array at PowerSearch.tsx:1402–1407; running pnpm exec prettier --write packages/core/src/PowerSearch/PowerSearch.tsx fixes it.
  • The popover reset on a presentation swap still runs from an Effect at PowerSearch.tsx:784–790; reset the state during render as autoFocusState does and hide the layer from the detaching root's callback-ref cleanup (:696–703), so the Effect can go.

Two small follow-ups are optional: DEC-3 records 333 elements with Filters open, while the head reads 313 because the Clear all confirmation mounts only when it opens, and a few new comments carry stray *. lines or a stale "offers Remove".

[Automated review]

Comment on lines +1402 to +1407
[
announce,
handleFieldFocus,
isSheetOpen,
isSheetPresented,
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI lint fails here: Prettier puts this dependency array on one line ([announce, handleFieldFocus, isSheetOpen, isSheetPresented]). Run pnpm exec prettier --write packages/core/src/PowerSearch/PowerSearch.tsx.

Comment on lines +784 to +790
useEffect(() => {
if (isSheetPresentation && wasPopoverOpenRef.current) {
setPopoverStateRaw({type: 'idle'});
popover.hide();
}
wasPopoverOpenRef.current = popoverState.type !== 'idle';
}, [isSheetPresentation, popover, popoverState]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect resets popover state and hides the layer on the presentation swap, and copies popoverState into wasPopoverOpenRef after every commit of every PowerSearch. The swap is the popover root detaching, and its callback-ref cleanup at :696–703 already runs exactly then. Record wasPopoverOpenRef in setPopoverState, then do the reset and popover.hide() in that cleanup, or reset during render with the previous-value pattern autoFocusState uses. The Effect can then go (architecture:react-component-runtime INV2, INV6).

@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec 63c355f

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: one shared-path fix remains. An engineering owner approved this head's record edits, and both earlier blockers (the Prettier failure and the popover-reset Effect) are fixed.

  • ChoiceRows in PowerSearchBottomSheet.tsx (lines 599–634) hand-rolls arrow, Home and End roving focus for the enum and date_relative radio rows, which Core already owns in useInternalListFocus (as SegmentedControl uses it): call the shared hook, check the row that takes focus from inside the group, and delete the local handler and manual tabIndex.

Non-blocking: a stray *. before @default true in PowerSearchEditPopover.tsx, and the SheetConfirmation and useDismissSearchOnTouchDrag headers name consumers that do not use them.

[Automated review]

Comment on lines +602 to +634
const handleRadioKeyDown = (
event: React.KeyboardEvent<HTMLButtonElement>,
index: number,
) => {
const nextIndexFor = (key: string): number | null => {
switch (key) {
case 'ArrowDown':
case 'ArrowRight':
return (index + 1) % items.length;
case 'ArrowUp':
case 'ArrowLeft':
return (index - 1 + items.length) % items.length;
case 'Home':
return 0;
case 'End':
return items.length - 1;
default:
return null;
}
};
const next = nextIndexFor(event.key);
if (next === null) {
return;
}
event.preventDefault();
const rows =
groupRef.current?.querySelectorAll<HTMLButtonElement>('[role="radio"]');
rows?.[next]?.focus();
if (!isDisabled) {
const target = items[next];
onToggle(target.value, selected.has(target.value));
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hand-rolls the roving radio focus (arrows, Home, End, one tab stop) that Core already owns in useInternalListFocus (hooks/useListFocus.ts); SegmentedControl uses it for its radio group with selection following focus. Calling the shared hook here, and checking the row that takes focus from inside the group, keeps one path for fixes such as its tab-stop repair and right-to-left key mapping; the local handler and the manual tabIndex can then go.

@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec de2e652

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: two implementation fixes remain. An engineering owner approved this head's record edits, and ChoiceRows now uses the shared list focus.

  • The content-search searchbox and suggestion rows in PowerSearchBottomSheet.tsx (lines 2174–2179 and 2226–2251) move focus with hand-written ArrowUp/ArrowDown handlers, a list-focus concern Core already owns in useInternalListFocus, which this component already calls for its choice rows: drive the searchbox and the rows through one useInternalListFocus call (extending the hook if it cannot take the searchbox) and delete the local handlers.
  • The chunk preload in PowerSearch.tsx (lines 614–618) runs in an Effect, but it only fires when the sheet root attaches, and handOffFocusRef already runs on that attach: call loadPowerSearchBottomSheet() there when isSheetRoot is true and remove the Effect.

Non-blocking: the SheetConfirmation and useDismissSearchOnTouchDrag headers name consumers that do not use them, and the hasAutoFocus comment above autoFocusState could be cut to its reason.

[Automated review]

label={item.label}
isDisabled={!isInteractive}
onClick={() => handleContentSuggestionSelect(item)}
onKeyDown={event => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hand-rolls ArrowUp/ArrowDown focus across the suggestion rows (and the searchbox's ArrowDown branch at line 2174 does the same), while this component already calls useInternalListFocus for ChoiceRows. Core owns linear list focus there (architecture:react-component-runtime INV16). Could one useInternalListFocus call cover the searchbox and the rows (orientation: 'vertical', wrap: false, no roving tab index so the rows stay in the Tab order), with Enter kept in the searchbox handler?

isSheetOpen || isSheetPresented || resolvedPresentation === 'bottom-sheet';
// Preload the flow's chunk while the sheet presentation is active, so the
// first press opens without waiting for the network; nothing renders.
useEffect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isSheetPresentation turns true only when the key="sheet" root attaches, and handOffFocusRef (line 761) already runs on that attach and computes isSheetRoot. Calling void loadPowerSearchBottomSheet() there when isSheetRoot is true starts the preload at attach and removes this Effect; the call is idempotent.

* @input Dialog, Layout, Heading, Text, Button; caller-owned copy
* @output SheetConfirmation — the confirmation a touch sheet opens over
* itself for Clear all and for discarding a changed draft
* @position Core-internal, shared by the Tokenizer sheet and the PowerSearch

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: on main the Tokenizer sheet confirms with AlertDialog (TokenizerBottomSheet.tsx:937), and nothing here discards a draft. Suggest: "Core-internal; the PowerSearch sheets use it for Clear all. Not a public export."

* @input Caller-owned search inputs, a focus target, and touch events from a
* separate results list
* @output useDismissSearchOnTouchDrag — touch handlers for the results list
* @position Core-internal, shared by the touch sheets; not a public export

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: only PowerSearch's Add filter sheet calls this. Suggest: "Core-internal; PowerSearch's Add filter sheet uses it. Not a public export."

}
}, [isSheetPresentation]);
const triggerRef = useRef<HTMLButtonElement>(null);
// `hasAutoFocus` passes through unchanged — the popover presentation hands

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (architecture:knowledge-contracts INV29): 14 lines with sentences over 25 words. Suggest: "A swap remounts the field's control, and autoFocus would take focus from elsewhere on the page. Only the first control keeps autoFocus. The roots' callback refs hand over focus the field held."

@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec 3d1186d

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: two fixes remain. An engineering owner approved this head's record edits, and both findings from the previous round are fixed (the content search now uses the shared list focus, and the chunk preload runs in the sheet root's callback ref).

  • PowerSearchBottomSheet.tsx decides whether Filters stays open beneath Add filter, and whether Add filter shows Done, from fieldPickerOriginRef.current read during render (lines 1215 and 2332) and written during render by entryStep (lines 930–938), which architecture:react-component-runtime INV4 does not allow: keep the origin in state beside step and derive isFieldPickerDirectEntry from it.
  • The sheet's Clear all announces "All filters removed" from a new @astryx.powersearch.sheet.filtersCleared key (line 1637) that duplicates the existing @astryx.powersearch.allFiltersRemoved the popover uses, so locales and caller overrides can word one outcome two ways: use allFiltersRemoved and delete the new key.

Non-blocking: the handOffFocusRef comment in PowerSearch.tsx (lines 728–743) could be cut to its reason in a few short sentences.

[Automated review]


// The steps stack: each opens above the sheet that led to it, which stays
// open beneath. Add filter entered directly has no Filters sheet beneath.
const isFieldPickerDirectEntry = fieldPickerOriginRef.current === 'trigger';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads fieldPickerOriginRef.current during render to decide whether Filters stays open beneath Add filter (and, at line 2332, whether Add filter shows Done), and entryStep writes the ref during render. architecture:react-component-runtime INV4: "A ref mutation MUST NOT be the sole source for a value later read to produce React-rendered output." Keep the origin in state beside step (set together in openFieldList and the open adjustment) and derive this from it.

} else {
announce(
changedCount == null
? t('@astryx.powersearch.sheet.filtersCleared')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@astryx.powersearch.sheet.filtersCleared has the same English and meaning as the existing @astryx.powersearch.allFiltersRemoved, which the popover's Clear all uses; with two keys, locales and caller overrides can word this outcome differently in the two presentations. Use t('@astryx.powersearch.allFiltersRemoved') here, as the line above reuses editableFiltersRemoved, and drop the new key from en.json.

…tion

`experimental={{presentation: 'bottom-sheet' | 'adaptive'}}` makes the whole
field one trigger with display-only filter tokens and moves filter editing
into a stack of tall modal sheets, the touch flow the Lab prototype ships:

- Each step opens above the one that led to it, which stays open beneath;
  a dismissal closes only the top sheet and reveals the step beneath, so no
  sheet has a Back action.
- Filters lists the filters as rows that edit a filter, each with a trailing
  remove button (a one-tap filter's row removes it), under a fixed header
  with Add filter and over a fixed footer with Clear all and Done. With no
  filter it is one empty state with a centered Add filter.
- Clear all asks for confirmation, removes the editable filters, keeps
  read-only ones, leaves Filters open and announces there.
- Add filter lists the grouped fields, or the content search with inline
  suggestions, and leaves its search unfocused; a vertical touch drag on the
  rows moves focus off a focused search. A field whose only operator takes
  no value adds at once.
- The value step stages operator → value and confirms from its footer. A lone
  editor sits on the sheet surface; beside the Operator group each section is
  a muted container.

The flow loads on demand; `adaptive` resolves through the shared
compact-touch policy and stays latched until the flow's exit has returned
focus. Value editors are Core's own in a sheet presentation; list values edit
through the Tokenizer's sheet titled with the field label; nested values use
the built-in nested editor. Announcements made while a sheet is open go
through the open dialog's own live region. The confirmation and the
search-drag handling are the internal modules the Tokenizer sheet shares.

Implements `component:PowerSearch` DEC-3 (FR10–FR21, FR15a, AR4–AR6, PR5)
and lifts the record's implementation-gap notes, except the sheet's date
handling in `timezoneID` (FR11, FR12, FR17).
@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec 1920c3f

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: one fix remains. An engineering owner approved this head's record edits, the touch sheet flow follows component:PowerSearch FR10–FR21, and both fixes the previous review asked for are in (the Add filter origin is now state, and Clear all speaks the shared "All filters removed" message).

  • The new useLayoutEffect at PowerSearchBottomSheet.tsx:997–1001 clears the pending focus target when the flow closes without naming an external system (architecture:react-component-runtime INV2, INV3); clear the target in closeFlow and in the step Effect at :1144 when step is null, delete the Effect, and add a test that reopens the flow with a stale target pending.

Non-blocking: several new source comments run past INV29's sentence length, and the comment at PowerSearchBottomSheet.tsx:1823–1826 still says the Operator group leaves nested operators out.

[Automated review]


// A focus target left pending when the flow closed never carries into the
// next open.
useLayoutEffect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect does the flow's closing work and synchronizes no external system (architecture:react-component-runtime INV3: "A new or changed Effect MUST name the external system it synchronizes"). Every close already reaches code that can drop the target: closeFlow (:1003) for Done and the bottom sheet's dismissal, and the step Effect at :1144, which runs in the commit where any close sets step to null, the disabled close included. Suggest deleting this Effect, clearing pendingSheetFocusRef in closeFlow, and in the step Effect: if (step == null) { pendingSheetFocusRef.current = null; return; }. A test that reopens the flow with a target pending would pin it.

draftOperator == null ? '' : resolveOperatorLabel(draftOperator, t);
// The title names the filter's own operator when the step offers no
// Operator group — one editable operator, or an operator the group does not
// list (a `nested` one with no Editor override) — so a nested filter's step

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: getVisibleOperators returns every operator, so the Operator group lists nested operators too, and "a nested one with no Editor override" no longer happens. Suggest: "The title names the operator when the step shows no Operator group."

This branch was successfully deployed

1 active deployment
Preview — 1920c3f4 Deployed Nov 3, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant