fix(Typeahead): keep the field's width when a value is selected - #5682
Open
freddymeta wants to merge 8 commits into
Open
fix(Typeahead): keep the field's width when a value is selected#5682freddymeta wants to merge 8 commits into
freddymeta wants to merge 8 commits into
Conversation
…d's end lane, and the input reserves it Three defects in one block. The indicator a search painted was `<Icon icon="clock">` — a static glyph, byte-identical to TimeInput's, 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: 17x20px of overlap in Typeahead, 19x20px in Tokenizer, the latter leaving part of the clear glyph unclickable. And the combobox never carried `aria-busy`. The engine reports the busy state instead of painting it, and each field paints it in the one inline-end lane it already owns beside its clear button and end content. A direct `BaseTypeahead` caller keeps the visible, named status it has always had — as a Spinner now, so the fix reaches those callers too — and passing the callback is what hands the indicator over, so a field never renders two. The lane is out of flow (these wrappers wrap, and an in-flow sibling gets pushed onto a second row by a token), so it reserved nothing and the query ran underneath it at a narrow width. `useEndLaneReserve` measures the rendered lane and returns the padding the input needs. Measured rather than assumed, because what the lane holds varies with the field's state and, in Tokenizer, includes arbitrary `endContent`. At 280px, all six states measured, overlap of the input's content box with the lane's controls: | | main | here | |---|---|---| | Typeahead, value settled | clear 17px | 0 | | Typeahead, value + search in flight | 0 | 0 | | Tokenizer, value idle / settled | clear 25px | 0 | | Tokenizer, value + search in flight | 0 | 0 | The two rows that were already 0 were only 0 because the in-flow indicator reserved its own width; moving it out is what would have regressed them, and the reserve is what holds them. The other two are the pre-existing case of the same bug, with no spinner in it at all. Reporting goes out at the call site through a ref rather than from an Effect, so the field's state change batches into the commit React was already doing instead of forcing a second one, and it is edge-triggered, so the redundant clear on every keystroke below the query threshold reports nothing. Stacked on #5385 until it landed; replanted on main now that it has.
… during render Two audit findings, neither user-visible. The latest-callback ref was assigned during render. The repo's own convention splits on exactly this: plain values are assigned during render (`snapPointsRef` in BottomSheetPanel), but the latest-callback refs beside them — `onMotionStartRef`, `onMotionCompleteRef` — are synced in a layout effect, because a render React discards must not leave the ref pointing at a callback from the abandoned pass. This effect writes a ref and nothing else, so it commits nothing and no wrapper re-renders for it; the doubled commits the review asked about stay gone. And the ResizeObserver's fallback for a browser with no `borderBoxSize` read `contentRect`, which is the content box — equal today, since the lane has no padding or border, and silently short the moment one is added. `offsetWidth` on the observed element is border-box, like the primary path.
The reserve fixed the overlap and paid for it in commits: a 20-token
search went from 20 renders to 120, and the reserve alone accounted for
the second half. `useEndLaneReserve` held the measured width in state,
so the lane changing size — which it does exactly twice a search, as the
spinner arrives and as it leaves — re-rendered the whole field to carry
a number no JavaScript ever reads.
It reaches CSS as a custom property written straight to the field
wrapper and inherited by the input, so the padding follows the lane
without React seeing the value at all. The rule is static now: one
class, `calc(inset + var(--_astryx-end-lane-width, 0px))`, generated
once instead of regenerated per width.
Measured, same test either way — `Profiler` around the field, one search
start to settle:
state-held reserve 2 commits at search start (fails)
custom property 1 commit (passes)
which is the doubling, and it holds for the settle edge too. jsdom
reports every width as 0, so a state-held reserve never re-renders
there and the regression is invisible; the test stubs a ResizeObserver
that reports a width, which is the smallest thing that makes it
reproducible in CI.
The observer is shared as well — `observeResize`, the same singleton
`useTruncation` uses. Three fields on a page created three observers
before and create one now, so the browser dispatches one callback a
frame rather than N.
Verified in Chromium at 280px that the accepted fix still holds: while
busy, the input's content box ends at 268 and the lane starts at 276 —
zero overlap with the caret, and the reserve is released when the lane
goes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
community
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsTokenizer (@astryxdesign/core) · View in Storybook
Typeahead (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: 1 accessibility violation(s) found — 1 serious. Tokenizer - 1 issue(s)
Visual Regression518 added · 0 removed. View the report To accept these exact frames: Added — After Added — After Added — After Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
cixzhang
requested changes
Aug 30, 2026
cixzhang
left a comment
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
The default collapse is real, but this Typeahead-only contract is not safe yet: InputGroup cancels the 200px floor, narrow supported widths can overflow, Tokenizer has the same family-level failure, and the custom variable differs semantically from standard themed minWidth.
Please define consistent input-family sizing first and use one coherent standard-property contract rather than a component-specific public variable.
[Reviewed by Robohands]
…easure Tokenizer's lane in local space Two things, both from the transform report. **The measured width was in the wrong coordinate space.** `getBoundingClientRect()` reports viewport space — it carries every CSS transform above the element — while the padding it fed is in the element's own local space. Reproduced in Chromium on a 98px lane: `scale(.5)` published 49px, reserved half of what was needed, and put the live query back under the controls by 22.83px; `scale(2)` published 196px and left the caret in a 202.69px gap. `offsetWidth` is the untransformed border-box width and reads 98 at every scale, so that is what the reserve now uses. It is already an integer, which is the rounding the old `Math.ceil` was there for. **Typeahead does not need a measured lane at all.** Its spinner and clear button are now ordinary in-flow flex siblings of the input, which is TextInput's shape for exactly these controls: an in-flow box takes up room, so nothing can run underneath it and there is nothing to measure. The whole reserve, and the absolute lane it existed for, are gone from this field. What stood in the way was `flex-wrap: wrap` on Typeahead's wrapper — not a decision about this field, but something it picked up wholesale in the #2941 rename migration. The shared field base does not set it and TextInput does not use it. It cannot coexist with in-flow end controls either, because flex moves an item to a new line rather than shrinking it: with it, a long value put the clear button and spinner on a row of their own and a 280px field grew to 46px tall. Removed, the token ellipsizes instead — which is what Token already does, capping itself at 100% and clipping — and the field stays one row. Two rules I tried on the way out did nothing and are not here: `min-width: 0` on the token and `flex-shrink: 0` on the lane both measured byte-identical with and without, because Token already shrinks and the controls' min-content is their own size. Tokenizer keeps the lane. 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. Chromium, busy, at scale .5 / 1 / 2 — Typeahead: no reserve at all, overlap 0, 0, 0. Tokenizer: 98px published at all three, overlap 0, 0, 0, and a constant 3.34px local gap. Before, the same three read 22.83px of overlap, 0, and 202.69px of gap. Coverage, per the review: Tokenizer gains the busy-only path (a spinner as the lane's only occupant) and the collapsed path (the input takes no reserve when it has no width to pad), plus a direct regression test for the bug — the stub reports 24px from `offsetWidth` and 12px from the rect, and the published value must be 24px. Typeahead gains three guards that its controls stay in flow: no absolute lane, no reserve property, no wrapping. Reverting either fix fails its test.
…erve with its one caller Self-review of the previous commit found one bug and one misfiling. **The clear button drifted into the middle of the field.** Putting the controls in flow is right, but TextInput gets them to the inline end for free: its input is always present and `flex: 1`, so it absorbs the free space and pushes them over. Typeahead collapses its input to nothing whenever a token shows — which is the ordinary state of a field with a value — leaving no flexible item in the row, so the controls came to rest against the token. Measured: the clear button at x=39 in a 300px field, where TextInput's sits at 281. An `auto` margin gives the free space to the margin instead of to a sibling, so it needs no sibling to exist: 281 now, and 0px of drift when the spinner joins it, matching TextInput exactly. That is also why these two are wrapped again. The wrapper is an ordinary in-flow flex child, not the absolute lane the review objected to — it is simply the one element the margin can sit on when either control may be absent. **`useEndLaneReserve` moved from `Field/` to `Tokenizer/`.** It has exactly one caller now. Everything else in `Field/` is genuinely shared — `InputClearButton` has 15 consumers — so a Tokenizer-only workaround filed there read as shared infrastructure and invited the next field to reach for the measured reserve, which is the thing this review was about not doing. Also checked and left alone: no dead style keys in any of the three files, no unused imports, the hook is not re-exported from any barrel, and `--typeahead-min-width` is not involved. Two candidate rules from the last round measured byte-identical with and without and are still absent. Chromium, busy, scale .5 / 1 / 2 — Typeahead: no reserve, overlap 0 / 0 / 0, controls pinned to the field's end at a constant inset, one row throughout. Tokenizer: 98px published at all three, overlap 0 / 0 / 0. One note for a separate change: the `@astryx.typeahead.loading` catalog entry describes a spinner "inside a Typeahead dropdown". It has always rendered in the field row, not the dropdown, so the description was already wrong on main; left for its own diff rather than widening this one.
…apse Replaces the --typeahead-min-width floor. Review's objection held up on every point I could check: it was a second sizing contract beside the documented Field.width prop, the 200px was hand-derived and overshot (the empty field measures 199), InputGroup cancelled it, and it could not help Tokenizer. The cause is narrower than a floor. Every other field's width comes from its <input> staying in flow with its intrinsic width; this one set `width: 0; min-width: 0; flex: 0 0 0; position: absolute` while a token showed, so the field measured the token instead. The input now keeps its place and its width and 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, which is the same value-dependent sizing from the other direction. Measured in Chromium, field in a max-content parent: TextInput (baseline) 199 -> 199 Typeahead before 199 -> 57 Typeahead after 199 -> 223 The residual 24 is the clear button joining the row, ordinary for any field with a conditional clear. The collapse this removes was 142. Stacked on #5555, whose commits are below this one; it reworked the same end-lane layout and documented this very cause in prose ("TextInput's input is always present and flex: 1. This field collapses its input to nothing while a token shows"). Based on main so CI runs. 73 Typeahead tests; the three new ones each verified failing on #5555's head. Tokenizer is deliberately untouched and still sizes to content — different mechanism, and two other PRs are already in that file.
freddymeta
force-pushed
the
fix/typeahead-collapse-width
branch
from
August 31, 2026 06:13
fdca9db to
35a6925
Compare
Contributor
Author
|
Reworked, and stacked on #5555 — its commits are below this one, base still The var is gone. Every point in your review held up when I checked it, and together they said the floor was the wrong shape:
The cause is narrower than a floor. Every other field's width comes from its Measured in Chromium, field in a
No constant, nothing for Being straight about the residual 24px: that is the clear button entering the row, not the value's length — ordinary for any field whose clear is conditional, and it does not vary with the value. The collapse this removes was 142px. If you want the field flat across both states, that is reserving the clear slot the way Tokenizer is deliberately not fixed here, and you were right that it shares the failure — I measured it rather than taking the fix's word for it. Removing tokens one at a time from Three tests, each verified failing on #5555's head. 73 Typeahead tests pass; tsc and lint clean (the two |
github-actions
Bot
added
the
needs:design-review
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
August 31, 2026 06:13
Contributor
Author
|
Before/after, captured in Chromium at 3× against a built Storybook of each side. BEFORE is #5555's head, so this isolates just this change.
The red dashed outline is the shrink-to-fit parent — a table cell, an inline-flex toolbar, a floated column. It is the layout that exposes this; a block-level parent hides it entirely, which is why no story caught it.
The second frame is the bug in one picture: the field has shrunk to its value, and the clear button is jammed against the token with the field's own border cutting through the gap. In the fourth, the field is where it was and the token sits inside it. (81px here vs the 57px in my earlier comment — same collapse, measured on a different story with a shorter value. The point is that the number tracks the value's length, which is exactly what a field's width must not do.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5560.
It is the component, not the docsite
The docsite only exposes it. Its preview is centred, a centred child is a flex item, and a flex item is sized to its content — a perfectly ordinary layout that every other field survives. Three fields in identical
display: flexparents, measured in Chromium at a 1000px viewport, before and after giving each one a value:TextInputTokenizerTypeaheadTextInputis the control: same parent, same viewport, no movement. So the layout is not what is wrong.inline-blockreproduces it exactly (199 → 44). A block-level parent hides it completely (984 → 984 for all three), which is why no story caught it: every Typeahead story renders in a fixed-width container.Cause
A field's width must not depend on its value, and every other field keeps that promise for free: its
<input>stays in flow, so the field is as wide as the input's own default size. Typeahead takes the input out of flow when the token shows —— and the input is the only child with an intrinsic width. Neither the Typeahead wrapper nor the shared
inputWrapperStyles.basesets one. Remove the input and the only thing left to measure is the token, so a content-sized parent shrinks the field onto it. At 44px the token's own label is clipped to one letter and the clear button lands on top of it.The two approaches that don't work
Both were measured, not reasoned about:
Keeping the collapsed input in flow — the issue's own suggestion, and my first instinct — moves the field from 44px to 95px. It does not fix it.
width: 0removes the input's intrinsic contribution whether or not the input is in flow, and droppingposition: absolutegives back only the flex gap.min(200px, 100%), the shape this repo reaches for elsewhere to mean "yield when there is no room", silently does nothing — the field stayed at 44px. A percentage min-width resolves against an indefinite containing block during shrink-to-fit, so it computes to 0 andmin()picks it. Worth knowing before someone copies that idiom into another intrinsic-sizing context.The fix
The field states the width it already had instead of inheriting it from the input:
Applied only while the token shows, so an unselected field is byte-identical to today.
min-width, notwidth, so a block-level or stretched field still fills exactly as it does now — this only stops the collapse. The default is the width the field already measures: 181px, which is what a browser gives an<input>at the base font, plus this field's own 19px of padding and border.The value is public and themeable, because the right minimum for a field is a design decision rather than a constant:
Measured
Chromium, before → after selection:
display: flexinline-blockdisplay: blockThe remaining 1px is the gap between the browser's font-derived default and the stated floor; a theme that wants them identical can say so.
(Assets live on the
assets/pr-5560branch — asset-only, deletable with the PR. I have no fork.)Tests and story
Two unit tests, in the probe-class style this repo already uses for declarations jsdom cannot measure: the floor is present when a token shows, and absent when one does not. Reverting the fix fails the first and leaves the second passing.
One story,
With Selected Value. No Typeahead story rendered a selected value, and every story renders inside a fixed-width container — between them, that is exactly why a bug this visible survived. The new one shows a token in a flex parent, the case that used to collapse.Full build, core typecheck, docs typecheck, Storybook typecheck,
check:repoandlint:strictpass; 286 test files green.Two things CI caught, both fair
A documented var has to be registered.
derivedVarRegistry's test requires every documented var to either map onto a standard CSS property or be listed as unmappable with a reason.min-widthmaps onto this one, so it gets the entry — which also means a theme can write the standard property and have both it and the var emitted:A var no element declares is a var no theme can reach.
theme-var-reachabilitywalks the built Storybook asking which element sets each documented var, and--typeahead-min-widthhad no answer: it was declared inside the collapsed-state style, and no story ever selected a value. The declaration moved up to the wrapper — the element carrying thetypeaheadtheme target, always rendered — and only themin-widthreading it stays conditional. Same behavior, and the same shape Spinner settled on for its own public vars. Reachability now reports.astryx-typeahead sets it (200px).Left alone
Tokenizer collapses its input the same way and shrinks for the same reason (199 → 151). Its input is multi-token and wrapping, so the right floor there is a different question than a single-value field's, and worth its own change.
Long values still widen the field (a 49-character value takes it to 278px). That is the same invariant seen from the other side, and it predates this issue; fixing it properly means laying the token over the input rather than beside it, which changes long values from widening the field to truncating in place — a product decision rather than a bug fix, so not smuggled in here.