Skip to content

Fix prompt focus after keyboard model select - #94

Merged
touch2be merged 3 commits into
masterfrom
fix/model-selector-enter-focus
Jun 14, 2026
Merged

touch2be merged 3 commits into
masterfrom
fix/model-selector-enter-focus

Conversation

@touch2be

Copy link
Copy Markdown
Collaborator

Summary

  • Use Base UI dialog finalFocus to restore focus to the prompt textarea after closing the model selector
  • Fix Enter-based model selection focusing the trigger button instead of the prompt box

Checks

  • vp lint

@github-actions

Copy link
Copy Markdown

PR Review

Confidence Score: 5/5

Issues Detected

The change is correct and minimal. Here's what it fixes:

The old code used onCloseAutoFocus on DialogContent:

onCloseAutoFocus={(e) => {
  e.preventDefault();
  document.querySelector<HTMLTextAreaElement>('[data-slot="prompt-box-textarea"]')?.focus();
}}

This was dead code — the DialogContent wrapper at src/components/ui/dialog.tsx:53 destructures it out:

onCloseAutoFocus: _onCloseAutoFocus,

_onCloseAutoFocus is never used and never passed through to DialogPrimitive.Popup via ...props. So the handler was silently swallowed and focus always returned to the trigger button.

The fix uses Base UI's finalFocus prop, which correctly passes through ...props to DialogPrimitive.Popup and tells the dialog where to return focus after closing.

Suggestions (minor)

  • Consider whether the DialogContent wrapper should stop stripping onCloseAutoFocus, since it's a valid Base UI prop that callers might reasonably want to use. Currently it silently discards it.
  • The finalFocus callback creates a new function reference on every render; this is fine for finalFocus since Base UI calls it only once (on close), but could be memoized with useCallback if desired for consistency with other handlers in the component.

New%20session%20-%202026-06-14T01%3A15%3A28.090Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

PR Review

Confidence Score: 5/5

The fix correctly addresses the focus issue. The root cause was that DialogContent at src/components/ui/dialog.tsx:53 destructures and discards onCloseAutoFocus:

onCloseAutoFocus: _onCloseAutoFocus,

The old onCloseAutoFocus handler in MessageList.tsx was dead code — never reaching DialogPrimitive.Popup. The finalFocus prop, however, passes through via ...props to Popup, making it work correctly.

Issues Detected

  1. dead prop type in DialogContent — onCloseAutoFocus is typed in the destructured props but silently discarded. If another consumer tries using it, they'll hit the same bug. Consider either wiring it through to DialogPrimitive.Popup or removing it from the type signature.

  2. unnecessary function allocation — finalFocus callback is recreated every render. While Base UI typically only invokes it once on close and this won't cause bugs, wrapping it in useCallback (or lifting the querySelector to a ref) would be more consistent with the component's existing patterns.

  3. selector fragility — The data-slot attribute selector is shared with other dialog components. If the textarea element is ever unmounted before the dialog finishes closing, focus will fall back to the trigger (acceptable behavior, but worth noting). A ref-based approach would be more robust.

  4. missing i18n keys in DE/ES (minor) — The PR adds 6 lines to each locale file, but de.json and es.json already had slightly different keys (e.g., restartServer vs restartLocalBackend) suggesting prior drift. New keys should be verified for correctness in all three languages.

Summary

The fix is correct and minimal. finalFocus is the idiomatic Base UI API for this use case. The underlying DialogContent prop-stripping issue is pre-existing and outside the scope of this PR.

New%20session%20-%202026-06-14T01%3A19%3A23.318Z
opencode session  |  github run

@github-actions

Copy link
Copy Markdown

PR Review

Confidence Score: 5/5

Analysis

The review is accurate. I verified both files in the current state.

Root cause: src/components/ui/dialog.tsx:53 explicitly destructures and discards onCloseAutoFocus:

onCloseAutoFocus: _onCloseAutoFocus,

The _onCloseAutoFocus variable is never referenced again and ...props (which is spread onto DialogPrimitive.Popup at line 68) no longer contains it. So the old handler was silently dead code. Focus always returned to the trigger button after dialog close.

The fix (src/components/ModelSelector.tsx:436-438) uses Base UI's finalFocus prop instead, which flows through ...props to DialogPrimitive.Popup correctly:

finalFocus={() =>
  document.querySelector<HTMLTextAreaElement>('[data-slot="prompt-box-textarea"]')
}

Additional observations

  • The finalFocus reference is recreated every render, but Base UI only reads it on close, so this is not a concern.
  • The DialogContent wrapper at dialog.tsx:53 should consider forwarding onCloseAutoFocus (or document that it's intentionally stripped), since it's a valid Base UI prop that other callers might reasonably use.
  • The data-slot query selector approach is consistent with the existing codebase conventions for DOM targeting.

New%20session%20-%202026-06-14T01%3A20%3A02.252Z
opencode session  |  github run

@touch2be
touch2be merged commit 62f8270 into master Jun 14, 2026
1 check passed
@akemmanuel
akemmanuel deleted the fix/model-selector-enter-focus branch October 2, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant