address Copilot review on #109

Six findings (4 inline + 2 in the review body, all valid):

1. `clear()` in header-search.js fired BOTH `filter.onInput('')` AND
   `filter.onClear()`. Most gears were re-filtering twice; any side-
   effectful onClear would run on top of the empty-input path. Pick
   one path: prefer onClear if registered, otherwise dispatch empty
   onInput. Removed the now-unused dispatchClear helper.

2. toggleSidebar() / applySidebarState() only updated the hidden
   `#sidebar-icon-{collapse,expand}` shim left behind in the sidebar
   for legacy code. The VISIBLE icons live in the new header toggle
   (`#sidebar-icon-{collapse,expand}-header`) and were left untouched,
   so clicking the toggle moved the sidebar but the icon never
   swapped. Mirror the state to both pairs.

3. The `?` shortcut overlay is `role="dialog" aria-modal="true"` but
   open() just removed `hidden` — focus stayed wherever it was, so
   the dialog wasn't announced and keyboard focus could land outside
   it. Make the overlay focusable on open and move focus into it;
   restore focus to the prior element on close. No formal focus
   trap is needed since the overlay has no interactive controls
   (clicking anywhere closes).

4. The Overview gear restored the saved text filter by writing
   __backendFilterQuery directly THEN calling HeaderSearch.setValue,
   BEFORE registering the filter — so the bridge would never carry
   restored state through the registered onInput. Swap the order:
   register first, then setValue, and let the dispatch update the
   query variable through the same code path keystrokes use.

5. The outside-click handler that collapses the compact mobile
   search row checked `row.classList.contains('hidden')` to bail
   when closed — but the row's templ markup is `hidden md:flex`, so
   it ALWAYS carries `hidden` regardless of overlay state. The check
   effectively disabled the feature. Use the .header-search-expanded
   marker class instead, which the overlay state actually toggles.

6. The HeaderSearch templ doc-comment still described the original
   "registered placeholder switches the input's placeholder" plan;
   we dropped that earlier so the box reads the same on every page.
   Updated the comment to match.

Verified in the browser: toggling the sidebar swaps the visible
header icon; clear() with both callbacks registered fires onClear
exactly once and onInput zero times; opening the help overlay moves
focus onto it.

Refs #92

Author: sarg3nt

gearbox/internal/framework/templates/layouts/base.templ

@@ -892,27 +892,41 @@ templ Base(title string, user *models.User, currentPath ...string) {
 				// Header now spans full viewport width and lives ABOVE the
 				// sidebar (issue #92), so it no longer tracks sidebar width.
 				// We only flip the sidebar/main-content classes here.
+				// Two pairs of icon elements need to be kept in sync:
+				//   - `sidebar-icon-{collapse,expand}` — hidden shim
+				//     inside the sidebar, kept so legacy lookups still
+				//     resolve to something.
+				//   - `sidebar-icon-{collapse,expand}-header` — the
+				//     VISIBLE icons inside the new header toggle.
+				// Updating only the shim left the visible icon stuck
+				// (Copilot review).
 				const sidebar = document.getElementById('sidebar');
 				const mainContent = document.getElementById('main-content');
 				const iconCollapse = document.getElementById('sidebar-icon-collapse');
 				const iconExpand = document.getElementById('sidebar-icon-expand');
+				const iconCollapseHeader = document.getElementById('sidebar-icon-collapse-header');
+				const iconExpandHeader = document.getElementById('sidebar-icon-expand-header');
 				const isCollapsed = sidebar.classList.contains('sidebar-collapsed');
 
 				if (isCollapsed) {
 					sidebar.classList.remove('sidebar-collapsed');
 					sidebar.classList.add('sidebar-expanded');
 					mainContent.classList.remove('ml-16');
 					mainContent.classList.add('ml-64');
-					if (iconCollapse) iconCollapse.classList.remove('hidden');
-					if (iconExpand) iconExpand.classList.add('hidden');
+					if (iconCollapse)       iconCollapse.classList.remove('hidden');
+					if (iconExpand)         iconExpand.classList.add('hidden');
+					if (iconCollapseHeader) iconCollapseHeader.classList.remove('hidden');
+					if (iconExpandHeader)   iconExpandHeader.classList.add('hidden');
 					localStorage.setItem('sidebarCollapsed', 'false');
 				} else {
 					sidebar.classList.add('sidebar-collapsed');
 					sidebar.classList.remove('sidebar-expanded');
 					mainContent.classList.add('ml-16');
 					mainContent.classList.remove('ml-64');
-					if (iconCollapse) iconCollapse.classList.add('hidden');
-					if (iconExpand) iconExpand.classList.remove('hidden');
+					if (iconCollapse)       iconCollapse.classList.add('hidden');
+					if (iconExpand)         iconExpand.classList.remove('hidden');
+					if (iconCollapseHeader) iconCollapseHeader.classList.add('hidden');
+					if (iconExpandHeader)   iconExpandHeader.classList.remove('hidden');
 					localStorage.setItem('sidebarCollapsed', 'true');
 				}
 			}
@@ -923,6 +937,10 @@ templ Base(title string, user *models.User, currentPath ...string) {
 				const mainContent = document.getElementById('main-content');
 				const iconCollapse = document.getElementById('sidebar-icon-collapse');
 				const iconExpand = document.getElementById('sidebar-icon-expand');
+				// Visible header-button icons — see toggleSidebar()
+				// for context on why both pairs are tracked.
+				const iconCollapseHeader = document.getElementById('sidebar-icon-collapse-header');
+				const iconExpandHeader = document.getElementById('sidebar-icon-expand-header');
 
 				// IMPORTANT: apply the correct sidebar/main-content classes
 				// BEFORE removing the .sidebar-initially-collapsed pin from
@@ -945,15 +963,19 @@ templ Base(title string, user *models.User, currentPath ...string) {
 						sidebar.classList.remove('sidebar-expanded');
 						mainContent.classList.add('ml-16');
 						mainContent.classList.remove('ml-64');
-						if (iconCollapse) iconCollapse.classList.add('hidden');
-						if (iconExpand) iconExpand.classList.remove('hidden');
+						if (iconCollapse)       iconCollapse.classList.add('hidden');
+						if (iconExpand)         iconExpand.classList.remove('hidden');
+						if (iconCollapseHeader) iconCollapseHeader.classList.add('hidden');
+						if (iconExpandHeader)   iconExpandHeader.classList.remove('hidden');
 					} else {
 						sidebar.classList.remove('sidebar-collapsed');
 						sidebar.classList.add('sidebar-expanded');
 						mainContent.classList.remove('ml-16');
 						mainContent.classList.add('ml-64');
-						if (iconCollapse) iconCollapse.classList.remove('hidden');
-						if (iconExpand) iconExpand.classList.add('hidden');
+						if (iconCollapse)       iconCollapse.classList.remove('hidden');
+						if (iconExpand)         iconExpand.classList.add('hidden');
+						if (iconCollapseHeader) iconCollapseHeader.classList.remove('hidden');
+						if (iconExpandHeader)   iconExpandHeader.classList.add('hidden');
 					}
 				}
 
@@ -2473,12 +2495,13 @@ templ Header(currentPath string) {
 
 // HeaderSearch is the unified search / filter / command-palette input.
 // One text input drives three behaviours, chosen by what the user types:
-//   - empty / search mode: hint reads "Type / to search · Cmd+K for palette".
-//     Per-gear pages can register a filter callback via window.gearbox.filter
-//     (see common/gear-commands.js); when active, the placeholder switches
-//     to whatever the page asked for and each keystroke calls the callback.
-//     With no filter callback registered, Enter falls back to a DuckDuckGo
-//     web search.
+//   - empty / search mode: the same hint chips appear on every page —
+//     "/ to search · Cmd+K for palette" — overlaid on the input. There
+//     is no per-gear placeholder; gears that want to react to keystrokes
+//     register a filter callback via window.gearbox.filter (see
+//     common/gear-commands.js) and the controller dispatches each input
+//     event to it. With no filter callback registered, Enter falls back
+//     to a DuckDuckGo web search.
 //   - leading `>` (typed manually or inserted by Cmd+K): palette mode.
 //     The slide-down `#header-search-panel` opens just below the input and
 //     ranks boxes / gears / settings pages / global actions / per-gear

gearbox/internal/framework/templates/pages/overview.templ

@@ -572,22 +572,10 @@ templ Overview(user *models.User, servers []models.BoxConfig) {
 			// Move page header content to main header
 			setupPageHeader();
 
-			// Restore global filters from localStorage. The text query
-			// lives in __backendFilterQuery now (issue #92); the health
-			// select is still a per-page dropdown.
-			const savedTextFilter = localStorage.getItem('global-text-filter') || '';
-			const savedHealthFilter = localStorage.getItem('global-health-filter') || 'all';
-			const healthSelect = document.getElementById('global-health-filter');
-
-			if (savedTextFilter) {
-				__backendFilterQuery = savedTextFilter.toLowerCase();
-				if (window.HeaderSearch) window.HeaderSearch.setValue(savedTextFilter);
-			}
-			if (healthSelect && savedHealthFilter) {
-				healthSelect.value = savedHealthFilter;
-			}
-
 			// HeaderSearch + command palette wiring (issue #92).
+			// Register BEFORE replaying the saved filter so setValue()'s
+			// dispatch flows through the registered onInput and keeps
+			// __backendFilterQuery in sync (Copilot review).
 			if (window.gearbox && window.gearbox.filter) {
 				window.gearbox.filter.register({
 					placeholder: 'Filter backends…',
@@ -598,6 +586,20 @@ templ Overview(user *models.User, servers []models.BoxConfig) {
 					onClear: function () { __backendFilterQuery = ''; applyGlobalFilters(); },
 				});
 			}
+
+			// Restore global filters from localStorage. The text query
+			// flows through HeaderSearch → registered onInput now; the
+			// health select is still a per-page dropdown.
+			const savedTextFilter = localStorage.getItem('global-text-filter') || '';
+			const savedHealthFilter = localStorage.getItem('global-health-filter') || 'all';
+			const healthSelect = document.getElementById('global-health-filter');
+
+			if (savedTextFilter && window.HeaderSearch) {
+				window.HeaderSearch.setValue(savedTextFilter);
+			}
+			if (healthSelect && savedHealthFilter) {
+				healthSelect.value = savedHealthFilter;
+			}
 			if (window.gearbox && window.gearbox.commands) {
 				const cmds = window.gearbox.commands;
 				const hSel = document.getElementById('global-health-filter');

gearbox/static/js/common/header-search.js

@@ -181,14 +181,6 @@
         clear();
     }
 
-    function dispatchClear() {
-        const filter = window.gearbox && window.gearbox.filter && window.gearbox.filter.current();
-        if (filter && typeof filter.onClear === 'function') {
-            try { filter.onClear(); }
-            catch (e) { console.error('filter.onClear threw', e); }
-        }
-    }
-
     /* -------------------------------------------------------------- *
      * Public mode / value mutators
      * -------------------------------------------------------------- */
@@ -218,8 +210,22 @@
         if (!input) return;
         input.value = '';
         repaint();
-        dispatchSearchInput();
-        dispatchClear();
+        // Fire exactly one filter callback. Earlier this called both
+        // dispatchSearchInput() (→ filter.onInput('')) AND
+        // dispatchClear() (→ filter.onClear()), so most gears
+        // re-filtered twice and any side-effectful onClear would
+        // run alongside the empty-query path (Copilot review).
+        // Prefer onClear if the gear registered one; otherwise fall
+        // back to the standard empty-query dispatch.
+        notify(queryListeners, '');
+        const filter = window.gearbox && window.gearbox.filter && window.gearbox.filter.current();
+        if (filter && typeof filter.onClear === 'function') {
+            try { filter.onClear(); }
+            catch (e) { console.error('filter.onClear threw', e); }
+        } else if (filter && typeof filter.onInput === 'function') {
+            try { filter.onInput(''); }
+            catch (e) { console.error('filter.onInput threw', e); }
+        }
     }
 
     function focus(selectAll) {
@@ -339,9 +345,15 @@
         }
 
         // Compact-mode: clicking outside the search collapses the row.
+        // Check the .header-search-expanded marker rather than the
+        // `hidden` class — the row's templ markup is `hidden md:flex`,
+        // so it ALWAYS carries `hidden` and the old check effectively
+        // disabled the outside-click collapse (Copilot review). The
+        // overlay state is driven by the marker class, not by toggling
+        // `hidden` directly.
         document.addEventListener('click', function (e) {
             if (!window.matchMedia('(max-width: 767px)').matches) return;
-            if (!row || row.classList.contains('hidden')) return;
+            if (!row || !row.classList.contains('header-search-expanded')) return;
             if (wrap && wrap.contains(e.target)) return;
             // Don't collapse while the palette panel is the click target.
             const panel = document.getElementById('header-search-panel');

gearbox/static/js/common/shortcut-help.js

@@ -77,16 +77,37 @@
         });
     }
 
+    let returnFocusEl = null;
+
     function open() {
         const overlay = document.getElementById('shortcuts-help-overlay');
         if (!overlay) return;
         renderShortcuts();
+        // Remember whoever had focus so we can hand it back on close.
+        returnFocusEl = document.activeElement;
         overlay.classList.remove('hidden');
+        // The overlay is role="dialog" aria-modal="true"; move focus
+        // into it so the dialog is announced by screen readers and
+        // subsequent Tab/Esc keystrokes land here, not on the page
+        // behind. The overlay has no interactive controls (clicking
+        // anywhere closes), so a single focusable container is enough
+        // — no formal trap needed (Copilot a11y review).
+        overlay.setAttribute('tabindex', '-1');
+        try { overlay.focus({ preventScroll: true }); }
+        catch (_) { overlay.focus(); }
     }
     function close() {
         const overlay = document.getElementById('shortcuts-help-overlay');
         if (!overlay) return;
         overlay.classList.add('hidden');
+        // Restore focus to whatever opened the overlay (typically the
+        // page body or a button), so keyboard users don't get dumped
+        // back at the top of the document.
+        if (returnFocusEl && typeof returnFocusEl.focus === 'function') {
+            try { returnFocusEl.focus({ preventScroll: true }); }
+            catch (_) { returnFocusEl.focus(); }
+        }
+        returnFocusEl = null;
     }
 
     function init() {