Add search functionality to sidebar by dadahsueh · Pull Request #42 · zerebos/ghostty-config

dadahsueh · 2026-03-03T19:44:01Z

Add Search component with Cmd/Ctrl+K shortcut
Add HighlightText component for matching text display
Add SearchSubItem component for individual setting results
Add search store for query state and highlighted setting
Add search utility for filtering settings and external tabs
Update Tab component to support highlighted text rendering
Update sidebar to show search results with keyboard navigation
Add scroll-to-setting with highlight animation on navigation
Remove dynamic buffer height from Page component

Implemented with AI assistance.

Added because it could be hard to find one particular config. Looks something like this:

* Add Search component with Cmd/Ctrl+K shortcut * Add HighlightText component for matching text display * Add SearchSubItem component for individual setting results * Add search store for query state and highlighted setting * Add search utility for filtering settings and external tabs * Update Tab component to support highlighted text rendering * Update sidebar to show search results with keyboard navigation * Add scroll-to-setting with highlight animation on navigation * Remove dynamic buffer height from Page component Implemented with AI assistance.

zerebos · 2026-03-04T00:17:51Z

Having copilot's review take a first crack at it, but at a glance there are a number of issues with the way this is implemented.

Copilot

Pull request overview

Adds a sidebar search experience to help users quickly find settings (and a couple external links), including match highlighting, keyboard navigation, and scroll-to-setting with a highlight animation after navigation.

Changes:

Introduces a search store + search utilities to filter categories/settings and compute highlight ranges.
Adds Search / SearchSubItem / HighlightText components and updates sidebar rendering + keyboard navigation behavior.
Adds scroll-to-highlight behavior on the settings category page and adjusts Page layout scrolling spacing.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/routes/settings/[category]/+page.svelte	Scroll-to-setting + highlight flash behavior when navigating from search results.
src/routes/+layout.svelte	Renders Search in sidebar, swaps nav content for results, and adds keyboard navigation / selection behavior.
src/lib/views/Page.svelte	Removes dynamic buffer height behavior and updates content container spacing.
src/lib/utils/search.ts	New search/filter + match-range generation utilities and flattening for rendering.
src/lib/stores/search.svelte.ts	New shared search state (query, selectedIndex, highlightedSettingId) + helpers.
src/lib/components/Tab.svelte	Adds optional match highlighting rendering via HighlightText.
src/lib/components/SearchSubItem.svelte	Renders individual matched setting rows and navigates + sets highlight on click.
src/lib/components/Search.svelte	Adds search input with Cmd/Ctrl+K focus shortcut and clear behavior.
src/lib/components/HighlightText.svelte	Renders highlighted substrings based on computed match ranges.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/lib/utils/search.ts

src/routes/+layout.svelte

src/routes/settings/[category]/+page.svelte

src/lib/utils/search.ts

src/routes/+layout.svelte

src/lib/components/Search.svelte

…mprovements) Copilot-suggested fixes: - Reset and clamp search.selectedIndex when query/results change - Add visual selected state for category/external search results - Fix race condition in scrollToHighlighted timeout - Normalize query with trim() for consistent matching - Handle external navigation with globalThis.open() - Gate keyboard navigation to search/sidebar focus - Reset selectedIndex when clearing search - Add 73 comprehensive unit tests for search utilities Additional UX/accessibility improvements: - Add scroll-into-view for keyboard navigation - Add role/listbox accessibility attributes to search results - Fix focus management after selection (restore to search input) - Add Escape key handler to return focus to search input - Add platform-aware keyboard shortcut hint (⌘K on Mac, Ctrl+K on Windows/Linux) - Fix HighlightText color to use theme variable for light theme support - Consolidate reactivity effects to prevent infinite loops

dadahsueh · 2026-03-04T15:58:07Z

Not really familiar with how to deal with copilot, can copilot have another go at it? Made some changes. Also, do you think this is a useful feature to have?

zerebos · 2026-03-04T18:05:15Z

You said:

Implemented with AI assistance.

Realistically can you estimate how much of this PR was AI generated or driven?

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-04T18:10:47Z

src/routes/+layout.svelte

+                globalThis.open(route, "_blank", "noopener noreferrer");
+            } else {
+                await goto(route);
+            }
+        } else {
+            if (isExternal) {
+                globalThis.open(route, "_blank", "noopener noreferrer");


globalThis.open(route, "_blank", "noopener noreferrer") is not a valid feature string for reliably enabling noopener/noreferrer (features are comma-separated). This can leave window.opener intact, which is a security risk for external links. Use a comma-separated feature string (e.g. noopener,noreferrer) or otherwise ensure opener is nullified.

Suggested change

globalThis.open(route, "_blank", "noopener noreferrer");

} else {

await goto(route);

}

} else {

if (isExternal) {

globalThis.open(route, "_blank", "noopener noreferrer");

globalThis.open(route, "_blank", "noopener,noreferrer");

} else {

await goto(route);

}

} else {

if (isExternal) {

globalThis.open(route, "_blank", "noopener,noreferrer");

Copilot · 2026-03-04T18:10:47Z

src/routes/+layout.svelte

+                                        route={item.result.categoryRoute}
+                                        highlightRanges={item.result.categoryMatchRanges}
+                                        label={item.result.categoryName}
+                                        selected={index === search.selectedIndex}


In the search results list, category/external items render a <Tab> anchor without any click handler, so clicking a result won’t call clearSearch() (unlike keyboard Enter which goes through selectItem() and clears). Consider routing clicks through the same selection handler (prevent default + call selectItem(index)), or provide a dedicated result item component that clears search consistently.

Suggested change

selected={index === search.selectedIndex}

selected={index === search.selectedIndex}

on:click|preventDefault={() => selectItem(index)}

Copilot · 2026-03-04T18:10:47Z

src/routes/settings/[category]/+page.svelte

+                    element.classList.remove("highlight-flash");
+                    clearHighlightedSetting();
+                }
+            }, 2000);


If the highlighted setting element cannot be found (e.g. stale highlightedSettingId or navigating to the wrong category), highlightedSettingId is never cleared, so the app can remain in a "pending highlight" state across pages. Consider clearing the highlighted id when the element isn’t found after the post-render wait (or adding a bounded retry).

Suggested change

}, 2000);

}, 2000);

} else {

clearHighlightedSetting();

Copilot · 2026-03-04T18:10:47Z

src/routes/settings/[category]/+page.svelte

+    let highlightTimeout: number | null = null;
+
+    function smoothScrollTo(element: HTMLElement, duration: number = 300) {
+        const container = document.querySelector(".content-container") as HTMLElement;
+        if (!container) return;
+
+        const containerRect = container.getBoundingClientRect();
+        const elementRect = element.getBoundingClientRect();
+        const scrollTop = container.scrollTop;
+        const elementTop =
+            elementRect.top -
+            containerRect.top +
+            scrollTop -
+            containerRect.height / 2 +
+            elementRect.height / 2;
+        const targetScroll = Math.max(0, elementTop);
+        const startTime = performance.now();
+        const startScroll = container.scrollTop;
+
+        function animate(currentTime: number) {
+            const elapsed = currentTime - startTime;
+            const progress = Math.min(elapsed / duration, 1);
+            const easeProgress = 1 - Math.pow(1 - progress, 3);
+            container.scrollTop = startScroll + (targetScroll - startScroll) * easeProgress;
+
+            if (progress < 1) {
+                requestAnimationFrame(animate);
+            }
+        }
+
+        requestAnimationFrame(animate);
+    }
+
+    async function scrollToHighlighted() {
+        const highlightedId = search.highlightedSettingId;
+        if (!highlightedId) return;
+
+        if (highlightTimeout !== null) {
+            clearTimeout(highlightTimeout);
+        }


highlightTimeout is cleared when a new highlight starts, but it isn’t cleared on component teardown. If the user navigates away quickly, the timeout callback can still run later and mutate the global search store unexpectedly. Consider clearing the timeout in an onDestroy cleanup (and optionally removing the highlight class if needed).

Copilot · 2026-03-04T18:10:48Z

src/routes/+layout.svelte

+                globalThis.open(route, "_blank", "noopener noreferrer");
+            } else {
+                await goto(route);
+            }
+        } else {
+            if (isExternal) {
+                globalThis.open(route, "_blank", "noopener noreferrer");


Same issue as above: the window.open feature string uses spaces instead of commas, so noopener/noreferrer may not be applied and window.opener may remain accessible.

Suggested change

globalThis.open(route, "_blank", "noopener noreferrer");

} else {

await goto(route);

}

} else {

if (isExternal) {

globalThis.open(route, "_blank", "noopener noreferrer");

globalThis.open(route, "_blank", "noopener,noreferrer");

} else {

await goto(route);

}

} else {

if (isExternal) {

globalThis.open(route, "_blank", "noopener,noreferrer");

Copilot · 2026-03-04T18:10:48Z

src/lib/utils/search.ts

+                if (hasMatch(setting.name, q) || (setting.note && hasMatch(setting.note, q))) {
+                    matchedSettings.push({
+                        id: setting.id,
+                        name: setting.name,
+                        matchRanges: getMatchRanges(setting.name, q)


Settings matched via setting.note are added to results, but matchRanges is always computed from setting.name. When the query only appears in the note (not the name), the UI will show a match with no highlighted text. Either compute/display highlight ranges for the note as well, or only include note matches when the name also matches.

Suggested change

if (hasMatch(setting.name, q) || (setting.note && hasMatch(setting.note, q))) {

matchedSettings.push({

id: setting.id,

name: setting.name,

matchRanges: getMatchRanges(setting.name, q)

const nameMatchRanges = getMatchRanges(setting.name, q);

if (nameMatchRanges.length > 0) {

matchedSettings.push({

id: setting.id,

name: setting.name,

matchRanges: nameMatchRanges

Copilot · 2026-03-04T18:10:48Z

src/routes/+layout.svelte

+                    <div
+                        bind:this={searchNavContainer}
+                        role="listbox"
+                        aria-label="Search results"
+                        class="search-results-list"
+                    >
+                        {#each flattenedResults as item, index (`${item.result.categoryId}-${item.type}-${item.setting?.id || ""}`)}
+                            {#if item.type === "category" || item.type === "external"}
+                                <div
+                                    class="search-result-group"
+                                    role="option"
+                                    aria-selected={index === search.selectedIndex}
+                                >
+                                    <Tab
+                                        route={item.result.categoryRoute}


ARIA pattern mismatch: the container is role="listbox" with children role="option", but the category/external option contains an interactive <a> (via <Tab>). Listbox options should not generally contain nested interactive controls. Prefer making the option itself the interactive element (e.g., a single button with role="option"), or switch to a more appropriate pattern (e.g. role="menu"/menuitem) and manage focus/selection accordingly.

Copilot · 2026-03-04T18:10:48Z

src/lib/components/Search.svelte

+    function handleInputKeydown(e: KeyboardEvent) {
+        if (e.key === "Escape") {
+            e.preventDefault();
+            clearSearch();
+            inputRef?.blur();
+        }
+    }


Escape handling is currently contradictory: this input handler clears search and blurs the field, but the window-level handler in +layout.svelte also handles Escape while the search input is active and then refocuses it. Consolidate Escape handling in one place (or make the window handler ignore Escape when the active element is the search input) to avoid flicker/inconsistent focus behavior.

Copilot · 2026-03-04T18:10:49Z

src/lib/components/Search.svelte

+    function handleKeydown(e: KeyboardEvent) {
+        if ((e.metaKey || e.ctrlKey) && e.key === "k") {
+            e.preventDefault();
+            inputRef?.focus();
+        }
+    }


The Cmd/Ctrl+K shortcut check uses e.key === "k", which can fail with different keyboard layouts or when Shift is held (key may be "K"). Prefer e.code === "KeyK" or normalizing with e.key.toLowerCase() === "k" for robustness.

dadahsueh · 2026-03-05T08:46:54Z

You said:

Implemented with AI assistance.

Realistically can you estimate how much of this PR was AI generated or driven?

Probably now 80% AI generated, so more like Implemented via vibe coding. Will go through the changes again.

zerebos requested a review from Copilot March 4, 2026 00:17

Copilot started reviewing on behalf of zerebos March 4, 2026 00:17 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

zerebos requested a review from Copilot March 4, 2026 18:04

Copilot started reviewing on behalf of zerebos March 4, 2026 18:04 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

zerebos mentioned this pull request Mar 9, 2026

Make settings searchable + quick switcher #56

Open

	selected={index === search.selectedIndex}
	selected={index === search.selectedIndex}
	on:click\|preventDefault={() => selectItem(index)}

Conversation

dadahsueh commented Mar 3, 2026

Uh oh!

zerebos commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dadahsueh commented Mar 4, 2026

Uh oh!

zerebos commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

dadahsueh commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants