Skip to content

fix(web): focus settings search with command-f - #17859

Open
extoci wants to merge 1 commit into
pingdotgg:mainfrom
extoci:t3/settings-cmd-f-search
Open

extoci wants to merge 1 commit into
pingdotgg:mainfrom
extoci:t3/settings-cmd-f-search

Conversation

@extoci

@extoci extoci commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Problem

Cmd+F did not focus Settings search. Most sections fell through to the browser's Find command, while Keybindings focused its separate filter.

Change

Handle Cmd+F and Ctrl+F in the shared Settings sidebar, opening it when collapsed and focusing/selecting the search query. Remove the competing Keybindings handler so every section uses the same search. Preserve / search, dialog focus traps, and shortcut recording.

Scope and approval

This is a small, focused fix for the standard Find shortcut on the existing Settings search. It changes two web components shared by web and desktop, with no server, provider, contract, or native mobile changes.

Verification

  • The requester tested the running web app over LAN and confirmed it worked.
  • vp test run apps/web/src/components/settings/KeybindingsSettings.environment.test.tsx apps/web/src/components/settings/settingsSearch.test.ts: 64 tests passed.
  • Web vp run typecheck: passed.
  • Targeted lint and formatting checks passed, with one existing react/set-state-in-effect warning in SettingsSidebarNav.

Built with GPT-6.1 Sol through the Codex harness.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 10, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 0534b64

Macroscope's review found this PR approvable — This is a focused settings-search bug fix that consolidates Cmd/Ctrl+F handling into the existing sidebar search and removes a duplicate keybindings handler. The change is limited to two web components and preserves dialog, popup, composition, and shortcut-recording focus protections.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 098fc928-c3da-401f-ba15-7872f882c72f

📥 Commits

Reviewing files that changed from the base of the PR and between cfbd145 and 0534b64.


📒 Files selected for processing (2)
  • apps/web/src/components/settings/KeybindingsSettings.tsx
  • apps/web/src/components/settings/SettingsSidebarNav.tsx

💤 Files with no reviewable changes (1)
  • apps/web/src/components/settings/KeybindingsSettings.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

The settings sidebar now handles / and Ctrl/Cmd+F search shortcuts. The keybindings settings page no longer installs its own Ctrl/Cmd+F listener.

Changes

Settings Search Shortcuts

Layer / File(s) Summary
Handle settings search shortcuts
apps/web/src/components/settings/KeybindingsSettings.tsx, apps/web/src/components/settings/SettingsSidebarNav.tsx
The keybindings settings page removes its search shortcut listener. The sidebar handles / and Ctrl/Cmd+F, with checks for prevented or composing events and specified target elements.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge


Merge Risk: ⚪ Minimal · up to 0534b

Settings search shortcuts remain available across settings, including while the sidebar is collapsed, without intercepting shortcut recording. No material merge-blocking risk remains.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly describes the primary change: focusing the Settings search with the command-F shortcut.
Description check Passed The description covers the problem, change, scope, approval rationale, verification steps, test results, and agent attribution. It is mostly complete, although it does not include the template-request…
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.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

This branch has not been deployed

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

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant