Repository navigation
feat(core): Tokenizer fits the pointer — a chip row and a sheet on a finger - #5315
imdreamrunner wants to merge 2 commits into
Conversation
…finger Adds a second surface to `Tokenizer`, chosen at runtime from the primary pointer, so a phone gets a control shaped for a thumb instead of one shaped for a keyboard. The pointer control needs a keyboard for both of its core gestures. You type BETWEEN the chips, in an input sharing their line, and you remove the last one with Backspace on an empty input. On a phone, focusing that input raises the virtual keyboard over the bottom half of the screen — where the suggestion popover opens — and each chip added grows the field by a line and pushes the page under your thumb. The touch surface is three ideas: - The chips scroll sideways instead of wrapping, so the field is exactly one line tall however many there are and nothing below it moves. - Add is a separate, fixed target OUTSIDE the scroller, at the trailing edge: same place with two chips or twenty, and a tap on it can never be mistaken for the start of a sideways drag. (The prototype had it inside the scroller, where it scrolls away once the chips overflow.) - Suggestions are a pinned-tall sheet: search at the top where the keyboard cannot cover it, full-width rows a thumb can hit, and `tall` is the one BottomSheet height that is keyboard-aware. The list populates before anything is typed — in a sheet the list IS the content, so `hasEntriesOnFocus`, which is about not putting a popover over the page unbidden, does not apply. Built from the mobile prototype at sandbox/pages/mobile-prototypes/?p=tokenizer. The switch is `pointer: coarse` alone, with no width bound — same test, and same reasoning, as the DateInput touch surface: `pointer` is the PRIMARY device, so a touchscreen laptop keeps the typable field and a narrowed desktop window is still a mouse, while a tablet gets the sheet at any width. The selection logic is now one hook, `useTokenSelection`, rather than two copies. That is what keeps the "Create X" sentinel — a synthetic item that has to be recognised on the way back in and turned into a real one — from forking between the surfaces. The pointer surface's rendered output is unchanged; its diff is the imports, the extraction, and a rename to `PointerTokenizerField`. Two things a browser caught that jsdom could not, both now covered by a test: - The sheet's sticky header sits between two layers in ONE stacking context. A bare `z-index: 1` put it over BottomSheet's grab handle and hid the pill; no z-index at all let List's `position: relative` rows paint straight through its background. It needs an isolated wrapper to scope the layer. - `minBlockSize` compiles to `min-height`, so a 44px floor declared next to the size scale is the same property and simply loses the merge. The floor is folded into the scale as `max(44px, <size>)`. Costs 16.4 KB gzipped on top of Tokenizer's 58.6 KB (measured: bundle the component alone, minified, from dist before and after), for every consumer including desktop-only ones, since the choice is made at runtime. New public API is one export, `TokenizerTouchSurface` — the touch half with the pointer test skipped, so the Storybook stories are reviewable on a laptop — plus three `@astryx.tokenizer.*` catalog keys. No media-query constant is exported: the DateInput surface already publishes `TOUCH_POINTER_QUERY`, and two of those in the root barrel would collide. Verified on an iPhone 15 profile in Chromium: the row scrolls (scrollWidth 540 vs clientWidth 253) while the field stays 52px and does not move, the sheet opens at 92dvh with the pill visible over the header, the header covers rows scrolling under it, and tapping the search field through it still focuses it.
meta-cla
Bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
needs:design-review
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsTokenizer (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: 1 accessibility violation(s) found — 1 serious. Tokenizer - 1 issue(s)
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
…nt of their own
`pr-a11y` failed, and the interesting part is why: the story title
`Core/Tokenizer/Touch surface` is the only three-segment title in the repo, and
the a11y tooling reads those two ends differently. The audit FILTERS by
`title.split('/')[1]` — `Tokenizer`, so `--components Tokenizer` found the
stories — but KEYS its report by `title.split('/').pop()`, so every violation
was filed under a component named "Touch surface", which does not exist. The
weekly a11y summary is built from the same report, so it would have grown a
phantom component too.
Folded the eight stories into `Tokenizer.stories.tsx` behind a section comment
instead. Two files can share a title (`Vega/VegaChart` does), but one file per
component is the repo's shape and leaves nothing to reason about.
That leaves one real violation, now correctly keyed
`Tokenizer::Touch: disabled, with a reason::color-contrast`, and it is the
disabled treatment the whole system already has rather than anything this
surface does. Measured, the three nodes are the field label at 2.52 and the two
chips at 1.72 — the same nodes and the same ratios as the already-baselined
`Tokenizer::Disabled` and `Tokenizer::Disabled With Message`. Both surfaces
compose the same two dims: `inputWrapperStyles.disabled` puts `opacity: 0.5` on
the field and `Token` adds its own, so a chip is at 0.25 effective on the
pointer surface today (verified in the built Storybook: `core-tokenizer--disabled`
reports the identical 0.25). Fixing it means changing what disabled looks like
system-wide, which is not this PR, and WCAG 1.4.3 exempts inactive controls. So
it joins the two entries beside it in the baseline.
The disabled Add button lands at the same 0.25 as the chips around it, which is
why axe does not flag it separately and why it should not be exempted from the
field's dim — it would be the one bright thing in a disabled field.
Verified: a11y gate passes (0 new, 3 baselined), RTL audit passes, the modal
close-visibility guard passes, storybook typecheck and build pass, eslint clean.
|
Follow-up commit: The story title broke the tooling. The remaining violation is the system-wide disabled treatment, not this surface. Measured, the three nodes on The disabled Add button sits at that same 0.25, which is why axe does not flag it separately — and why it should not be exempted from the field dim: it would be the one bright thing in a disabled field. Also worth flagging for #5243: the DateInput touch branch uses the same Locally: a11y gate passes (0 new, 3 baselined), RTL audit, modal close-visibility guard, storybook typecheck + build, eslint all clean. |
|
AI review status for this pull request.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on a maintainer decision. Tokenizer's current component spec requires one mounted combobox input and a suggestion menu tied to the field, and this touch surface replaces both on every coarse-pointer device. A maintainer needs to decide, and record in Tokenizer's spec, whether Tokenizer gets a touch surface, and whether callers reach it through an automatic switch plus the TokenizerTouchSurface export or through a presentation prop like Selector's. Whatever the decision, please rebase onto main, land the useTokenSelection extraction separately to bring the PR under 2,000 authored lines, and fix the touch-surface issues below:
- Removing a chip at
maxEntriessends focus to the page body. maxMenuItemsis applied before selected items are dropped, so eligible items go missing and the sheet can say "No results found".- The sheet stays usable after the field becomes disabled, and it shows rows from the previous search while a new one loads.
- Opening the sheet fires
onBlur.
[Automated review]
| * ``` | ||
| */ | ||
| export function Tokenizer<T extends SearchableItem>(props: TokenizerProps<T>) { | ||
| const isTouch = useMediaQuery(TOUCH_POINTER_QUERY); |
There was a problem hiding this comment.
This makes every existing Tokenizer switch surfaces on any (pointer: coarse) device, with no opt-out. The current Tokenizer contract requires one mounted combobox input and a field-anchored suggestion menu. A maintainer needs to settle the direction first: an automatic switch, or a presentation prop like Selector's.
| * handset-only app can render it directly. `Tokenizer` is the one to use: it | ||
| * picks this surface itself wherever the primary pointer is a finger. | ||
| */ | ||
| export {TouchTokenizerField as TokenizerTouchSurface} from './TouchTokenizerField'; |
There was a problem hiding this comment.
This is a new public export with no current authority behind it. Selector and MultiSelector already say "always use a sheet" with presentation="bottom-sheet". Settle it together with the direction question before it ships, because removing an export later is a breaking change.
| searchSource, | ||
| hasCreate, | ||
| maxEntries, | ||
| onAfterRemove: () => addButtonRef.current?.focus(), |
There was a problem hiding this comment.
At maxEntries the Add button is still natively disabled when this runs, because Button only uses aria-disabled when it has a tooltip. So focus() does nothing, and focus falls to <body> once the chip's remove button unmounts. Clear all at the cap goes down the same path. Consider moving focus after the commit, or to the next chip while Add is disabled.
| const [rawResults, setRawResults] = useState<T[]>([]); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const openSheet = useCallback(() => { |
There was a problem hiding this comment.
Opening the sheet keeps the previous rawResults, and isLoading stays false until the search Effect runs. As a result, the first commit shows the empty-results text, and a reopen shows the last search's rows, which can be tapped while the new bootstrap is pending. Clearing the results and setting loading here, and in handleQueryChange, fixes both.
| if (searchGenRef.current !== gen) { | ||
| return; | ||
| } | ||
| setRawResults(found.slice(0, maxMenuItems)); |
There was a problem hiding this comment.
This caps the results before decorateResults drops selected IDs. The sheet then offers fewer than maxMenuItems rows and never shows items past the first N raw results: with 12 options and 3 selected you get 7 rows, and once those are tapped the sheet says "No results found". The pointer path caps after filtering. Slice after decorating instead.
| [onFocus], | ||
| ); | ||
|
|
||
| const handleBlurCapture = useCallback( |
There was a problem hiding this comment.
BottomSheet focuses its panel on open, and the panel is outside wrapperRef. So this reports onBlur as soon as the sheet opens, and onFocus again when it closes. The prop is documented as firing only when focus leaves the tokenizer entirely. Treating focus inside the sheet as inside the field keeps that contract.
| ))} | ||
| </div> | ||
| <BottomSheet | ||
| isOpen={isSheetOpen} |
There was a problem hiding this comment.
If isDisabled flips to true while the sheet is open, the sheet stays open and handlePick still adds, because addItem in useTokenSelection only checks isAtMax. Deriving isOpen={isSheetOpen && !isDisabled} and guarding handlePick would keep a disabled field non-interactive.
| @@ -0,0 +1,57 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
This changes the functional default on coarse pointers, and nothing restores the old surface. Once the direction is settled, the Changeset should name the old default, the new default and the option that restores the old one, or move to the incompatible tier.
|
Superseded by #7976 (spec) and #7977 (implementation). Those port the Lab |
Builds the Tokenizer mobile prototype into
Tokenizeritself, as a second surface the component picks from the primary pointer — not a parallel component to adopt.Why the pointer control cannot just be made bigger
Both of its core gestures need a hardware keyboard. You type between the chips, in an input sharing their line, and you remove the last one with Backspace on an empty input. On a phone, focusing that input raises the virtual keyboard over the bottom half of the screen — where the suggestion popover opens — and each chip added grows the field by a line and pushes the page under your thumb.
The touch surface
tallis the one BottomSheet height that is keyboard-aware. The list populates before anything is typed: in a sheet the list is the content.Tapping a row adds that token and leaves the sheet up, so building a set of five is five taps.
Nothing changes at the call site — same props, same values, no new import, no media query to write. The switch is
pointer: coarsealone with no width bound, the same test and the same reasoning as the DateInput touch surface:pointeris the PRIMARY device, so a touchscreen laptop keeps the typable field and a narrowed desktop window is still a mouse, while a tablet gets the sheet at any width.Notable
useTokenSelectionholds what a token is — including theCreate "X"sentinel, a synthetic item recognised on the way back in and turned into a real one. That is the part that must not fork. The pointer surface's rendered output is unchanged; its diff is the extraction and a rename toPointerTokenizerField.z-index: 1put it over BottomSheet's grab handle and hid the pill; no z-index at all let List'sposition: relativerows paint straight through its background. It needs an isolated wrapper to scope the layer.minBlockSizecompiles tomin-height, so a 44px thumb floor declared next to the size scale is the same property and simply loses the merge. Folded into the scale asmax(44px, <size>).distbefore and after.)TokenizerTouchSurface(the touch half with the pointer test skipped, so the stories are reviewable on a laptop), plus three@astryx.tokenizer.*catalog keys. No media-query constant: feat(core)!: DateInput fits the pointer — a touch picker on a finger, the text field on a mouse #5243's DateInput surface already publishesTOUCH_POINTER_QUERY, and two of those in the root barrel would collide.CheckboxIndicatordraws its own tick; registering a name would put all seven bundled themes on the hook for an icon.Test plan
pnpm lint:strictclean (0 errors; the 53 warnings are pre-existing on main),pnpm build,pnpm test— 7,530 core+lab and 2,801 cli+apps tests pass.core/lab/charts typecheck:docs,cli typecheck:strict+typecheck:template-docs,storybook typecheck+build,lab:readiness:check.TokenizerTouch.test.tsxcovering the surface switch (including that width alone does not switch it, and that a tablet does), the closed field's contract, the sheet,hasCreate,maxEntries, and the disabled-with-a-reason path. The existingTokenizer.test.tsxis untouched and still passes — the shared setup answers(pointer: coarse)false, so it keeps testing the pointer surface.dist): the row scrolls (scrollWidth540 vsclientWidth253) while the field stays 52px and does not move; the sheet opens at 92dvh with the pill visible above the header; the header covers rows scrolling under it; tapping the search field through it still focuses it.Storybook: Core/Tokenizer/Touch surface — the first story reports which surface you are actually looking at; the rest force the touch half.