diff --git a/src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx b/src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx index 9519508c..0c1c7b86 100644 --- a/src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx @@ -32,8 +32,9 @@ const { */ export const ThemeSwitcher = () => { const { theme, themeId, setTheme, themes } = useTheme(); - const { open, setOpen, focused, setFocused } = useThemeKeys(); const { favorites, isFavorite, toggle } = useThemeFavorites(); + // Declared before the key hook, which needs the toggle: `f` in the list and `ctrl+shift+f` from anywhere. + const { open, setOpen, focused, setFocused } = useThemeKeys({ onToggleFavorite: toggle }); const [onlyFavorites, setOnlyFavorites] = useState(false); /* @@ -61,7 +62,9 @@ export const ThemeSwitcher = () => { {/* @@ -169,15 +175,34 @@ export const ThemeSwitcher = () => { setTheme(t.id); setOpen(false); }} - className={`w-full rounded-md px-4 py-1 text-left transition-all cursor-pointer ${ + className={`flex w-full items-center gap-2 rounded-md px-4 py-1 text-left transition-all cursor-pointer ${ isFocused ? 'text-2xl font-extrabold' : 'text-lg opacity-60 hover:opacity-100' }`} > - {t.name} - {isFavorite(t.id) && ( - - )} - {t.id === themeId && active} + {t.name} + {t.id === themeId && active} + {/* + A span, not a button: this sits inside the row's own ); @@ -186,7 +211,7 @@ export const ThemeSwitcher = () => { {/* Always visible: it sits outside the scroll box above, for the same reason the dropdown's header does. */}

- j/k or ↑/↓ move · l or → preview · h or ← reset · enter keep · esc cancel + j/k or ↑/↓ move · l or → preview · f star · h or ← reset · enter keep · esc cancel

, diff --git a/src/workspaces/theme/src/useThemeKeys.ts b/src/workspaces/theme/src/useThemeKeys.ts index c986b3f0..9b930b05 100644 --- a/src/workspaces/theme/src/useThemeKeys.ts +++ b/src/workspaces/theme/src/useThemeKeys.ts @@ -10,6 +10,7 @@ import type { ThemeId } from './themes'; * ctrl+shift+→ next theme, from anywhere, no UI * ctrl+shift+← previous theme * ctrl+shift+space open the list, then browse it + * ctrl+shift+f star or unstar the current theme, from anywhere * * One modifier for all three, which took some choosing. `ctrl+alt+arrows` switches WORKSPACE on GNOME and * `ctrl+alt+l` locks the screen, so ctrl+alt is unusable for this. `shift+alt` is free on Linux but is the @@ -20,6 +21,8 @@ import type { ThemeId } from './themes'; * action invited the guess that `ctrl+shift+j` moved down — which opens the browser console instead. One * spelling, no guessing. * + * Inside the list `f` stars or unstars what is FOCUSED, so a browse and a shortlist are one pass. + * * Inside the list the vim keys stay, UNMODIFIED — `j`/`k`/`l`/`h` alongside the arrows. No modifier means * no collision with anything the browser owns, and this is a vim person's application. * @@ -38,8 +41,22 @@ import type { ThemeId } from './themes'; * This hook owns the keys and the selection; something else draws the list. It lives in the theme package * because the behaviour belongs to themes, while a dialog would drag a component library in behind it. */ -export function useThemeKeys() { +type ThemeKeysOptions = { + /** + * Star or unstar the focused theme — bound to `f` while the list is open. + * + * Passed in rather than owned here: favourites are per ACCOUNT and live behind an API, which is app + * knowledge. This hook owns keys and selection and nothing else. + */ + onToggleFavorite?: (themeId: ThemeId) => void; +}; + +export function useThemeKeys(options: ThemeKeysOptions = {}) { const { themeId, setTheme, themes } = useTheme(); + // A ref so the window listener never needs re-binding when the callback identity changes — which it does + // on every render, since it closes over a mutation. + const onToggleFavorite = useRef(options.onToggleFavorite); + onToggleFavorite.current = options.onToggleFavorite; const [open, setOpen] = useState(false); const [focused, setFocused] = useState(themeId); const startedOn = useRef(themeId); @@ -65,6 +82,20 @@ export function useThemeKeys() { // fingers are quicker to learn than to unlearn. if (event.ctrlKey && (event.shiftKey || event.altKey) && event.code === 'Space') { event.preventDefault(); + /* + Two problems solved by two lines, and both were reported from the same keystroke. + + `stopPropagation` stops the header's dropdown ALSO opening. The theme pill is a Radix + DropdownMenuTrigger, and a focused trigger opens on Space — so if the pill happened to have focus, + the chord opened the overlay and the menu behind it. React attaches its handlers at the root, which + bubbles before window, so the trigger saw the key first no matter what this listener did with it. + This listener now runs in the CAPTURE phase and stops the event dead. + + The blur stops the overlay opening with focus still in a text field, which made j/k/l do nothing: + the guard below correctly refuses to steal keys from an input, and the input never lost focus. + */ + event.stopPropagation(); + (document.activeElement as HTMLElement | null)?.blur?.(); setOpen((was) => { if (!was) { startedOn.current = current; @@ -83,6 +114,22 @@ export function useThemeKeys() { const target = event.target as HTMLElement | null; const typing = Boolean(target && (target.isContentEditable || /^(input|textarea|select)$/i.test(target.tagName))); + /* + Star or unstar the CURRENT theme, from anywhere, list open or not. + + Not behind the `typing` guard, unlike the arrows beside it, and the difference is the point: word + selection is what `ctrl+shift+←/→` collides with inside a text field, and `ctrl+shift+F` collides + with nothing — it types no character and no browser claims it. Guarding it would only mean the + binding fails in the composer, which is where you are when a theme annoys you. + */ + if (event.ctrlKey && event.shiftKey && (event.key === 'f' || event.key === 'F')) { + event.preventDefault(); + event.stopPropagation(); + // The one you are WEARING, not the one focused in the list — this fires with no list open. + onToggleFavorite.current?.(current); + return; + } + // Cycle from anywhere, list open or not. Arrows only — see the note above. if (event.ctrlKey && event.shiftKey && !typing) { if (event.key === 'ArrowRight') { @@ -97,9 +144,18 @@ export function useThemeKeys() { } } - if (!isOpen || typing) return; + // Only the GLOBAL bindings above defer to a text field. Once the list is open there is no input in + // it, focus was blurred to open it, and a `typing` check here is what made every vim key dead when + // the overlay was opened from the composer. + if (!isOpen) return; switch (event.key) { + case 'f': + case 'F': + // Star or unstar what is focused, without moving or applying it. + event.preventDefault(); + onToggleFavorite.current?.(focus); + return; case 'j': case 'J': case 'ArrowDown': @@ -140,8 +196,10 @@ export function useThemeKeys() { } }; - window.addEventListener('keydown', onKeyDown); - return () => window.removeEventListener('keydown', onKeyDown); + // CAPTURE, so the chord is claimed before React's root-level handlers see it — see the note above about + // the dropdown trigger opening alongside the overlay. + window.addEventListener('keydown', onKeyDown, true); + return () => window.removeEventListener('keydown', onKeyDown, true); }, [setTheme, step]); return { open, setOpen, focused, setFocused };