test: cover the two keyboard paths we hand-wrote - #522
Merged
Conversation
#86's last open workstream. It reads as "no arrow-key/Home/End/Escape tests for menus, tabs, accordion, palette, pagination", which sounds like a large gap and is not one: all of that is reka-ui's roving focus and typeahead, and testing it here would be testing reka-ui. Measured instead: five places in `src/runtime` handle a key themselves. `defineShortcuts`, `ChatPrompt` and `useEditorMenu` already have specs. Two did not, and both are things a keyboard user meets directly. `CommandPalette` — Backspace means two different things depending on the caret. With text typed it deletes a character; with the field empty it steps back out of a group the user drilled into, which is the only keyboard way out of one. Confusing them is immediately visible: hitting backspace to fix a typo would throw you out of the group. The second case is asserted through the placeholder rather than the visible items. With text in the field the root group's items are filtered out anyway, so "Pictures is not shown" holds whether or not we stepped back — the first draft asserted exactly that and could not fail. The placeholder comes off the group on the history stack, which the filter does not touch. `FileUpload` — a drop target is not a native control, so everything a keyboard user needs is hand-written: a `tabindex` to reach it, `role="button"` to announce it, Enter and Space to activate it, and `@keydown.space.prevent` so Space does not scroll the page out from under them between keydown and keyup. All four are gated on `interactive` and `disabled`, and none was exercised. Both specs are mutation-checked branch by branch — dropping the empty-search guard, removing `history.pop()`, dropping `keydown.space.prevent`, narrowing `keyup.enter.space` to Enter alone, pinning `tabindex` at 0, and removing the `interactive`/`disabled` guards each redden a named test. `vi.restoreAllMocks()` runs after each `FileUpload` case: `spyOn` on a prototype method hands back the existing spy when one is already installed, so without it the click count carries between cases and "was not called" passes or fails on another test's clicks. Refs #86
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
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.
Linked issue
Refs #86 — its last open workstream.
Type of change
revert(Scope): ...)No
src/changes — two new spec files.The workstream is much smaller than it reads
#86 describes it as "no arrow-key/Home/End/Escape tests for menus, tabs, accordion, palette, pagination". That sounds like a large gap. It is not one: all of that is reka-ui's roving focus and typeahead, and a test here would be testing reka-ui.
Measured instead — five places in
src/runtimehandle a key themselves:composables/defineShortcuts.tscomponents/ChatPrompt.vuecomposables/useEditorMenu.tscomponents/CommandPalette.vuecomponents/FileUpload.vueThose two are what this covers, and both are things a keyboard user meets directly.
CommandPalette— Backspace means two different thingsWith text typed it deletes a character, the ordinary meaning. With the field empty it steps back out of a group the user drilled into — the only keyboard way out of one. Confusing them is immediately visible: hitting backspace to fix a typo would throw you out of the group instead.
The second case is asserted through the placeholder, not the visible items. With text in the field the root group's items are filtered out anyway, so "Pictures is not shown" holds whether or not we stepped back — the first draft asserted exactly that and could not fail. Removing the empty-search guard left it green. The placeholder comes off the group on the history stack, which the filter does not touch.
FileUpload— a dropzone is not a native controlEverything a keyboard user needs is hand-written on it, and none of it was exercised:
tabindexto reach itrole="button"to announce it@keydown.space.prevent, so Space does not scroll the page out from under them between keydown and keyupAll four are gated on
interactiveanddisabled.Mutation-checked, branch by branch
history.pop()@keydown.space.preventkeyup.enter.spaceto Entertabindexat0interactive/disabledguardsOne note worth carrying to other specs:
vi.restoreAllMocks()runs after eachFileUploadcase.spyOnon a prototype method hands back the existing spy when one is already installed, so without it the click count carries between cases and "was not called" passes or fails on another test's clicks.Gate
lint,typecheck, 340 test files / 7694 tests.Checklist
Generated by Claude Code