officerdev/platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

favourite a theme from anywhere, and the browser stops fighting the menu

Browse Source 
Three reported faults and two additions, all from one keystroke's worth of use.

THE MENU OPENED BEHIND THE OVERLAY. The theme pill is a Radix DropdownMenuTrigger, and a focused trigger
opens on Space — so ctrl+shift+space opened the browser AND the dropdown behind it. preventDefault could
never have fixed it: React attaches its handlers at the root, which bubbles before window, so the trigger
saw the key first whatever this listener did with it. The listener now runs in the CAPTURE phase and stops
the event dead.

THE VIM KEYS DIED WHEN OPENED FROM AN INPUT. Same shape as the Spotlight bug earlier tonight: the guard
that refuses to steal keys from a text field was still true after the overlay opened, because focus had
never left the field. Opening now blurs, and the guard no longer applies once the list is open — there is no
input in it, so there is nothing left for it to protect. Only the GLOBAL bindings defer to a text field now,
which is the case it was written for.

f STARS WHAT IS FOCUSED, inside the list, so a browse and a shortlist are one pass rather than two.

ctrl+shift+f STARS THE CURRENT THEME FROM ANYWHERE, and deliberately without the typing guard the arrows
beside it need: word selection is what ctrl+shift+←/→ collides with in a text field, and ctrl+shift+f
collides with nothing — no character, no browser binding. Guarding it would only break it in the composer,
which is where you are when a theme starts annoying you.

The header pill now shows a star when what you are wearing is one of yours — only when true, because a
hollow star on every theme is chrome that says nothing two hundred times.

The row star in the overlay is a span with role="button", not a button: it sits inside the row's own
<button>, and nesting one is invalid HTML that browsers resolve by dropping one of them.

useThemeKeys takes the toggle as a callback rather than reaching for it. Favourites are per account and live
behind an API, which is app knowledge; this hook owns keys and selection and nothing else.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
pastilhas
2026-08-22 15:15:36 +00:00
co-authored by Claude Opus 5
parent bd15a186b5
commit dd6135cac1
2 changed files with 96 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx
+34 -9
View File
@@ -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 = () => {
<DropdownMenu>
<DropdownMenuTrigger asChild>
<button
title={`Theme: ${theme.name} · ctrl+shift+←/→ to cycle · ctrl+shift+space for the list`}
title={`Theme: ${theme.name} · ctrl+shift+←/→ cycle · ctrl+shift+space browse · ctrl+shift+f ${
isFavorite(themeId) ? 'unstar' : 'star'
}`}
aria-label={`Theme: ${theme.name}`}
// Named rather than a palette glyph: with 227 themes the one you are wearing is worth
// reading at a glance, and an icon cannot say which that is. Same pill as JobsIndicator so
@@ -70,6 +73,9 @@ export const ThemeSwitcher = () => {
style={{ color: 'var(--header-fg)' }}
>
{theme.name}
{/* A star on the pill, so "is this one of mine" is answerable without opening anything. Only
when true — a hollow star on every theme would be chrome that says nothing 200 times. */}
{isFavorite(themeId) && <Star className="ml-1.5 h-3 w-3 fill-current opacity-80" />}
</button>
</DropdownMenuTrigger>
{/*
@@ -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) && (
<Star className="ml-2 inline h-3 w-3 fill-current align-middle opacity-70" />
)}
{t.id === themeId && <span className="ml-2 align-middle text-xs opacity-70">active</span>}
<span className="min-w-0 flex-1 truncate">{t.name}</span>
{t.id === themeId && <span className="shrink-0 text-xs opacity-70">active</span>}
{/*
A span, not a button: this sits inside the row's own <button>, and a button inside
a button is invalid HTML that browsers resolve by dropping one of them. The click
handler and the role are set explicitly instead, and `f` does the same thing from
the keyboard.
*/}
<span
role="button"
tabIndex={-1}
aria-label={isFavorite(t.id) ? `Unstar ${t.name}` : `Star ${t.name}`}
title={isFavorite(t.id) ? 'Remove from favourites (f)' : 'Add to favourites (f)'}
onClick={(ev) => {
// Or the row's own onClick applies the theme and closes the overlay, which is
// the opposite of what starring is for.
ev.preventDefault();
ev.stopPropagation();
toggle(t.id);
}}
className="shrink-0 rounded p-1 opacity-70 hover:opacity-100"
>
<Star className={`h-4 w-4 ${isFavorite(t.id) ? 'fill-current' : ''}`} />
</span>
</button>
</li>
);
@@ -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. */}
<p className="mt-6 text-center text-xs opacity-60">
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
</p>
</div>
</div>,
src/workspaces/theme/src/useThemeKeys.ts
+62 -4
View File
@@ -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>(themeId);
const startedOn = useRef<ThemeId>(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 };