Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions .changeset/typeahead-collapsed-width.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
'@astryxdesign/core': patch
---

[fix] Typeahead: the field keeps its width when a value is selected, and the value stays out of the end controls (#5560)

Two halves of one promise from the input-field family contract
(`docs/families/input-fields.md`): **FR1**, a field's available width does not
change because its value did; and **FR2**, a visible end affordance does not have
field content painted under it.

**FR1 — the input keeps its place.** Every other field in the family gets a
stable width for free: the `<input>` stays in flow, and the field is as wide as
the input's own intrinsic width. Typeahead took the input out of flow and zeroed
its width while a token showed, so the field was left measuring the token. In any
shrink-to-fit parent it snapped to the value's length. Block-level parents hid
it, because they fill their container whatever their content is, which is why no
story caught it. The input now keeps its place in the row and its own width — it
is only made invisible and inert — and the token is painted over that space
rather than beside it. In flow the token would add its own width instead, which
is the same value-dependent sizing from the other direction: a long value would
grow the field.

**FR2 — the value is bounded by a content lane.** The input and the token share
a content lane: an ordinary flex item, `flex: 1` with `min-width: 0`, that ends
exactly where the end lane begins. That is TextInput's own arrangement — the lane
takes the free space so the end controls sit in the corner, and yields all of it
when the field is narrow, so a narrow field cannot overflow. The token is
anchored at both of the lane's inline edges, so a long value ellipsizes at the
lane's edge instead of reaching the controls. Positioned against the whole field
instead, as the first revision of this change did, it had no idea where those
controls start.

Measured in Chromium. Widths are the field's border box, field in a `max-content`
parent, `Field.width` otherwise unset:

| | empty | short value | long value |
| --------------------------------- | ----- | ----------- | ------------ |
| TextInput (family baseline) | 199px | 227px | 227px |
| Typeahead before | 199px | **54.7px** | **224.09px** |
| Typeahead after | 199px | 223px | 223px |
| Typeahead in `InputGroup`, before | 397px | **252.7px** | **422.09px** |
| Typeahead in `InputGroup`, after | 397px | 421px | 421px |

The 24px between the empty and valued columns is the clear button entering the
row — ordinary for any field whose clear is conditional, it does not vary with the
value, and TextInput's is 28px.

Overlap is the value's trailing edge past the clear button's leading edge; escape
is how far the value reaches past the field's border. The middle column is this
change's own first revision, which fixed the width and made the overlap worse:

| field, long value | overlap on main | first revision | now |
| ----------------- | --------------- | -------------- | --------------- |
| shrink-to-fit | 12px | 28.09px | none, 7px clear |
| in `InputGroup` | 12px | 33px | none, 7px clear |
| 220px | 12px | 31.09px | none, 7px clear |
| 180px | 12px | 33px | none, 7px clear |
| 140px | 12px | 33px | none, 7px clear |
| escape, 140–220px | none | up to 4px | none |

No new API and no constants. An earlier revision floored the field with a
`--typeahead-min-width` public var defaulting to 200px, which review rightly
rejected: it was a second sizing contract beside the documented `Field.width`
prop, it was hand-derived (the empty field measures 199, so the floor overshot by
1), `InputGroup` cancelled it, and it could not help `Tokenizer`. Nothing here
states a width; the lane's `min-width: 0` is the opposite of a floor.

`Tokenizer` is **not** fixed here. It shares the family promise and breaks it —
199px empty to 114.7px with one token, in the same probe — but by a different
mechanism: its tokens are in flow and wrap, and its input deliberately becomes a
40px continuation lane after them, so what a wrapping multi-value field's width
should be is a design question rather than this bug. Its numbers are identical
before and after this change.

@freddymeta
21 changes: 21 additions & 0 deletions .changeset/typeahead-tokenizer-busy-indicator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
'@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 `<Icon icon="clock">` — 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 overlap is visual, not functional — the clear button is positioned, so it paints above the in-flow indicator and stays clickable across the whole covered band. 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.

Typeahead puts both controls **in flow**, as ordinary flex siblings of the input, exactly as TextInput does with its own spinner and clear button — an in-flow box takes up room, so the input cannot run under it and there is nothing to measure. Getting there meant dropping `flex-wrap: wrap` from its wrapper, which the shared field base does not set and TextInput does not use: this field holds at most one token, so there is no second row to wrap to, and wrapping is what made an in-flow lane impossible, since flex moves an item to a new line rather than shrinking it. Measured in Chromium: with `flex-wrap` restored and a token too wide to share the row, the end controls drop to a second row and a 280px field grows from 32px to 46px tall. Unwrapped, a long value ellipsizes in the token instead.

Tokenizer's own pre-existing case of the overlap closes with it: at 280px with a token and no search running, its clear button covered 20px of the input's content box, and covers none now.

Tokenizer keeps a measured lane, because it cannot use the in-flow shape: its lane stays pinned to the field's first row while tokens wrap below it, so it has to be out of flow, and an out-of-flow box reserves nothing. Its width is measured with `offsetWidth` rather than `getBoundingClientRect()`. The rect is in viewport space — it carries every CSS transform above the element — while the padding it feeds is in local space, so mixing them broke under any transform: measured in Chromium, `scale(.5)` reserved half of what was needed and put the query back under the controls by 22.83px, and `scale(2)` left the caret in a 202.69px gap. `offsetWidth` is the untransformed border-box width and reports the same number at every scale.

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. The property is `--_tokenizer-end-lane-width`: private and component-named, like every other runtime layout var in the package, and never something a theme writes.

The busy indicator now appears in each field's documented anatomy, delegating its theming to `component:Spinner` rather than gaining a target of its own — the disposition `TextArea`, `CheckboxList` and `CommandPalette` already use for the same part.

@freddymeta
57 changes: 56 additions & 1 deletion apps/storybook/rtl-audit/targets.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
[
{
"component": "AppShell",
"storyId": "core-appshell--top-nav-with-side-nav",
"dims": [
"D4"
],
"selectors": {
"overlay": ".astryx-app-shell-sidenav",
"overlayRoot": ".astryx-app-shell"
}
},
{
"component": "ButtonGroup",
"storyId": "core-buttongroup--horizontal",
Expand All @@ -10,6 +21,28 @@
"next": "[role=\"group\"] button:last-child"
}
},
{
"component": "CheckboxList",
"storyId": "core-checkboxlist--rich-descriptions",
"dims": [
"D2"
],
"selectors": {
"prev": ".astryx-checkbox-input",
"next": "[data-testid=\"checkbox-end-content\"]"
}
},
{
"component": "RadioList",
"storyId": "core-radiolist--rich-content",
"dims": [
"D2"
],
"selectors": {
"prev": "input[aria-label=\"Pro\"]",
"next": "[data-testid=\"radio-end-content\"]"
}
},
{
"component": "Calendar",
"storyId": "core-calendar--default",
Expand Down Expand Up @@ -45,5 +78,27 @@
"prev": "button[aria-label=\"Previous\"]",
"next": "button[aria-label=\"Next\"]"
}
},
{
"component": "Typeahead",
"storyId": "core-typeahead--logical-order",
"dims": [
"D2"
],
"selectors": {
"prev": ".astryx-token",
"next": ".astryx-input-clear-button"
}
},
{
"component": "Tokenizer",
"storyId": "core-tokenizer--logical-order",
"dims": [
"D2"
],
"selectors": {
"prev": ".astryx-token",
"next": ".astryx-input-clear-button"
}
}
]
]
69 changes: 69 additions & 0 deletions apps/storybook/stories/Tokenizer.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof Tokenizer> = {
title: 'Core/Tokenizer',
component: Tokenizer,
Expand Down Expand Up @@ -454,3 +475,51 @@ export const StatusVariantComparison: Story = {
);
},
};

export const Loading: Story = {
render: args => {
const [value, setValue] = useState<SearchableItem[]>([users[0]]);
return (
<Tokenizer
{...args}
searchSource={slowUserSource}
value={value}
onChange={items => setValue(items)}
hasClear
endContent={<span>{value.length} selected</span>}
/>
);
},
args: {
label: 'Team Members',
placeholder: 'Search people...',
},
name: 'Loading (async source, with clear and end content)',
};

/**
* Tokens plus a clear-all button — the two ends of the field. Under RTL they
* must swap sides; this is the story the RTL audit measures as a D2
* layout-order-flip.
*/
export const LogicalOrder: Story = {
render: args => {
const [value, setValue] = useState([users[0], users[2]]);
return (
<div style={{width: 420}}>
<Tokenizer
{...args}
searchSource={userSource}
value={value}
onChange={items => setValue(items)}
/>
</div>
);
},
args: {
label: 'Team Members',
placeholder: 'Add more...',
hasClear: true,
},
name: 'Logical order',
};
126 changes: 126 additions & 0 deletions apps/storybook/stories/Typeahead.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,45 @@ const fruitSource: SearchSource = {
bootstrap: () => fruits.slice(0, 5),
};

/**
* A value longer than the input's own width — the case that used to collapse
* the field onto its value, and then to run under the clear button.
*/
const longFruit: SearchableItem = {
id: '9',
label: 'Elderberry and Blackcurrant Preserve',
};

const longFruitSource: SearchSource = {
search: (query: string) =>
[...fruits, longFruit].filter(f =>
f.label.toLowerCase().includes(query.toLowerCase()),
),
bootstrap: () => [longFruit, ...fruits.slice(0, 4)],
};

/**
* 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<typeof Typeahead> = {
title: 'Core/Typeahead',
component: Typeahead,
Expand Down Expand Up @@ -265,3 +304,90 @@ export const StatusVariantComparison: Story = {
);
},
};

export const Loading: Story = {
render: () => {
const [value, setValue] = useState<SearchableItem | null>(null);
return (
<div style={{width: 320}}>
<Typeahead
label="Fruit"
placeholder="Type to search…"
searchSource={slowFruitSource}
value={value}
onChange={setValue}
hasClear
debounceMs={0}
/>
</div>
);
},
name: 'Loading (async source)',
};

/**
* The two cases no Typeahead story covered, which is why a bug this visible
* survived: a value selected, and a parent that is sized by its content.
*
* Every other story renders in a fixed-width container, and a block-level
* parent fills its container whatever its content is — so both hid a field
* that sized itself to its value. Here the field is a flex item, so it is
* shrink-to-fit: a table cell, an inline toolbar, a floated column.
*
* Left, a long value: it must not widen the field, and it must ellipsize
* before the clear button rather than under it. Right, an empty field for
* comparison — the two must be the same width.
*/
export const SelectedValueInAContentSizedParent: Story = {
render: () => {
const [a, setA] = useState<SearchableItem | null>(longFruit);
const [b, setB] = useState<SearchableItem | null>(null);
return (
<div style={{display: 'flex', alignItems: 'flex-start', gap: 16}}>
<Typeahead
label="Selected"
searchSource={longFruitSource}
value={a}
onChange={setA}
hasClear
/>
<Typeahead
label="Empty"
placeholder="Type to search…"
searchSource={longFruitSource}
value={b}
onChange={setB}
hasClear
/>
</div>
);
},
name: 'Selected value in a content-sized parent',
};

/**
* One field with a selected value and a clear button — the two ends of the
* content lane. The token opens the lane and the clear button closes it, so
* under RTL they must swap sides: this is the story the RTL audit measures as
* a D2 layout-order-flip.
*
* A single field on purpose. The comparison story next to it renders two, and
* the audit's selectors would match across both.
*/
export const LogicalOrder: Story = {
render: () => {
const [value, setValue] = useState<SearchableItem | null>(longFruit);
return (
<div style={{width: 320}}>
<Typeahead
label="Fruit"
searchSource={longFruitSource}
value={value}
onChange={setValue}
hasClear
/>
</div>
);
},
name: 'Logical order',
};
Loading
Loading