fix(Typeahead): keep the field's width when a value is selected - #5682
Open
freddymeta wants to merge 3 commits into
Open
fix(Typeahead): keep the field's width when a value is selected#5682freddymeta wants to merge 3 commits into
freddymeta wants to merge 3 commits into
Conversation
Selecting a value shrank the field — 199px to 44px for a one-word value, measured in Chromium — in any layout that sizes it to its content: a flex item, `inline-block`, a shrink-to-fit grid track. The docsite's own Typeahead page is one, since its preview is centred and a centred child is a flex item. A field's width must not depend on its value, and every other field keeps that promise for free: its `<input>` stays in flow and the field is as wide as the input's own default size. This one takes the input out of flow when the token shows, and the input is the only child with an intrinsic width, so the field was left measuring the token. Block-level parents hid it, because they fill their container whatever their content is — which is why no story caught it. So the field now states the width it already had rather than inheriting it from the input, as a floor applied only while the token shows: an unselected field is untouched, and a block-level or stretched one still fills. The value is public and themeable as `--typeahead-min-width`, since the right minimum for a field is a design decision, not a constant. Measured in Chromium in a flex parent, before -> after selection: 199 -> 44 becomes 199 -> 200. In a block parent both are 984 -> 984, unchanged. Note for the issue's suggested fix, which was to keep the collapsed input in flow: measured, that moves the field from 44px to 95px, because `width: 0` removes the input's intrinsic contribution whether or not it is in flow. The percentage form the repo usually reaches for, `min(200px, 100%)`, is also no use here — a percentage min-width resolves against an indefinite containing block during shrink-to-fit, computing to 0, and the field stayed at 44px. Tokenizer collapses its input the same way and shrinks for the same reason (199 -> 151); left alone here, since its multi-token wrapping makes the right floor a different question.
|
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 ComponentsTypeahead (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual RegressionStatus: Skipped — Broad stable scope is deferred to the daily release gate. It covers 1964 trusted baseline shots instead of recapturing them for this PR. View the report Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
The var was documented but not mapped, which derivedVarRegistry's own test
catches: a documented var either maps onto a standard CSS property through a
derived[] entry, or is listed as unmappable with a reason. `min-width` maps
onto this one directly, so it gets the entry rather than the exemption — which
also means a theme can write the standard property instead of the var:
typeahead: {base: {minWidth: '16rem'}}
That emits both the property and the var, so a theme raising the field's
minimum raises the collapsed floor with it rather than letting the two drift.
…ith a story theme-var-reachability walks the built Storybook and asks, for every documented var, which element sets it. `--typeahead-min-width` had no answer: it was declared inside the collapsed-state style, and no Typeahead story ever selects a value, so nothing in the built stories carried it. A var no element declares is a var no theme can reach, which is the gate's whole point. The declaration moves up to the wrapper, which is the element carrying the `typeahead` theme target and is always rendered. Only the `min-width` that reads it stays conditional, so the behavior is unchanged — an unselected field is still untouched — while a theme always has an element to set the value on. This is also the shape Spinner settled on for its own public vars. The missing story is worth having on its own account: no story rendered a selected value, and every one of them renders in a fixed-width container, which is exactly why a bug this visible survived. `With Selected Value` shows a token in a flex parent — the case that used to collapse. Verified against the built Storybook: reachability now reports `.astryx-typeahead sets it (200px)`, and the a11y audit is unchanged at 0 violations across all 17 Typeahead stories.
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
1 participant
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.