Skip to content

fix: focus keybindings search when opening Manage Shortcuts (FE-845)#12709

Open
LittleSound wants to merge 1 commit into
mainfrom
rizumu/fix/fe-845-focus-keybindings-search
Open

fix: focus keybindings search when opening Manage Shortcuts (FE-845)#12709
LittleSound wants to merge 1 commit into
mainfrom
rizumu/fix/fe-845-focus-keybindings-search

Conversation

@LittleSound

Copy link
Copy Markdown
Collaborator

Summary

Opening the Keybinding panel from the Manage Shortcuts button now focuses the Search Keybindings field instead of the Search Settings field.

Changes

  • What: The Settings dialog's "Search Settings" input had an unconditional autofocus, so opening directly to the keybinding panel always stole focus to the wrong field. Made it conditional (:autofocus="activeCategoryKey !== 'keybinding'") and added autofocus to the keybinding panel's own search input.

Review Focus

  • autofocus maps to the native attribute, which only fires on DOM insertion — flipping the reactive :autofocus while navigating between categories inside the dialog will not re-steal focus, so there is no regression for in-dialog navigation.
  • Added an E2E test verified in both directions: it fails on the original code (Search Settings focused) and passes with the fix (Search Keybindings focused).

Fixes FE-845

@LittleSound LittleSound requested a review from a team June 8, 2026 15:01
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jun 8, 2026
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b4ee1b79-4d33-4d69-a8d3-5c545ad96d71

📥 Commits

Reviewing files that changed from the base of the PR and between 74b9f16 and 9c0479f.

📒 Files selected for processing (3)
  • browser_tests/tests/bottomPanelShortcuts.spec.ts
  • src/components/dialog/content/setting/KeybindingPanel.vue
  • src/platform/settings/components/SettingDialog.vue

📝 Walkthrough

Walkthrough

This PR adjusts focus management in the settings dialog by making the general SearchInput autofocus conditional and ensuring the keybindings SearchInput receives focus when that panel is active. A test verifies the new focus behavior when navigating to keybindings via the shortcuts panel.

Changes

Keybinding Search Focus Management

Layer / File(s) Summary
Conditional autofocus for settings and keybindings inputs
src/platform/settings/components/SettingDialog.vue, src/components/dialog/content/setting/KeybindingPanel.vue
Settings dialog SearchInput now autofocuses conditionally—only when the active category is not 'keybinding'—while KeybindingPanel SearchInput gains an autofocus attribute to receive focus when the keybindings panel renders.
Test focus behavior verification
browser_tests/tests/bottomPanelShortcuts.spec.ts
New test confirms that opening the shortcuts panel and clicking manage shortcuts displays the settings dialog with keybindings search focused and settings search not focused.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A rabbit hops through focus flows,
Where keybindings' searchbox glows!
Settings bow when bindings rise—
Autofocus, smart and wise! 🎯✨

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing focus behavior for the keybindings search when opening Manage Shortcuts, with issue reference.
Description check ✅ Passed The description covers all required template sections: a clear summary, detailed changes with the specific fix and approach, and review focus with technical considerations. Issue reference included.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
End-To-End Regression Coverage For Fixes ✅ Passed PR uses bug-fix language ("fix:") and modifies src/ files. Critically, it includes an E2E test at browser_tests/tests/bottomPanelShortcuts.spec.ts, satisfying regression test requirements.
Adr Compliance For Entity/Litegraph Changes ✅ Passed Check applies only to litegraph, ECS, and graph entity changes. This PR modifies only settings dialog UI components and tests, not graph-related code.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rizumu/fix/fe-845-focus-keybindings-search

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🎨 Storybook: ✅ Built — View Storybook

Details

⏰ Completed at: 06/08/2026, 03:03:56 PM UTC

Links

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🎭 Playwright: ✅ 1666 passed, 0 failed · 1 flaky

📊 Browser Reports
  • chromium: View Report (✅ 1647 / ❌ 0 / ⚠️ 0 / ⏭️ 5)
  • chromium-2x: View Report (✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0)
  • chromium-0.5x: View Report (✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0)
  • mobile-chrome: View Report (✅ 16 / ❌ 0 / ⚠️ 1 / ⏭️ 0)

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@             Coverage Diff             @@
##             main   #12709       +/-   ##
===========================================
- Coverage   76.61%   61.33%   -15.29%     
===========================================
  Files        1561     1448      -113     
  Lines      105415    75006    -30409     
  Branches    32300    19527    -12773     
===========================================
- Hits        80765    46002    -34763     
- Misses      23827    28650     +4823     
+ Partials      823      354      -469     
Flag Coverage Δ
e2e ?
unit 61.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ponents/dialog/content/setting/KeybindingPanel.vue 0.00% <ø> (-66.17%) ⬇️
src/platform/settings/components/SettingDialog.vue 2.15% <ø> (-54.70%) ⬇️

... and 1154 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant