Skip to content

Autofocus searchbar and open dropdown on typing#9052

Merged
rubenfiszel merged 3 commits into
mainfrom
di/filter-searchbar-autofocus
May 6, 2026
Merged

Autofocus searchbar and open dropdown on typing#9052
rubenfiszel merged 3 commits into
mainfrom
di/filter-searchbar-autofocus

Conversation

@diegoimbert
Copy link
Copy Markdown
Contributor

No description provided.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 6, 2026

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 07cf90e
Status:⚡️  Build in progress...

View logs

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

Claude finished @diegoimbert's task in 1m 59s —— View job


Mergeable, but should ideally address nits: misleading "if not handled" comment in TaggedTextInput.handleKeyDown — small, focused PR; no blocking issues.

Summary

Adds an optional autofocus prop and a generic onKeyDown callback to TaggedTextInput, threads them through FilterSearchbar, and turns both on for the runs page so typing into a freshly-opened page autofocuses the searchbar and reopens the dropdown after Escape.

Findings

P2 — onKeyDown is not an "unhandled-only" hook (TaggedTextInput.svelte:409)

The new comment reads:

// If not handled by our logic, call external onKeyDown handler if provided
onKeyDown?.(e)

This is partially misleading. The early return at the top correctly skips Arrow{Up,Down}/Enter, but Backspace, Delete, ArrowLeft, ArrowRight only return when their specific escape-sequence branch fires. For ordinary navigation/deletion (e.g. plain ArrowLeft at a normal cursor position, plain Backspace when not preceded by \\), the function falls through and onKeyDown is invoked.

That happens to match the current consumer's intent (open the dropdown on basically any input), but the rule it documents is "called whenever no escape-sequence shortcut intercepted the key", not "called when not handled". Worth tightening the comment so a future caller doesn't read it as "this only fires for unhandled keys" and rely on that.

Nit (informational) — autofocus re-renders

autofocus={false} on the contenteditable div in Svelte 5 is correctly omitted (Svelte treats autofocus as a boolean attribute), so passing autofocus from a parent that doesn't bind it to anything is safe. The runtime browser behavior of autofocus on a contenteditable <div> is supported (HTML5 made it a global attribute), so this works as intended in modern browsers — no change requested.

AGENTS.md compliance

  • No $bindable(default_value) on optional props: ✅ — both new props are plain optional props, not bindable.
  • Frontend rules (frontend/CLAUDE.md) — no design-system violation since the touched surfaces are existing components.
  • Change is focused (15/-1, 3 files) — ✅ matches the "keep changes focused" core principle.

Test coverage

  • Frontend UI / component changes only — per the project policy in REVIEW.md, no Svelte component tests expected.
  • No new pure-logic utility introduced that would warrant a *.test.ts.
  • CI / docs / config: not touched.

Manual verification: open the Runs page from a fresh navigation and confirm the searchbar receives focus; with the searchbar focused, press Escape to dismiss the dropdown, then begin typing — the dropdown should reopen on the first keystroke. Also verify that ArrowDown/ArrowUp/Enter while the dropdown is closed do not reopen it (intended), and that ArrowLeft/ArrowRight or Backspace navigation in normal text does reopen it (current behavior of the new onKeyDown wiring).
· Branch

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Codex Review

Mergeable, but should ideally address nits: Tab key opens filter dropdown

Findings

[P2] frontend/src/lib/components/FilterSearchbar.svelte:609onKeyDown={() => (open = true)} now opens the dropdown for every unhandled key from TaggedTextInput, not just typing. For example, pressing Tab while the filter input is focused opens the dropdown during keydown, then focus moves away; there is no blur/focusout close path, only svelte:window onmousedown, so the dropdown can remain visible after tabbing out. This is a keyboard navigation regression from this PR. Consider opening only for input-producing keys, or opening from the input/change path instead.

Test coverage

Frontend-only Svelte component change. This codebase does not generally expect component tests for Svelte UI wiring, so I’m not flagging missing automated tests.

Manual verification should cover the Runs page: loading the page autofocuses the filter searchbar, typing a printable character opens the filter dropdown, and keyboard navigation such as Tab does not leave the dropdown open after focus leaves the field.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Pi Review

Mergeable, but should ideally address nits: autofocus HTML attribute on contenteditable div may not autofocus on all browsers per spec, onKeyDown fires for non-typing keys like ArrowLeft/ArrowRight/Tab


P2 — Non-typing keys also trigger open = true via the new onKeyDown callback

File: frontend/src/lib/components/FilterSearchbar.svelte, line ~609

The new onKeyDown={() => (open = true)} fires on every keydown that TaggedTextInput doesn't internally consume, not just typing keys. TaggedTextInput only returns early for ArrowDown, ArrowUp, and Enter (line ~337), and for ArrowLeft/ArrowRight only when skipping an escape sequence (lines ~367–383). In all other cases — Tab, Escape, normal ArrowLeft/ArrowRight, Ctrl+A, Ctrl+Z, Backspace, etc. — the callback runs and sets open = true.

The FilterSearchbar's own window-level handleKeyDown (line 505) does handle Escape to close the dropdown, and Tab will naturally move focus away (triggering the onmousedown={() => (open = false)} on <svelte:window>). So there is no functional bug here — it just means the dropdown opens in response to navigation/editing keys, not only typing. This is more open-than-needed but not incorrect.

If you wanted the dropdown to open strictly on "typing" (printable characters), consider filtering in the callback — e.g. checking e.key.length === 1 or similar. This is a polish-level nit; the current behavior is harmless.


P2 — autofocus on a contenteditable div has no effect per the HTML spec

File: frontend/src/lib/components/TaggedTextInput.svelte, line ~526

The autofocus attribute is added to a <div contenteditable="true">. Per the HTML specification, the autofocus content attribute is only meaningful on form-associated elements (<input>, <select>, <textarea>, <button>). While some browsers may focus any element with tabindex and autofocus, this is not guaranteed and the spec does not require it for contenteditable divs.

If the intent is to auto-focus on page load, programmatic focus (e.g., contentEditableDiv.focus() in an onMount or $effect) would be more reliable. Alternatively, if the intent is just to pass through the attribute for styling or other purposes, no change is needed. The svelte-ignore a11y_autofocus comment is appropriate regardless.


Test coverage

Frontend only — no pure-logic utilities changed, so no unit tests expected. Svelte component tests are not required per codebase conventions.

Manual verification needed:

  • Navigate to the Runs page in a browser and verify that when the FilterSearchbar receives focus (via Tab or click) and the user starts typing, the filter dropdown opens automatically. Confirm that pressing Escape closes the dropdown, and that clicking outside also closes it. Verify that the dropdown does not open on arrow-key navigation (ArrowLeft/ArrowRight) within the search input — or confirm that opening on arrow keys is acceptable UX. Test in Chrome and Firefox.

@rubenfiszel rubenfiszel merged commit 73358c2 into main May 6, 2026
5 of 6 checks passed
@rubenfiszel rubenfiszel deleted the di/filter-searchbar-autofocus branch May 6, 2026 10:25
@github-actions github-actions Bot locked and limited conversation to collaborators May 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants