feat: add search back to settings#2468
Conversation
Greptile SummaryAdds a search box to the Settings page (Cmd+F to focus) and wires settings search into the command palette. The implementation extracts tab/section metadata into a plain-data config (
Confidence Score: 5/5Safe to merge — all changed paths are additive UI features with no data mutations or side effects. The change is purely additive: new search UI, new helper modules, and a command palette extension. No existing logic is altered and the new code paths all degrade gracefully (empty query = original tab view, no matches = empty message). The two noteworthy patterns (index-based section spreading and a dead displayedTab fallback) are maintenance concerns rather than current breakages. settings-page-content.tsx and settings-search.ts have the patterns worth watching on future edits.
|
| Filename | Overview |
|---|---|
| apps/emdash-desktop/src/renderer/features/settings/components/settings-page-config.tsx | New file: plain-data config (no JSX) extracted from SettingsPage — tabs, section search metadata, and type definitions. Clean separation from component code. |
| apps/emdash-desktop/src/renderer/features/settings/components/settings-search.ts | New file: search logic for settings — getSettingsSearchView (in-page) and searchSettings (command palette). The displayedTab fallback to firstMatchingTab is dead code since its consumer is guarded by !normalizedQuery. searchSettings can emit redundant tab + section results for the same destination. |
| apps/emdash-desktop/src/renderer/features/settings/components/settings-page-content.tsx | New file: React component tree for settings tabs, composed by spreading config entries by array index. Index-based coupling means config/content arrays can silently diverge if sections are reordered or inserted. |
| apps/emdash-desktop/src/renderer/features/settings/components/SettingsPage.tsx | Refactored to consume new search/content modules; adds search input, live region for a11y, match-count badges, cross-tab grouped results view, and highlightMatch utility. Logic is clean and the conditional render paths are correct. |
| apps/emdash-desktop/src/renderer/lib/ui/search-input.tsx | Adds onClear, clearLabel, and selectOnHotkey props; Escape-to-clear handler stops propagation correctly; clear button is accessible. No issues. |
| apps/emdash-desktop/src/renderer/features/command-palette/command-palette-modal.tsx | Adds a settingsResults memo that calls searchSettings, capped at 6 results, rendered as a Settings group in the palette. Integration is minimal and correct. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User types in Settings search] --> B{normalizedQuery?}
B -- No --> C[Show active tab content\ndisplayedContent.sections]
B -- Yes --> D[getSettingsSearchView\nbuilds resultGroups]
D --> E{resultGroups.length > 0?}
E -- Yes --> F[Render grouped results\nacross all tabs]
E -- No --> G[Show no-match message]
H[User types in Command Palette] --> I[searchSettings called\nvia useMemo]
I --> J[Match tabs + sections\nfrom settings-page-config]
J --> K[Slice top 6, render\nSettings group in palette]
K --> L[navigate settings + tab\non select]
subgraph config["settings-page-config.tsx (plain data)"]
M[settingsSearchContent\nsettingsTabs]
end
subgraph content["settings-page-content.tsx (JSX)"]
N[settingsTabContent\ncomponent per section]
end
subgraph search["settings-search.ts"]
O[getSettingsSearchView\nsearchSettings]
end
config --> search
config --> content
search --> F
content --> F
search --> K
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/emdash-desktop/src/renderer/features/settings/components/settings-page-content.tsx:46-170
**Index-based spreading couples config and content arrays by position**
Each `SectionConfig` entry here gets its `id` and `searchText` by spreading the config entry at the same array index. If someone later inserts a new section into `settingsSearchContent.*.sections` at position `N` (in `settings-page-config.tsx`) without inserting the matching component at the same position here, every subsequent entry's `id` shifts one slot — causing the `find(candidate => candidate.id === section.id)` lookup in `SettingsPage.tsx` to silently return `null` for those sections and drop them from search results. A map keyed by section `id` rather than parallel index would make the coupling explicit and impossible to misalign.
### Issue 2 of 3
apps/emdash-desktop/src/renderer/features/settings/components/settings-search.ts:83-89
**`displayedTab` fallback to `firstMatchingTab` is never observed**
The `firstMatchingTab` path sets `displayedTab` to the first tab with matches when the active tab has none and a `normalizedQuery` is present. But every consumer of `displayedTab` in `SettingsPage.tsx` — both `displayedContent` and the `isActive` nav check — is guarded by `!normalizedQuery`, so this fallback value is always overridden by the query being active. The cross-tab jump described in the comment never actually fires; the `resultGroups` path handles that instead.
### Issue 3 of 3
apps/emdash-desktop/src/renderer/features/settings/components/settings-search.ts:125-148
**Tab-header hit produces a redundant result alongside section hits for the same tab**
When `tabHeaderMatchesSearch` is true, `searchSettings` pushes a tab-level entry (`settings:${tab.id}`, score 2), then continues into the section loop and pushes every individual section (`settings:${tab.id}:${section.id}`, score 1) because `sectionMatchesSearch` is evaluated independently. For a query like `"account"`, the palette receives "Account Settings" (tab) and "Account" (section) — two entries that both `navigate('settings', { tab: 'account' })`. If this is intentional, the tab-level entry could be skipped when sections already cover the same destination.
Reviews (2): Last reviewed commit: "fix(settings): decouple search config" | Re-trigger Greptile
Description
Screenshot/Recording (if applicable)
https://streamable.com/a56grz
Checklist
messages and, when possible, the PR title