Skip to content

feat(lab): MobilePowerSearch gains the Core PowerSearch sheet's behavior - #8740

Merged
imdreamrunner merged 1 commit into
mainfrom
feat/lab-mobile-powersearch-core-parity
Oct 27, 2026
Merged

imdreamrunner merged 1 commit into
mainfrom
feat/lab-mobile-powersearch-core-parity

Conversation

@imdreamrunner

@imdreamrunner imdreamrunner commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

feat(lab): MobilePowerSearch takes the Core sheet flow's behaviors

Lab only: packages/lab/src/MobilePowerSearch/ and its tests and doc. Nothing in @astryxdesign/core changes. One commit on main.

Governing record

component:PowerSearch (packages/core/src/PowerSearch/PowerSearch.spec.md, current, merged in #8000) — DEC-3 and its FR10–FR21, AR4–AR6 — specifies the touch sheet flow: the trigger, the Filters, Add filter and value steps, focus after each step and after a controlled update, the in-sheet announcement route, Clear all's and the date editors' semantics. Lab's MobilePowerSearch is the prototype that implements that flow outside the stable component so its UX can iterate; this PR brings the prototype up to the record's current behavior where Lab can reach it with main's public API, and lists the remaining gaps below. Lab has no record of its own; its .doc.mjs is the consumer surface.

User need

The touch PowerSearch flow is about to get a UX pass, and that pass should start from the best version of the flow. Two versions exist: Lab's MobilePowerSearch, the prototype the flow was designed in, and the Core PowerSearch sheet presentation (its experimental.presentation option), which went through many review rounds and gained behaviors the prototype never had — announcements delivered where assistive technology can hear them, one rule for where focus goes when a controlled update removes the control it was on, rows that keep their element across a parent's reorder, stale filters that can still be removed, a field that cannot keep a modal flow open once it is disabled. Why does the gap matter? The owner's pass will be made against Lab, so every behavior Lab lacks is one the pass either re-derives or loses. Why not point the pass at Core instead? Core's sheet is behind an experimental option on the stable component and carries its own review constraints; Lab is where the design iterates. Why not wait for graduation? The pass is now.

This PR ports what Lab lacked, aligns the semantics the two components deliberately decided differently (listed below for veto), and names what cannot be ported without a Core change.

Problem and solution fit

Each delta ports one behavior of the Core sheet into Lab, with a test that fails on main and passes here:

  • Announcements inside the open sheet via useAnnounce. Lab kept a hand-rolled polite region in each sheet (a requestAnimationFrame clear-then-write with a two-second timer). main has the modal-aware useAnnounce (fix(core): useAnnounce delivers inside the open Astryx modal that encloses its caller #8009): called from inside a sheet it delivers into that sheet's dialog. A SheetAnnouncer rendered in each step publishes the dialog's announce; messages queued by a step change (an added suggestion, a removal) are spoken in the commit that shows the next step, so they land in that dialog rather than the one being replaced. The hand-rolled region, its frame and timer are gone. Tests: result count change and "Added …" land in the dialog's own live region, not the page body; two messages in one commit are joined.
  • Empty-result and unavailable messages spoken from their rendered element, once per occasion (a query that matches nothing, a filter that became unavailable), through a SheetEmptyMessage that reads its own text and announces it via useAnnounce from inside the sheet. "No filters match your search" shows only for a non-empty query — a config with no field lists nothing — and the content searchbox's aria-controls always names an element (the list, the message, or an empty results region). Tests for each.
  • The StrictMode-safe result count — Lab already compared against the last announced text; unchanged, covered by the existing StrictMode test. Added: a count that changes while the flow is closing (Clear all's) waits for the trigger focus with the after-close message.
  • One focus re-home rule over the button rows: where focus was in the Filters or Add filter sheet is recorded on every focus change (sheet + index among button rows); a layout effect on every commit of an open step checks whether focus is still inside the sheet's dialog and, when a controlled update unmounted the focused control, moves it to the nearest remaining button row by that index (following, else last), else the sheet's first control, else the panel. Plain read-only rows are passed over. Tests: removed focused row with a read-only row above → the row that followed; last row removed → the new last; Clear all gone with the last filter → Add filter; a dropped field's row → a remaining row.
  • Rows and tokens keyed by filter identity (createFilterIdentities in mobilePowerSearchUtils.ts: WeakMap id per object, a clone takes the id of the previous update's filter with the same signature, equal filters told apart by order). A parent's reorder or a removal above the focused row moves elements instead of remounting them. Tests: reorder with the same objects and with clones keeps focus on the same element; removal above keeps focus, not the old index.
  • Apply and Cancel after a controlled update removed the edited filter or the opening field: Apply writes nothing and focuses the nearest remaining button row around where the filter was, else Add filter; Cancel from creation with the opening field gone lands on the Add filter sheet's first control (search field, else first row, else Back); the content-search target has the same fallback. Tests for both.
  • The unavailable state with focus on Remove: when a controlled config update removes the field or operator being edited, the body becomes "This filter is no longer available" and focus moves to Remove (Cancel when there is none) instead of falling to the page body; the message is announced from its element. Test.
  • Stale filters listed by raw keys: a filter whose field or operator the config no longer knows was dropped from the trigger and the Filters sheet, so only Clear all could remove it. It now shows on the trigger as "legacy: was", lists as a row "legacy was" (the field's label when the field remains: "Author was"), opens the unavailable step titled the same way (the dialog's name), and Remove takes it out. Tests.
  • Date rows and tokens formatted as the Core token shows them (formatDateDay: year, abbreviated month, day, Gregorian calendar, the browser's zone, no time of day — formatDateAbsoluteCompact's text), so a Lab token or row and a Core token read byte for byte the same — see the semantic alignment on dates.
  • Removed {label} after Remove, through the catalog's @astryx.tokenizer.tokenRemoved with the row's text, spoken in the Filters dialog; Clear all's after-close announcement ("All filters removed", or "All filters removed. {count}" when the count changed in the same commit) spoken on the page once focus is back on the trigger, since the closing dialog is modal through its exit. Tests.
  • Disabled closes the flow, and focus comes back to the trigger: a field that becomes disabled while a step is open closes the flow in the render that sees the change and does not reopen it when re-enabled (Lab used to keep the sheet open with its actions disabled). The trigger is never natively disabled — aria-disabled only, as Core's trigger (spec:AST-085 FR1/FR2); openManager refuses while disabled — so the closing flow hands focus back to it instead of dropping it on the page body, and a disabled reason can be read from it. Tests: the flow closes and the trigger holds focus (fails without the change); a disabled trigger is focusable and does not open.
  • endContent outside the trigger: the end lane (result count and endContent) was already outside the trigger button, but a tap on it opened the flow through the wrapper; it now keeps its own taps. Test: a button in endContent fires and the flow stays closed; a tap on a token still opens it.
  • onFocus/onBlur counts: one onFocus on the trigger, one onBlur when the flow opens, one onFocus when it closes — Lab already behaved; guarded by a test.
  • Catalog strings: the Core sheet's @astryx.powersearch.sheet.* keys are not on main, so Lab keeps its literals (a Lab delta); it uses the existing @astryx.tokenizer.tokenRemoved and tokenAdded and @astryx.powersearch.resultCount.

Semantic alignments — component:PowerSearch DEC-3's deliberate differences, ported so Lab becomes the shipped behavior. Each is one line to veto:

  1. Clear all removes read-only filters too, reported as onChange([], 'remove', lastIndex), and shows whenever a filter exists (Lab kept read-only filters and reported the first removed index). Test.
  2. Date editors read and write the UTC day, as the Core popover's editor does: a date_absolute is the UTC midnight of the chosen day, a date_range likewise (Lab converted through timezoneID). Test: picking March 9 stores Date.UTC(2024, 2, 9) / 1000.
  3. isDateOnly: false gets the date-only editor (Lab rendered a DateTimeInput). Test.
  4. Tokens and rows show a date_absolute exactly as the Core token does (component:PowerSearch FR11, FR12): the day in the browser's zone, no time of day — the same text Core's formatTokenValueText produces — while the editors keep the UTC day (alignment 2), as Core's do. Core's record carries the zone difference between its token and its editor as a known gap; Lab mirrors Core rather than fixing it on its own. Test (it.each over Tokyo, Honolulu, unset): the token's and the row's text equal the Core token formatter's output for the instant, the editor reads March 10.
  5. timezoneID no longer changes a date value; the prop stays for API parity with Core, and its JSDoc, the file header and the doc say so.

Remaining gaps — cannot be ported without a Core change

  • nested values: Core's sheet edits them through the popover's built-in NestedEditor, exported only on the Core branch; Lab still leaves nested-only fields out with its development warning.
  • PowerSearchValueEditor's presentation branches (choice rows as radios/checkboxes, the list-value Tokenizer's sheet presentation wired through Core's editor): Lab keeps its own MobilePowerSearchValueEditor.
  • useAnnounceRenderedText is not exported from @astryxdesign/core/hooks; Lab's SheetEmptyMessage is a local equivalent (reads its rendered text, announces once per occasion through useAnnounce).
  • Sheet catalog strings (@astryx.powersearch.sheet.*) are Core-branch only; Lab keeps literals.
  • Token/row number and list formatting parity (formatTokenValueText, locale digits without grouping, Intl.ListFormat conjunction/narrow) lives in Core's formatTokenValue.ts; Lab's rows and tokens both use Core's public formatFilterValue, so they agree with each other but not byte-for-byte with Core's token.
  • adaptive presentation and its latch: Lab has no pointer switching by design, so nothing to port.

Current specification

component:PowerSearch (packages/core/src/PowerSearch/PowerSearch.spec.md, current) DEC-3, FR10–FR21, AR4–AR6 describe the sheet flow these behaviors come from; Lab is a prototype with no record of its own and the .doc.mjs is its consumer surface. component:BottomSheet FR5–FR10 own the stacked, replacing step sheets Lab already uses. spec:AST-086 FR14 is the announcement route useAnnounce implements.

Public delta and caller burden

Props unchanged. Observable: Clear all's result and reporting (above); dates read as the UTC day and the date-only editor for isDateOnly: false; stale filters now visible and removable; announcements inside the sheets through the shared hook (the mobile-power-search-sheet-status test id is gone with the hand-rolled region); the field becoming disabled closes an open flow; a tap on endContent no longer opens the flow. Doc: hasClear, timezoneID and the usage paragraph updated.

Evidence

  • Lab tests: MobilePowerSearch 114 and MobileTokenizer 10 pass. The 27 MobilePowerSearch tests that cover ported or aligned behavior fail on main with the test file alone (checked by stashing the source files): 13 rewritten for the new semantics (in-sheet regions, Clear all ×3, disabled closes with focus on the trigger, disabled trigger focusable, add announcements ×2, date-only editor, Core-token date text ×3, UTC midnight) and 14 new under "Core parity" (focus re-home ×3, identity keys ×2, stale filters ×2, unavailable state, Apply/Cancel gone ×2, Removed, "No filters match" ×2, end lane). Two parity guards pass on main as well (aria-controls target, onFocus/onBlur counts).
  • Chromium (headless shell via Playwright, iPhone profile 390×844, static Storybook of the previous head, Lab/MobilePowerSearch touch-flow story): tokens and rows read the date as a day with no time of day (then the UTC day; now the Core token's zone handling — the same text wherever the browser's zone agrees with UTC on that instant); Filters → first row → Remove: "Removed Status is any of Open, In Progress" in the Filters dialog's own live region, focus on "Priority is P1 - High"; Add filter → Back: focus on Add filter; Clear all: flow closed, tokens gone, "All filters removed" in the page region with focus on the trigger; one scrim in every state; no page errors.

Scope

  • One capability is added: Core-parity behavior for Lab's touch flow, in Lab only.
  • Unrelated fixes, cleanup, and policy changes were removed or split: Core is untouched; the semantic alignments are listed above for veto rather than slipped in.
  • Consumer docs (MobilePowerSearch.doc.mjs), focused tests (26 before/after) ship with the feature; no migration — props unchanged.
  • Public text and artifacts contain no internal Meta context.

Testing

  • pnpm exec vitest run packages/lab/src/MobilePowerSearch packages/lab/src/MobileTokenizer — 2 files, 124 tests pass; on main's sources with the MobilePowerSearch test file, 27 fail.
  • pnpm -F @astryxdesign/lab typecheck:docs — exit 0 (after pnpm -F @astryxdesign/core build and pnpm -F @astryxdesign/lab build, which the fresh worktree needed for its type resolution).
  • pnpm -F @astryxdesign/storybook typecheck — exit 0 (after pnpm build).
  • pnpm check:repo — exit 0; strict lint on packages/lab/src/MobilePowerSearch/ — 0 findings; prettier and git diff --check clean.
  • Real browser: the Chromium run above.

@vercel

vercel Bot commented Oct 27, 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 Oct 27, 2026 9:25pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟡 Reviewed (for maintainers only) Oct 27, 2026, 9:51 PM UTC

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Previews ready: Storybook and Sandbox were built with this PR's docsite. CI evidence will be added when available.

View Storybook for this PR · View Sandbox for this PR

@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.

Approval is on hold until an engineering owner identifies the spec that governs Lab MobilePowerSearch. No current record covers it, so the Core-parity behavior and semantic alignments 1–5 need that decision: for example, confirming component:PowerSearch DEC-3, FR10–FR21 and AR4–AR6 as its contract and ruling on alignment 4, whose UTC-day tokens and rows differ from FR11/FR12's popover-token zone handling. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Two fixes are needed whatever is decided:

  • With no disabledMessage, disabling the field while a sheet is open drops focus to the page body because the trigger keeps the native disabled attribute (MobilePowerSearch.tsx line 1724), which keeping only aria-disabled would fix.
  • The timezoneID prop JSDoc (line 209) and file header (lines 42–45) still say it drives date formatting, contradicting MobilePowerSearch.doc.mjs.

[Automated review]

Comment on lines +787 to +793
const [wasDisabled, setWasDisabled] = useState(isDisabled);
if (isDisabled !== wasDisabled) {
setWasDisabled(isDisabled);
if (isDisabled && step != null) {
setStep(null);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When this closes the flow while isDisabled is set and no disabledMessage is given, each sheet returns focus to triggerRef (finalFocusRef), but the trigger then carries the native disabled attribute (disabled={isDisabled && !showsDisabledMessage}, line 1724), so focus falls to <body>. Dropping native disabled from the trigger and keeping aria-disabled would fix it; openManager already returns early while disabled. The disabled-close test could also assert expect(trigger()).toHaveFocus() after settleSheets().

Comment on lines 42 to 45
* - `components[type].Token` overrides receive Core's PowerSearchTokenProps,
* which carry no `timezoneID`; the built-in token renders Core `Token`
* directly so the configured timezone reaches tokens and row summaries
* through the same formatter;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since this change, date tokens and rows read the UTC day and timezoneID no longer reaches them. This header line and the prop's JSDoc at line 209 ("Timezone ID for date formatting in tokens, row summaries, and date editors") now contradict MobilePowerSearch.doc.mjs.

@imdreamrunner
imdreamrunner force-pushed the feat/lab-mobile-powersearch-core-parity branch from f9c3b11 to d3b66c9 Compare October 27, 2026 20:25
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 27, 2026

@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.

Approval is on hold until an engineering owner identifies the spec that governs Lab MobilePowerSearch: no current record covers it, so the Core-parity behavior and semantic alignments 1–5 need that decision, for example confirming component:PowerSearch DEC-3, FR10–FR21 and AR4–AR6 as its contract, including the date gap that record lists as known and alignments 2 and 4 now bring into Lab (in a zone west of UTC, a picked March 10 shows as "Mar 9, 2024" on the token and row). An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. The two fixes from the previous round are in and nothing else blocks; two optional cleanups:

  • The comment above the result-count Effect (MobilePowerSearch.tsx lines 1497–1500) still describes the frame, clear timer and teardown Effect this change removed.
  • The timezone test builds its expected text in the runner's zone, so on a UTC runner it would not catch tokens going back to the UTC day; pinning a non-UTC zone and asserting literal text would.

[Automated review]

@@ -1194,9 +1504,17 @@ export function MobilePowerSearch({
return;

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 comment above this Effect (lines 1497–1500) still says announce "starts a frame and a clear timer, which the sheet announcer's teardown Effect and the next write release". This change removed that frame, timer and teardown, so the sentence now misstates the Effect's resources; pointing it at the useAnnounce route is enough.

(timezoneID, compact, long) => {
// The text the Core PowerSearch token shows for the instant: the day in
// the browser's zone, no time of day (`formatDateAbsoluteCompact`).
const coreTokenText = new Intl.DateTimeFormat('en-US', {

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 builds the expected text with formatDateDay's own Intl.DateTimeFormat options in the runner's zone, and it.each varies only the timezoneID prop, so on a UTC runner the test also passes if formatDateDay formats in UTC. Pinning a non-UTC zone for this block (at America/Los_Angeles this instant is Mar 9) and asserting the literal text would make it fail on that regression.

Ports what the Core PowerSearch sheet presentation gained into Lab's
MobilePowerSearch so the touch flow's UX pass starts from the best version:
announcements delivered inside the open sheet by useAnnounce, empty-result and
unavailable messages spoken from their rendered element, one focus re-home rule
over the button rows, rows and tokens keyed by filter identity, Apply and Cancel
after a controlled update removed the edited filter or the opening field, the
unavailable state with focus on Remove, stale filters listed by their raw keys
with the step titled the same way, Removed and Clear all announcements, the
field becoming disabled closing the flow, and endContent keeping its own taps.

Semantic alignments with Core (DEC-3): Clear all removes every filter, read-only
ones included, as one remove at the last index; date editors read and write the
UTC day; isDateOnly: false gets the date-only editor; dates read as the UTC day
on tokens and rows.
@imdreamrunner
imdreamrunner force-pushed the feat/lab-mobile-powersearch-core-parity branch from d3b66c9 to 3259f54 Compare October 27, 2026 21:09
@imdreamrunner

Copy link
Copy Markdown
Contributor Author

/approve-spec 3259f54

@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 Core-parity direction is settled by the engineering owner's /approve-spec on this head and the ported behavior checks out, but two of the new in-sheet announcements are spoken from sheets that are no longer open. Gate SheetEmptyMessage on its step being the open one, reset its spoken occasion when hidden, and add a regression test for each path:

  • Leaving Add filter (Back, Escape, the scrim) after typing a query that has suggestions announces "No results found", because contentResults is emptied whenever step !== 'fields' while the closed sheet's body stays mounted (MobilePowerSearch.tsx lines 817 and 1993–1997).
  • Once the value step has closed, a config update that drops the last edited field or operator announces "This filter is no longer available" with no filter open, and reopening the same stale filter does not announce it again (lines 2144–2147).

[Automated review]

// the empty-result message carries the results id.
<SheetEmptyMessage
id={contentResultsId}
message="No results found"

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 message also mounts in the closed Add filter sheet: contentResults is [] whenever step !== 'fields' (line 817) and BottomSheet keeps a closed sheet's body mounted, so leaving with Back or Escape after a query that listed suggestions speaks "No results found" — on the page, or in the Filters dialog after Back. FR18 limits it to a query whose result set is empty. Rendering it only while step === 'fields' (the empty results div keeps aria-controls valid), or gating SheetEmptyMessage with an isShown input, would fix it.

</Text>
</div>
<SheetEmptyMessage
message="This filter is no longer available"

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 draft outlives its step and this message has no step guard: after the flow closes, a config update that drops the last edited field or operator speaks "This filter is no longer available" on the page, and reopening the same stale filter speaks nothing because the occasion is unchanged. FR14 ties the message to the filter being edited, inside the sheet; an isShown={step === 'value'} gate that resets the spoken occasion when hidden covers both cases.

@imdreamrunner
imdreamrunner merged commit 7681363 into main Oct 27, 2026
65 of 67 checks passed
@github-actions
github-actions Bot deleted the feat/lab-mobile-powersearch-core-parity branch October 28, 2026 06:17

This branch was successfully deployed

1 active deployment
Preview — 3259f548 Deployed Oct 27, 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant