diff --git a/.changeset/typeahead-tokenizer-busy-indicator.md b/.changeset/typeahead-tokenizer-busy-indicator.md new file mode 100644 index 0000000000000..b512f9de44b66 --- /dev/null +++ b/.changeset/typeahead-tokenizer-busy-indicator.md @@ -0,0 +1,15 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] Typeahead, Tokenizer: the busy indicator is a Spinner in the field's end lane, and the input keeps its text out from under it (#5555) + +Three defects in one block. The indicator a search painted was `` — a static glyph, in a family where every other input paints busy with a `Spinner`, and where `clock` otherwise means _time_. It was an in-flow item at the row's inline end, which is where each field independently parks its clear button, so the two landed on each other: 17×20px of overlap in Typeahead and 19×20px in Tokenizer, the latter leaving part of the ✕ unclickable. And the combobox never carried `aria-busy`, unlike every sibling input. + +The base engine now reports the busy state to the field, which paints it in the one inline-end lane it already owns beside its clear button and end content, and sets `aria-busy` on the input. A caller using `BaseTypeahead` directly is unaffected: it still renders its own visible, named "Loading" status, now a Spinner rather than the clock. + +The lane is absolutely positioned — these wrappers wrap, and an in-flow sibling gets pushed onto a second row by a token — so it reserved no space, and at a narrow width the query ran underneath it. The input now reserves the lane's measured width, which also closes a pre-existing case of the same bug: at 280px with a value selected, the clear button covered 17px of the live query in Typeahead and 25px in Tokenizer before this change, with no spinner involved at all. Both are 0 now. + +The measurement reaches CSS as a custom property written to the field wrapper, never as React state, so a lane that grows or shrinks repaints without re-rendering the field. Held in state it cost a second commit every time the lane changed size — once as the spinner arrived and once as it left — which doubled the field's commits across a search for a value no JavaScript reads. The observation is shared too, through the same `observeResize` singleton `useTruncation` uses, so a page of fields costs one callback per frame rather than one observer each. + +@freddymeta diff --git a/apps/storybook/stories/Tokenizer.stories.tsx b/apps/storybook/stories/Tokenizer.stories.tsx index 35c84871efab8..10d7ce7bbb728 100644 --- a/apps/storybook/stories/Tokenizer.stories.tsx +++ b/apps/storybook/stories/Tokenizer.stories.tsx @@ -25,6 +25,27 @@ const userSource: SearchSource = { bootstrap: () => users.slice(0, 5), }; +/** + * A remote source, near enough — the busy state only exists between the + * keystroke and the response, so a synchronous source never shows it. + */ +const slowUserSource: SearchSource = { + search: (query: string) => + new Promise(resolve => { + setTimeout( + () => + resolve( + users.filter(u => + u.label.toLowerCase().includes(query.toLowerCase()), + ), + ), + 1200, + ); + }), + bootstrap: () => + new Promise(resolve => setTimeout(() => resolve(users.slice(0, 5)), 1200)), +}; + const meta: Meta = { title: 'Core/Tokenizer', component: Tokenizer, @@ -454,3 +475,24 @@ export const StatusVariantComparison: Story = { ); }, }; + +export const Loading: Story = { + render: args => { + const [value, setValue] = useState([users[0]]); + return ( + setValue(items)} + hasClear + endContent={{value.length} selected} + /> + ); + }, + args: { + label: 'Team Members', + placeholder: 'Search people...', + }, + name: 'Loading (async source, with clear and end content)', +}; diff --git a/apps/storybook/stories/Typeahead.stories.tsx b/apps/storybook/stories/Typeahead.stories.tsx index 4acadf6d44a24..4fb2d2e5421e2 100644 --- a/apps/storybook/stories/Typeahead.stories.tsx +++ b/apps/storybook/stories/Typeahead.stories.tsx @@ -24,6 +24,28 @@ const fruitSource: SearchSource = { bootstrap: () => fruits.slice(0, 5), }; +/** + * A remote source, near enough. The busy state only exists between the + * keystroke and the response, so a synchronous source never shows it — which + * is why the indicator went unexercised long enough to ship as a clock. + */ +const slowFruitSource: SearchSource = { + search: (query: string) => + new Promise(resolve => { + setTimeout( + () => + resolve( + fruits.filter(f => + f.label.toLowerCase().includes(query.toLowerCase()), + ), + ), + 1200, + ); + }), + bootstrap: () => + new Promise(resolve => setTimeout(() => resolve(fruits.slice(0, 5)), 1200)), +}; + const meta: Meta = { title: 'Core/Typeahead', component: Typeahead, @@ -265,3 +287,23 @@ export const StatusVariantComparison: Story = { ); }, }; + +export const Loading: Story = { + render: () => { + const [value, setValue] = useState(null); + return ( +
+ +
+ ); + }, + name: 'Loading (async source)', +}; diff --git a/packages/core/src/Field/useEndLaneReserve.ts b/packages/core/src/Field/useEndLaneReserve.ts new file mode 100644 index 0000000000000..940ed0ffc7852 --- /dev/null +++ b/packages/core/src/Field/useEndLaneReserve.ts @@ -0,0 +1,109 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +'use client'; + +/** + * @file useEndLaneReserve.ts + * @input Uses React and the shared ResizeObserver + * @output Exports useEndLaneReserve, which keeps a field's input clear of the + * absolutely-positioned lane at its inline end + * @position Shared field internal. Used by Typeahead and Tokenizer, both of + * which park their clear button, end content and busy indicator in one + * absolutely-positioned lane at the field's inline end. + * + * SYNC: When modified, update this header and the two callers: + * - /packages/core/src/Typeahead/Typeahead.tsx + * - /packages/core/src/Tokenizer/Tokenizer.tsx + */ + +import {useCallback} from 'react'; +import * as stylex from '@stylexjs/stylex'; +import {observeResize, unobserveResize} from '../utils/sharedResizeObserver'; + +/** + * The measured lane width, published on the field wrapper and read by the + * input through inheritance. A custom property is the point of the design: it + * carries a measurement into CSS without carrying it through React, so a lane + * that grows or shrinks repaints without re-rendering the field. + */ +const LANE_WIDTH_VAR = '--_astryx-end-lane-width'; + +// Keep the input's text and caret out from under the lane. +// +// The input's content box already stops one wrapper padding short of the +// border, and the lane is inset from that same border — so what is left for +// the input to clear is the lane's inset plus its width, less the padding it +// already has, plus a padding's worth of gap so the text does not touch the +// glyph. The two paddings cancel, which is why this reads as inset + width. +// +// The width arrives as a variable rather than a number, so this rule is +// static: one class, generated once, never regenerated as the lane changes. +// The `0px` fallback covers the frame before the lane is first measured. +const reserveStyles = stylex.create({ + reserve: (laneInset: string) => ({ + paddingInlineEnd: `calc(${laneInset} + var(${LANE_WIDTH_VAR}, 0px))`, + }), +}); + +/** + * Keep a field's input clear of the lane at its inline end. + * + * The lane is absolutely positioned — it has to be, because these wrappers + * wrap, and an in-flow sibling gets pushed onto a second row by a token — and + * an out-of-flow box reserves no space by definition. There is no CSS that + * makes one do so: the input cannot see a sibling's width. So the lane is + * measured, and the input spends the measurement as padding. + * + * What the lane holds is not a fixed set: a clear button that comes and goes + * with the value, a busy indicator that comes and goes with the search, and, + * in Tokenizer, arbitrary `endContent`. Measuring covers all of it, including + * the combinations, and needs no constant kept in step with what renders. + * + * **The measurement never enters React state.** It is written to a custom + * property on the field wrapper and inherited by the input. Held in state, it + * cost the field a second commit every time the lane changed size — once when + * the spinner arrived and again when it left — doubling the field's renders + * across a search for a value no JavaScript ever reads. The observation is + * shared as well: `observeResize` batches every field on the page into one + * callback per frame instead of one observer each. + * + * Takes the lane's inset from the field's inline-end border, as the CSS + * expression that positions it. Returns a ref callback for the lane, and the + * style for the input — which the caller applies only when it renders a lane, + * something it already knows without measuring anything. + */ +export function useEndLaneReserve( + laneInset: string, +): [(node: HTMLElement | null) => void, stylex.StyleXStyles] { + const laneRef = useCallback((node: HTMLElement | null) => { + if (node == null) { + return; + } + // The lane's parent: the field wrapper in both callers, and an ancestor + // of the input either way, which is what inheritance needs. + const host = node.parentElement; + + observeResize(node, () => { + // Border box: the lane's own padding and border are part of what the + // input has to clear. Rounded up, because a fractional width left as-is + // reserves a hair too little and the glyph's last subpixel column still + // lands on the caret. + host?.style.setProperty( + LANE_WIDTH_VAR, + `${Math.ceil(node.getBoundingClientRect().width)}px`, + ); + }); + + // React 19 runs a ref callback's return value as its cleanup, so the + // observer is released exactly when the lane unmounts. + return () => { + unobserveResize(node); + // The room the lane claimed goes back to the input. Removing beats + // setting 0px: the rule's fallback is already that, and this leaves no + // stale property behind on the DOM. + host?.style.removeProperty(LANE_WIDTH_VAR); + }; + }, []); + + return [laneRef, reserveStyles.reserve(laneInset)]; +} diff --git a/packages/core/src/Tokenizer/Tokenizer.tsx b/packages/core/src/Tokenizer/Tokenizer.tsx index 9b2494aaf31b3..2cfc9eb658291 100644 --- a/packages/core/src/Tokenizer/Tokenizer.tsx +++ b/packages/core/src/Tokenizer/Tokenizer.tsx @@ -40,6 +40,8 @@ import { type FieldStatusVariant, } from '../Field'; import {Token} from '../Token'; +import {Spinner} from '../Spinner'; +import {useEndLaneReserve} from '../Field/useEndLaneReserve'; import {renderIconSlot, type IconType} from '../Icon'; import {OverflowList} from '../OverflowList'; import {useLayer} from '../Layer/useLayer'; @@ -232,6 +234,10 @@ export interface TokenizerProps extends Omit< // Styles // ============================================================================= +// How far the end lane sits from the field's inline-end border, named once +// because the input's reserve is derived from the same value. +const END_LANE_INSET = spacingVars['--spacing-2']; + const styles = stylex.create({ wrapper: { position: 'relative', @@ -267,7 +273,7 @@ const styles = stylex.create({ // Match the field's inline padding (inputWrapperStyles.base uses // spacing-2) so end content (clear button, resultCount) lines up with // the text/start-icon inset instead of hugging the border at ~3px. - insetInlineEnd: spacingVars['--spacing-2'], + insetInlineEnd: END_LANE_INSET, display: 'flex', alignItems: 'center', gap: spacingVars['--spacing-2'], @@ -461,6 +467,18 @@ export function Tokenizer({ })); // Focus-within state for overflow truncation + // Reported by BaseTypeahead so the indicator can live in this field's own + // end lane, beside endContent and the clear button. + const [isLoading, setIsLoading] = useState(false); + // What sits in the end lane varies most here — a spinner, arbitrary + // `endContent`, a clear button, or all three — so its width is measured + // rather than assumed, and the input reserves it. + const [laneRef, laneReserve] = useEndLaneReserve(END_LANE_INSET); + // Whether a lane renders at all — known from props and state, without + // measuring anything, which is why the reserve costs no extra commit. + const hasEndLane = Boolean( + isLoading || endContent || (hasClear && value.length > 0 && !isDisabled), + ); const [isFocusedWithin, setIsFocusedWithin] = useState(false); const isTruncated = !isFocusedWithin && tokenOverflowBehavior !== 'none' && value.length > 0; @@ -814,17 +832,21 @@ export function Tokenizer({ ariaDescribedBy={ariaDescribedBy} onChangeQuery={onChangeQuery} __queryEntries={createEntries} + __onLoadingChange={setIsLoading} debounceMs={debounceMs} onKeyDown={handleKeyDown} anchorRef={wrapperRef} size={size} - inputXStyle={ + inputXStyle={[ isAtMax || isTruncated ? styles.inputAtMax : value.length > 0 ? styles.inputCompact - : undefined - } + : undefined, + // Not for the collapsed states above: those give the input no width + // to pad, and `inputAtMax` zeroes its padding outright. + !(isAtMax || isTruncated) && hasEndLane && laneReserve, + ]} /> {htmlName != null && value.map(item => ( @@ -838,8 +860,13 @@ export function Tokenizer({ disabled={isDisabled} /> ))} - {(endContent || (hasClear && value.length > 0 && !isDisabled)) && ( -
+ {hasEndLane && ( +
+ {isLoading && ( + + )} {endContent} {hasClear && value.length > 0 && !isDisabled && ( extends Omit< */ __queryEntries?: (query: string, results: T[]) => T[]; + /** + * Called when a search starts or settles, so a wrapper can paint the busy + * state in the one inline-end lane it already owns for its clear button and + * end content, instead of the base rendering a second indicator competing + * for the same corner. + * + * Passing this takes the indicator over: the base stops rendering its own. + * A caller that does not pass it keeps the visible, named status the base + * has always rendered. + * + * Underscored and `@internal` for the same reason as `__queryEntries` + * above: `BaseTypeaheadProps` is re-exported from the package entry point, + * so anything named on it ships as public API at the next cut, and this is + * a wiring detail between the two wrappers and the base. + * + * @internal + */ + __onLoadingChange?: (isLoading: boolean) => void; + /** * Debounce delay in ms before triggering search after typing. * Set to 0 for synchronous/local search sources that don't need debouncing. @@ -310,10 +331,15 @@ const styles = stylex.create({ fontSize: typeScaleVars['--text-supporting-size'], color: colorVars['--color-text-secondary'], }, - loadingSpinner: { + // The indicator a direct caller gets. In flow, where it has always been, so + // it reserves its own width and the input's text never runs under it. + // Typeahead and Tokenizer take the indicator over and paint it in their own + // inline-end lane instead; this is what renders for everyone else. + loadingStatus: { display: 'inline-flex', alignItems: 'center', justifyContent: 'center', + flexShrink: 0, padding: spacingVars['--spacing-1'], }, }); @@ -389,6 +415,7 @@ export const BaseTypeahead = function BaseTypeahead({ onChangeQuery, onOpenChange, __queryEntries, + __onLoadingChange, inputId: externalInputId, ariaDescribedBy, ariaLabelledBy, @@ -424,6 +451,37 @@ export const BaseTypeahead = function BaseTypeahead({ const [isLoading, setIsLoading] = useState(false); const [hasSearched, setHasSearched] = useState(false); + // Report the busy state to a wrapper that has taken the indicator over. + // + // Through a ref, and at the call site rather than from an effect: an effect + // would run after this component had already committed, so the wrapper's + // own state change landed in a second commit — two renders of the whole + // field per transition, four across a search. Called here, the wrapper's + // setState batches with ours into the one commit that React was already + // doing. The ref keeps the identity of a caller's inline arrow from + // mattering, and the guard makes the report edge-triggered: the redundant + // `false` on every keystroke below the query threshold reports nothing. + // + // The ref is synced in a layout effect rather than during render, following + // `onMotionStartRef` in BottomSheetPanel — a render that React discards + // must not leave the ref pointing at the callback from the abandoned pass. + // This effect only writes a ref, so it commits nothing and no wrapper + // re-renders for it; every caller of `setLoading` runs from an event or an + // awaited continuation, long after the first commit. + const onLoadingChangeRef = useRef(__onLoadingChange); + useIsomorphicLayoutEffect(() => { + onLoadingChangeRef.current = __onLoadingChange; + }, [__onLoadingChange]); + const loadingRef = useRef(false); + const setLoading = useCallback((next: boolean) => { + if (loadingRef.current === next) { + return; + } + loadingRef.current = next; + setIsLoading(next); + onLoadingChangeRef.current?.(next); + }, []); + // Track active pointer to defer popover.show() past click events. // With popover="auto", showing the popover between pointerdown and // pointerup/click causes the browser's light-dismiss to immediately @@ -499,7 +557,7 @@ export const BaseTypeahead = function BaseTypeahead({ // in-flight response for an older query fails the gen check below // instead of overwriting the newer results. const gen = ++searchGenRef.current; - setIsLoading(true); + setLoading(true); setHasSearched(true); try { const searchResults = await searchSource.search(searchQuery); @@ -534,7 +592,7 @@ export const BaseTypeahead = function BaseTypeahead({ setHighlightedIndex(-1); } finally { if (searchGenRef.current === gen) { - setIsLoading(false); + setLoading(false); } } }, @@ -545,6 +603,7 @@ export const BaseTypeahead = function BaseTypeahead({ announce, emptySearchResultsText, __queryEntries, + setLoading, t, ], ); @@ -552,7 +611,7 @@ export const BaseTypeahead = function BaseTypeahead({ // Perform bootstrap const performBootstrap = useCallback(async () => { const gen = ++searchGenRef.current; - setIsLoading(true); + setLoading(true); try { const bootstrapResults = await searchSource.bootstrap(); if (searchGenRef.current !== gen) { @@ -572,10 +631,10 @@ export const BaseTypeahead = function BaseTypeahead({ setResults([]); } finally { if (searchGenRef.current === gen) { - setIsLoading(false); + setLoading(false); } } - }, [searchSource, maxMenuItems, showLayer]); + }, [searchSource, maxMenuItems, showLayer, setLoading]); // Handle query change const handleQueryChange = useCallback( @@ -608,7 +667,7 @@ export const BaseTypeahead = function BaseTypeahead({ // its own `finally` will decline to clear this — so clear it here or // the field spins forever. Backspacing below the threshold on a remote // source is the everyday way to hit that. - setIsLoading(false); + setLoading(false); // Clear any lingering result-count / no-results announcement. announce(''); if (derived.length > 0) { @@ -645,6 +704,7 @@ export const BaseTypeahead = function BaseTypeahead({ debounceMs, searchSource, announce, + setLoading, ], ); @@ -673,11 +733,11 @@ export const BaseTypeahead = function BaseTypeahead({ // Same reason as in handleQueryChange: the invalidated search will not // clear this itself. Selecting a stale result while the next search is // still in flight would otherwise leave the field spinning. - setIsLoading(false); + setLoading(false); popover.hide(); inputRef.current?.focus(); }, - [onChange, popover, searchSource], + [onChange, popover, searchSource, setLoading], ); // Handle focus @@ -889,6 +949,7 @@ export const BaseTypeahead = function BaseTypeahead({ : undefined } aria-autocomplete="list" + aria-busy={isLoading || undefined} aria-describedby={ariaDescribedBy} aria-labelledby={ariaLabelledBy} aria-disabled={isFocusableDisabled ? 'true' : undefined} @@ -923,15 +984,11 @@ export const BaseTypeahead = function BaseTypeahead({ inputXStyle, )} /> - {isLoading && ( - - + {isLoading && __onLoadingChange == null && ( + + )} - {popover.render(
{ await waitFor(() => { expect(screen.getByRole('status', {name: 'Loading'})).toBeInTheDocument(); }); + expect(input).toHaveAttribute('aria-busy', 'true'); fireEvent.change(input, {target: {value: 'Ap'}}); await act(async () => { @@ -928,6 +931,7 @@ describe('BaseTypeahead minQueryLength', () => { expect( screen.queryByRole('status', {name: 'Loading'}), ).not.toBeInTheDocument(); + expect(input).not.toHaveAttribute('aria-busy'); expect(input).toHaveAttribute('aria-expanded', 'false'); }); }); @@ -1445,3 +1449,323 @@ describe('Typeahead statusVariant forwarding', () => { ); }); }); + +describe('busy indicator ownership', () => { + /** A source that stays in flight until the test settles it. */ + const pendingSource = () => { + let settle: (items: SearchableItem[]) => void = () => {}; + return { + source: { + search: async () => + new Promise(resolve => { + settle = resolve; + }), + bootstrap: () => [], + }, + settle: (items: SearchableItem[] = []) => settle(items), + }; + }; + + it('renders its own named status for a direct caller', async () => { + // BaseTypeaheadProps is re-exported from the package entry point, so the + // base has direct callers this repo cannot see. They painted no indicator + // of their own — the base did it for them — so it keeps doing it, and the + // status stays a named one rather than a bare aria-busy that only reaches + // assistive tech. + const {source, settle} = pendingSource(); + render( + {}} + debounceMs={0} + />, + ); + const input = screen.getByRole('combobox'); + + fireEvent.change(input, {target: {value: 'App'}}); + await waitFor(() => { + expect(screen.getByRole('status', {name: 'Loading'})).toBeInTheDocument(); + }); + // A Spinner, not the static clock glyph this used to render: `clock` + // means *time* everywhere else in core, and nothing about it moved while + // a search was out. + expect( + screen.getByRole('status', {name: 'Loading'}).querySelector('svg'), + ).toBeInTheDocument(); + + await act(async () => { + settle(); + await Promise.resolve(); + }); + expect( + screen.queryByRole('status', {name: 'Loading'}), + ).not.toBeInTheDocument(); + }); + + it('hands the indicator over to a field that takes it, and renders none itself', async () => { + // Typeahead and Tokenizer paint the spinner in the one inline-end lane + // they already own. Two indicators in one field is the defect this PR + // exists to fix, so the base must yield rather than add to it. + const {source, settle} = pendingSource(); + const onLoadingChange = vi.fn(); + render( + {}} + __onLoadingChange={onLoadingChange} + debounceMs={0} + />, + ); + const input = screen.getByRole('combobox'); + + fireEvent.change(input, {target: {value: 'App'}}); + await waitFor(() => { + expect(input).toHaveAttribute('aria-busy', 'true'); + }); + // Scoped by name: the announcer for result counts is a role="status" + // live region too, and it is not the indicator. + expect( + screen.queryByRole('status', {name: 'Loading'}), + ).not.toBeInTheDocument(); + expect(onLoadingChange).toHaveBeenLastCalledWith(true); + + await act(async () => { + settle(); + await Promise.resolve(); + }); + expect(onLoadingChange).toHaveBeenLastCalledWith(false); + }); + + it('reports each transition once, and reports nothing when nothing changed', async () => { + // Edge-triggered on purpose. Every keystroke below the query threshold + // clears the flag, and an unconditional report would hand the wrapper a + // `false` per character — each one a state write, and on a field that is + // re-rendering as the user types. + const {source, settle} = pendingSource(); + const onLoadingChange = vi.fn(); + render( + {}} + __onLoadingChange={onLoadingChange} + minQueryLength={3} + debounceMs={0} + />, + ); + const input = screen.getByRole('combobox'); + + // Below the threshold: no search, so nothing to report. + fireEvent.change(input, {target: {value: 'A'}}); + fireEvent.change(input, {target: {value: 'Ap'}}); + await act(async () => { + await Promise.resolve(); + }); + expect(onLoadingChange).not.toHaveBeenCalled(); + + fireEvent.change(input, {target: {value: 'App'}}); + await waitFor(() => { + expect(onLoadingChange).toHaveBeenCalledTimes(1); + }); + expect(onLoadingChange).toHaveBeenCalledWith(true); + + // Back below the threshold: one report out, not one per keystroke. + fireEvent.change(input, {target: {value: 'Ap'}}); + fireEvent.change(input, {target: {value: 'A'}}); + fireEvent.change(input, {target: {value: ''}}); + await act(async () => { + settle(); + await Promise.resolve(); + }); + expect(onLoadingChange).toHaveBeenCalledTimes(2); + expect(onLoadingChange).toHaveBeenLastCalledWith(false); + }); +}); + +describe('Typeahead end-lane reserve — render cost', () => { + // jsdom has no ResizeObserver and reports every width as 0, so the cost + // this guards is invisible without one: a reserve held in React state only + // re-renders when the measurement is non-zero. This stub is the smallest + // thing that makes the regression reproducible in CI — it reports a width + // the moment an element is observed, exactly as a browser would when the + // spinner mounts into the lane and again when it leaves. + class StubResizeObserver { + static instances = 0; + private readonly cb: ResizeObserverCallback; + constructor(cb: ResizeObserverCallback) { + this.cb = cb; + StubResizeObserver.instances++; + } + observe(target: Element) { + const entry = { + target, + borderBoxSize: [{inlineSize: 24, blockSize: 20}], + contentRect: {width: 24, height: 20}, + } as unknown as ResizeObserverEntry; + this.cb([entry], this); + } + unobserve() {} + disconnect() {} + } + + let originalRO: typeof ResizeObserver | undefined; + beforeEach(() => { + originalRO = globalThis.ResizeObserver; + StubResizeObserver.instances = 0; + globalThis.ResizeObserver = + StubResizeObserver; + // A real width, so a state-held reserve would have something to store. + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({ + width: 24, + height: 20, + top: 0, + left: 0, + right: 24, + bottom: 20, + x: 0, + y: 0, + toJSON: () => ({}), + }); + }); + afterEach(() => { + globalThis.ResizeObserver = originalRO as typeof ResizeObserver; + vi.restoreAllMocks(); + }); + + const pendingSource = () => { + let settle: (items: SearchableItem[]) => void = () => {}; + return { + source: { + search: async () => + new Promise(resolve => { + settle = resolve; + }), + bootstrap: () => [], + }, + settle: (items: SearchableItem[] = []) => settle(items), + }; + }; + + it('costs no commit of its own across a whole search', async () => { + // The lane's width reaches CSS as a custom property written to the DOM, + // never as state, so measuring it cannot re-render the field. The two + // commits below are the ones the search itself owes: the spinner + // arriving, and the spinner leaving. Held in state, the measurement + // doubled that — a second commit for each, carrying a number no + // JavaScript reads. + const {source, settle} = pendingSource(); + const commits: string[] = []; + render( + commits.push(phase)}> + {}} + debounceMs={0} + /> + , + ); + + const input = screen.getByRole('combobox'); + commits.length = 0; + + // Drive the whole cycle without waiting on any DOM signal, so the count + // is of the field's commits and nothing else — and so this reads the + // same against any implementation of the reserve. + await act(async () => { + fireEvent.change(input, {target: {value: 'App'}}); + }); + const afterStart = commits.length; + + await act(async () => { + settle([]); + await Promise.resolve(); + }); + + // One commit for the spinner arriving, one for it leaving. A reserve + // held in state adds a second to each, because the lane changes size + // exactly when it appears and disappears. + expect(afterStart).toBe(1); + expect(commits.length).toBe(2); + }); + + it('shares one observer across every field on the page', () => { + // One observer per lane is the other half of the cost: browsers batch per + // observer instance, so N fields meant N callback dispatches a frame. + render( + <> + {}} + /> + {}} + /> + {}} + /> + , + ); + // Three lanes (each field has a value, so each renders a clear button), + // one observer. + expect(StubResizeObserver.instances).toBeLessThanOrEqual(1); + }); + + it('publishes the measured width for CSS, and takes it back with the lane', async () => { + // The mechanism the two tests above are protecting: the number reaches + // the input as an inherited custom property, so the padding follows the + // lane without React seeing the value at all. + const {source, settle} = pendingSource(); + const {container} = render( + {}} + debounceMs={0} + />, + ); + const input = screen.getByRole('combobox'); + + await act(async () => { + fireEvent.change(input, {target: {value: 'App'}}); + }); + await waitFor(() => { + expect( + container.querySelector('[style*="--_astryx-end-lane-width"]'), + ).not.toBeNull(); + }); + + const host = container.querySelector( + '[style*="--_astryx-end-lane-width"]', + ); + expect(host?.style.getPropertyValue('--_astryx-end-lane-width')).toBe( + '24px', + ); + + await act(async () => { + settle([]); + await Promise.resolve(); + }); + // Lane gone, property gone — the input takes the room back. + await waitFor(() => { + expect( + container.querySelector('[style*="--_astryx-end-lane-width"]'), + ).toBeNull(); + }); + expect( + container.querySelector('[style*="--_astryx-end-lane-width"]'), + ).toBeNull(); + }); +}); diff --git a/packages/core/src/Typeahead/Typeahead.tsx b/packages/core/src/Typeahead/Typeahead.tsx index 4c05dd1579ff8..88cbce20690d0 100644 --- a/packages/core/src/Typeahead/Typeahead.tsx +++ b/packages/core/src/Typeahead/Typeahead.tsx @@ -42,6 +42,8 @@ import {Token} from '../Token'; import {useTooltip} from '../Tooltip'; import {renderIconSlot, type IconType} from '../Icon'; import {VisuallyHidden} from '../VisuallyHidden'; +import {Spinner} from '../Spinner'; +import {useEndLaneReserve} from '../Field/useEndLaneReserve'; import {spacingVars, sizeVars} from '../theme/tokens.stylex'; import {groupStyles} from '../InputGroup/groupStyles'; import {useInputGroup} from '../InputGroup/InputGroupContext'; @@ -167,6 +169,14 @@ export interface TypeaheadProps extends Omit< // Styles // ============================================================================= +// How far the end lane sits from the field's inline-end border. Also what the +// input has to clear, so it is named rather than repeated: the reserve below +// is derived from the same expression that positions the lane. +const LANE_INSET = { + md: `calc((${sizeVars['--size-element-md']} - 20px) / 2 - 1px)`, + sm: `calc((${sizeVars['--size-element-sm']} - 20px) / 2 - 1px)`, +} as const; + const styles = stylex.create({ wrapper: { position: 'relative', @@ -186,15 +196,24 @@ const styles = stylex.create({ // -(8px - 3px) = -5px positions token equidistant from left edge as top. margin: `calc(-1 * (${spacingVars['--spacing-2']} - ${spacingVars['--spacing-1']} + 1px))`, }, - clearButton: { + // One lane for everything that sits at the field's inline end. It is + // absolute rather than in-flow because the wrapper wraps: a selected token + // can push a later in-flow sibling onto a second row, and this has to stay + // on the field's first row. Anything added here is a flex child, so the + // busy indicator and the clear button sit beside each other instead of + // both landing in the same corner. + endLane: { position: 'absolute', - top: `calc((${sizeVars['--size-element-md']} - 20px) / 2 - 1px)`, - insetInlineEnd: `calc((${sizeVars['--size-element-md']} - 20px) / 2 - 1px)`, + display: 'flex', + alignItems: 'center', + gap: spacingVars['--spacing-1'], + top: LANE_INSET.md, + insetInlineEnd: LANE_INSET.md, height: '20px', }, - clearButtonSm: { - top: `calc((${sizeVars['--size-element-sm']} - 20px) / 2 - 1px)`, - insetInlineEnd: `calc((${sizeVars['--size-element-sm']} - 20px) / 2 - 1px)`, + endLaneSm: { + top: LANE_INSET.sm, + insetInlineEnd: LANE_INSET.sm, }, inputHidden: { width: 0, @@ -299,6 +318,17 @@ export function Typeahead({ }); // Edit mode: when the user clicks the token to edit the selected value + // Reported by BaseTypeahead so the indicator can live in this field's own + // end lane, beside the clear button, rather than in the engine's row. + const [isLoading, setIsLoading] = useState(false); + // The lane holds a spinner, a clear button, or both, so what the input has + // to clear changes while the field is in use. Measured rather than assumed. + const [laneRef, laneReserve] = useEndLaneReserve( + size === 'sm' ? LANE_INSET.sm : LANE_INSET.md, + ); + // Whether a lane renders at all — known from props and state, without + // measuring anything, which is why the reserve costs no extra commit. + const hasEndLane = Boolean(isLoading || (hasClear && value && !isDisabled)); const [isEditing, setIsEditing] = useState(false); const [editingValue, setEditingValue] = useState(null); @@ -485,10 +515,17 @@ export function Typeahead({ ariaLabelledBy={ariaLabelledBy} onChangeQuery={onChangeQuery} onOpenChange={onOpenChange} + __onLoadingChange={setIsLoading} debounceMs={debounceMs} anchorRef={wrapperRef} onKeyDown={handleKeyDown} - inputXStyle={showToken ? styles.inputHidden : undefined} + inputXStyle={ + showToken + ? styles.inputHidden + : hasEndLane + ? laneReserve + : undefined + } // While the token is shown the input is collapsed (width 0 / // opacity 0) — take it out of the Tab order so keyboard users // don't hit an invisible stop (WCAG 2.4.3 / 2.4.7). It stays @@ -497,15 +534,26 @@ export function Typeahead({ inputTabIndex={showToken ? -1 : undefined} size={size} /> - {hasClear && value && !isDisabled && ( - { - e.stopPropagation(); - handleClear(); - }} - xstyle={[styles.clearButton, size === 'sm' && styles.clearButtonSm]} - /> + {hasEndLane && ( +
+ {isLoading && ( + + )} + {hasClear && value && !isDisabled && ( + { + e.stopPropagation(); + handleClear(); + }} + /> + )} +
)}
{showsDisabledMessage &&