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 5d361dcc..9519508c 100644 --- a/src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Layout/Header/ThemeSwitcher.tsx @@ -1,8 +1,10 @@ +import { useMemo, useState } from 'react'; import { createPortal } from 'react-dom'; -import { Check } from 'lucide-react'; +import { Check, Star } from 'lucide-react'; +import { favouritesFirst, groupByFamily, rotateToSelected } from 'theme'; import * as Dropdown from '@/components/ui/dropdown-menu'; -import type { Theme } from '@/components/ui/ThemeProvider'; import { useTheme, useThemeKeys } from '@/components/ui/ThemeProvider'; +import { useThemeFavorites } from '@/state/useThemeFavorites'; const { DropdownMenu, @@ -31,13 +33,28 @@ const { export const ThemeSwitcher = () => { const { theme, themeId, setTheme, themes } = useTheme(); const { open, setOpen, focused, setFocused } = useThemeKeys(); + const { favorites, isFavorite, toggle } = useThemeFavorites(); + const [onlyFavorites, setOnlyFavorites] = useState(false); - // Grouped by family, so `Duck Light`/`Duck Dark` and the two Catppuccins sit together. The registry is - // readonly, so this builds mutable arrays rather than pushing into its own type. - const families = themes.reduce>((acc, t) => { - (acc[t.family] ??= []).push(t); - return acc; - }, {}); + /* + One ordering, used by both lists. + + ROTATED to the theme you are wearing rather than sorted: with 200+ installed, the list's only structure + is the registry order — families sit together in it — and lifting the selected theme out of its family + would answer "which am I on" by destroying "what is next to it". A rotation answers both. See + `theme/rotate.ts`. + + Then favourites first, which is a second rotation-preserving pass rather than a filter: each half keeps + the order it was given, so the theme you are wearing still leads. + */ + const ordered = useMemo(() => { + const rotated = rotateToSelected(themes, themeId); + const listed = onlyFavorites ? rotated.filter((t) => favorites.has(t.id)) : rotated; + return favouritesFirst(listed, favorites); + // `favorites` is a fresh Set each render; its CONTENTS are what matter, so key on those. + }, [themes, themeId, onlyFavorites, [...favorites].sort().join(',')]); + + const families = useMemo(() => groupByFamily(ordered), [ordered]); return ( <> @@ -55,21 +72,69 @@ export const ThemeSwitcher = () => { {theme.name} - - - ctrl+shift+←/→ · ctrl+shift+space - - - {Object.entries(families).map(([family, entries]) => ( -
- {entries.map((t) => ( - setTheme(t.id)} className="cursor-pointer"> - - {t.name} - - ))} -
- ))} + {/* + The CONTENT does not scroll — the list inside it does. + + It used to carry `overflow-y-auto` itself, which scrolled the instruction line away with the + themes. Two hundred entries meant the one line telling you the keyboard shortcuts existed was + visible for the first half-second and then gone for good, which is the same as not having written + it. Header and footer are now siblings of a scroll box rather than passengers in one. + */} + +
+ + ctrl+shift+←/→ cycle · ctrl+shift+space browse + alt+space to search every theme by name + + +
+ +
+ {ordered.length === 0 ? ( +

+ No favourites yet — star one with the button on its row. +

+ ) : ( + families.map(([family, entries]) => ( +
+ {entries.map((t) => ( + setTheme(t.id)} className="cursor-pointer pr-1"> + + {t.name} + {/* A sibling of the row's action, never nested inside it — starring is not choosing. */} + + + ))} +
+ )) + )} +
@@ -90,8 +155,11 @@ export const ThemeSwitcher = () => { }} onClick={(ev) => ev.stopPropagation()} > -
    - {themes.map((t) => { + {/* The SAME `ordered` list as the dropdown — rotated to what you are wearing, favourites + first. Both lists agreeing is the point: "the one below the one I have on" has to mean the + same thing in both, or the browse teaches you an order the menu then contradicts. */} +
      + {ordered.map((t) => { const isFocused = t.id === focused; return (
    • @@ -106,12 +174,17 @@ export const ThemeSwitcher = () => { }`} > {t.name} + {isFavorite(t.id) && ( + + )} {t.id === themeId && active}
      • ); })}
    + {/* 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

    diff --git a/src/apps/officer-web/state/useThemeFavorites.ts b/src/apps/officer-web/state/useThemeFavorites.ts new file mode 100644 index 00000000..2d54ac3a --- /dev/null +++ b/src/apps/officer-web/state/useThemeFavorites.ts @@ -0,0 +1,55 @@ +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { useClient } from 'hooks/useClient'; + +/* + Starred themes, per account. + + With 200+ installed, "which of these did I like" is a question the list cannot answer by itself, and the + answer belongs to the account rather than to the browser — the same taste follows you to the phone. + + Optimistic, and that is not a nicety here: the star sits in a dropdown you are about to close, so a + round trip before the icon fills means the confirmation arrives after the thing it confirms has gone. +*/ + +const KEY = ['themes', 'favorites']; + +export function useThemeFavorites() { + const client = useClient(); + const queryClient = useQueryClient(); + + const query = useQuery<{ favorites: string[] }>({ + queryKey: KEY, + queryFn: () => client.get<{ favorites: string[] }>('/theme-favorites'), + // Taste does not change while you look at it, and the mutation below keeps the cache honest. + staleTime: 5 * 60_000, + refetchOnWindowFocus: false, + retry: false, + }); + + const toggle = useMutation({ + mutationFn: ({ themeId, favorite }: { themeId: string; favorite: boolean }) => + client.put<{ themeId: string; favorite: boolean }>(`/theme-favorites/${themeId}`, { favorite }), + onMutate: async ({ themeId, favorite }) => { + await queryClient.cancelQueries({ queryKey: KEY }); + const previous = queryClient.getQueryData<{ favorites: string[] }>(KEY); + const current = previous?.favorites ?? []; + queryClient.setQueryData(KEY, { + favorites: favorite ? [...new Set([...current, themeId])] : current.filter((id) => id !== themeId), + }); + return { previous }; + }, + // Put it back on failure. A star that stayed filled after the write was refused is worse than one that + // never filled: it reports a preference the server does not hold. + onError: (_err, _vars, context) => { + if (context?.previous) queryClient.setQueryData(KEY, context.previous); + }, + }); + + const favorites = new Set(query.data?.favorites ?? []); + + return { + favorites, + isFavorite: (themeId: string) => favorites.has(themeId), + toggle: (themeId: string) => toggle.mutate({ themeId, favorite: !favorites.has(themeId) }), + }; +} diff --git a/src/databases/officer_db/src/index.ts b/src/databases/officer_db/src/index.ts index 7ec369a4..5c0c7d0d 100644 --- a/src/databases/officer_db/src/index.ts +++ b/src/databases/officer_db/src/index.ts @@ -38,6 +38,7 @@ export * from './chat-notification-prefs'; export * from './nudges'; export * from './ai-provider-usage'; export * from './anthropic-accounts'; +export * from './theme-favorites'; export * from './command-hq'; export * from './dashboards'; export * from './integrations'; diff --git a/src/databases/officer_db/src/schema.ts b/src/databases/officer_db/src/schema.ts index 53de81a8..4a0f79c9 100644 --- a/src/databases/officer_db/src/schema.ts +++ b/src/databases/officer_db/src/schema.ts @@ -35,6 +35,7 @@ export * from './chat-session-models/schema'; // chat_session_models export * from './chat-notification-prefs/schema'; // chat_notification_prefs export * from './ai-provider-usage/schema'; // ai_provider_usage — last usage reading per AI account export * from './anthropic-accounts/schema'; // anthropic_accounts — switchable OAuth credentials +export * from './theme-favorites/schema'; // theme_favorites — starred themes, per account export * from './command-hq/schema'; // command_hq_categories, command_hq_sessions, command_hq_tags, command_hq_session_tags export * from './agent-panels/schema'; // agent_panels diff --git a/src/databases/officer_db/src/theme-favorites/index.ts b/src/databases/officer_db/src/theme-favorites/index.ts new file mode 100644 index 00000000..eb271635 --- /dev/null +++ b/src/databases/officer_db/src/theme-favorites/index.ts @@ -0,0 +1,2 @@ +export * from './schema'; +export * from './queries'; diff --git a/src/databases/officer_db/src/theme-favorites/queries.ts b/src/databases/officer_db/src/theme-favorites/queries.ts new file mode 100644 index 00000000..a875b8a5 --- /dev/null +++ b/src/databases/officer_db/src/theme-favorites/queries.ts @@ -0,0 +1,28 @@ +import { and, eq } from 'drizzle-orm'; +import { db } from '../db'; +import { themeFavorites } from './schema'; + +/** The theme ids this account has starred. Order is not meaningful — the caller owns the ordering. */ +export async function listThemeFavorites(userId: number): Promise { + const rows = await db + .select({ themeId: themeFavorites.themeId }) + .from(themeFavorites) + .where(eq(themeFavorites.userId, userId)); + return rows.map((r) => r.themeId); +} + +/** + * Star or unstar, returning the state it ended in. + * + * Idempotent in both directions on purpose: a double-tap, or two tabs racing, must not end with the star + * in a state neither of them asked for. `onConflictDoNothing` makes the insert safe to repeat, and the + * delete is naturally so. + */ +export async function setThemeFavorite(userId: number, themeId: string, favorite: boolean): Promise { + if (favorite) { + await db.insert(themeFavorites).values({ userId, themeId }).onConflictDoNothing(); + return true; + } + await db.delete(themeFavorites).where(and(eq(themeFavorites.userId, userId), eq(themeFavorites.themeId, themeId))); + return false; +} diff --git a/src/databases/officer_db/src/theme-favorites/schema.ts b/src/databases/officer_db/src/theme-favorites/schema.ts new file mode 100644 index 00000000..d4464eb4 --- /dev/null +++ b/src/databases/officer_db/src/theme-favorites/schema.ts @@ -0,0 +1,37 @@ +import { integer, pgTable, serial, text, timestamp, uniqueIndex } from 'drizzle-orm/pg-core'; +import { users } from '../auth/schema'; + +/** + * Which themes an account has starred. + * + * ── Why a table, when a preference could be one JSON column ── + * + * Because it is per account and unbounded. `user_settings` would hold an array that grows with taste and + * has to be read-modify-written by whoever toggles a star, and two tabs doing that at once lose one of the + * stars. A row per favourite makes a toggle an INSERT or a DELETE, which cannot race with itself. + * + * ── Why the theme id is text with no foreign key ── + * + * Themes are not database rows. Most are compiled into the bundle; the rest are directories under + * `platform/themes/` discovered at request time, so a theme can appear or vanish without anything writing + * SQL. A foreign key would either be unenforceable or would make installing a theme a migration. + * + * The consequence is deliberate: a star can outlive its theme. That is why the reader intersects with the + * live registry rather than trusting the table — a favourite naming a theme nobody has installed is + * ignored, not shown as a broken entry, and it costs one row until the account unstars something. + */ +export const themeFavorites = pgTable( + 'theme_favorites', + { + id: serial('id').primaryKey(), + userId: integer('user_id') + .notNull() + .references(() => users.id, { onDelete: 'cascade' }), + /** The theme's registry id — `duck-dark`, `catppuccin`. Not a foreign key; see above. */ + themeId: text('theme_id').notNull(), + createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), + }, + // uniqueIndex, never unique().on() — drizzle-kit re-creates named composite constraints on every push. + // See src/databases/CLAUDE.md → "Composite keys". + (table) => [uniqueIndex('uq_theme_favorites_user_theme').on(table.userId, table.themeId)], +); diff --git a/src/servers/api/themes/favorites.ts b/src/servers/api/themes/favorites.ts new file mode 100644 index 00000000..006b17bb --- /dev/null +++ b/src/servers/api/themes/favorites.ts @@ -0,0 +1,44 @@ +import type { Context } from 'hono'; +import { listThemeFavorites, setThemeFavorite } from 'officerdb'; +import { createRouter } from '../../create-router'; + +/* + Starred themes, per account. + + ── Why this is not on `/themes` ── + + Because that prefix is deliberately UNAUTHENTICATED: the sign-in screen renders in the chosen theme, so + the list and the stylesheets have to be readable before anyone has an account. A per-account preference on + an anonymous prefix has no caller to attribute it to — the first version of this lived there and answered + 500 to an anonymous GET, which was the honest consequence arriving early. + + ── Why the `account` permission ── + + It is `core`, so every account holds it and it cannot be denied, and it already owns `/user`, `/dock` and + `/api-keys` — preferences and credentials belonging to the caller themselves. A theme favourite is the + same shape as a dock preference: it grants nothing, it is about this account's own eyes, and denying it + would only mean an account that can choose a theme cannot remember which ones it liked. + + Every route here reads `user.id` from the token. None of them names an account, so there is nothing to get + wrong about whose stars these are. +*/ +export const themeFavoritesRouter = createRouter(); + +// GET /theme-favorites — this account's starred theme ids. +themeFavoritesRouter.get('/', async (ctx: Context) => { + const user = ctx.get('user'); + return ctx.json({ favorites: await listThemeFavorites(user.id) }); +}); + +// PUT /theme-favorites/:id — star or unstar one. +// +// PUT rather than POST because it SETS a state rather than appending: sending `{favorite:true}` twice ends +// where sending it once does, which is what a double-tap and two racing tabs both need. The store is +// idempotent in both directions for the same reason. +themeFavoritesRouter.put('/:id', async (ctx: Context) => { + const user = ctx.get('user'); + const themeId = ctx.req.param('id'); + const body = await ctx.req.json().catch(() => ({}) as Record); + const favorite = body.favorite !== false; + return ctx.json({ themeId, favorite: await setThemeFavorite(user.id, themeId, favorite) }); +}); diff --git a/src/servers/api/themes/themes.ts b/src/servers/api/themes/themes.ts index dbef1c53..496a3c05 100644 --- a/src/servers/api/themes/themes.ts +++ b/src/servers/api/themes/themes.ts @@ -5,6 +5,12 @@ import { discoverThemes, readThemeCss } from '../../themes/discover'; /** * Installed themes — the ones in `platform/themes/`, not the ones the platform ships. * + * **This prefix is UNAUTHENTICATED** (`UNPROTECTED_API_PREFIXES` in hono.ts) and must stay that way: the + * sign-in screen paints in the chosen theme, before any account exists to ask. So nothing per-account can + * live here — theme FAVOURITES are on `/theme-favorites`, behind the `account` permission, for exactly that + * reason. Adding them here returned 500 on an anonymous request, which was the polite version of the + * problem. + * * The shipped themes are compiled into the bundle and need no server. These are read off disk at request * time, so dropping a directory in makes a theme appear on the next page load. No install step, no * rebuild, no restart — a theme is CSS custom properties, and none of that needs compiling. diff --git a/src/servers/hono.ts b/src/servers/hono.ts index 8293d397..ce276d0e 100644 --- a/src/servers/hono.ts +++ b/src/servers/hono.ts @@ -6,6 +6,7 @@ import { authRouter } from './api/auth'; import { serverSettingsRouter } from './api/server-settings/server-settings'; import { landingPageDataRouter } from './api/landing-page-data/landing-page-data'; import { themesRouter } from './api/themes/themes'; +import { themeFavoritesRouter } from './api/themes/favorites'; import { waitlistRouter } from './api/waitlist/waitlist'; import { usersRouter } from './api/users/users-router'; import { apiKeysRouter } from './api/api-keys/router'; @@ -122,6 +123,8 @@ const isDavPath = (path: string) => // disambiguate. `/pipeline-jobs` and `/jobs` deliberately share one router. const PROTECTED_MOUNTS: [prefix: string, router: ReturnType][] = [ ['/server-settings', serverSettingsRouter], + // Protected, unlike `/themes` beside it — see api/themes/favorites.ts for why they are separate mounts. + ['/theme-favorites', themeFavoritesRouter], ['/users', usersRouter], ['/skills', skillsRouter], ['/tasks', tasksRouter], diff --git a/src/servers/permissions/registry.ts b/src/servers/permissions/registry.ts index 0a59d572..b1f06476 100644 --- a/src/servers/permissions/registry.ts +++ b/src/servers/permissions/registry.ts @@ -147,7 +147,10 @@ const CORE_REGISTRY: Permission[] = [ // to present the authority the account already has, so denying it would only force the holder to keep // using a password in places a password should not go. What a key can then DO is decided by the same // permission checks as any other request from that user; nothing here widens them. - api: ['/user', '/dock', '/api-keys'], + // `/theme-favorites` sits here rather than on `/themes`, which is unauthenticated so the sign-in screen + // can paint. A starred theme is the same shape as a dock preference: the caller's own taste, granting + // nothing, and not a sensible thing to deny an account that can already choose a theme. + api: ['/user', '/dock', '/api-keys', '/theme-favorites'], routes: ['/settings/profile'], }, { diff --git a/src/workspaces/theme/src/index.ts b/src/workspaces/theme/src/index.ts index 5642611e..5e83a8fe 100644 --- a/src/workspaces/theme/src/index.ts +++ b/src/workspaces/theme/src/index.ts @@ -9,3 +9,5 @@ export { useThemeAsset } from './useThemeAsset'; export { useThemeKeys } from './useThemeKeys'; export { THEMES, DEFAULT_THEME, findTheme, isKnownTheme, isDarkTheme, themeFromLegacyColorMode } from './themes'; export type { Theme, ThemeId } from './themes'; +// Ordering helpers for the pickers — pure, and tested in rotate.test.ts. +export { rotateToSelected, favouritesFirst, groupByFamily } from './rotate'; diff --git a/src/workspaces/theme/src/rotate.test.ts b/src/workspaces/theme/src/rotate.test.ts new file mode 100644 index 00000000..936ff269 --- /dev/null +++ b/src/workspaces/theme/src/rotate.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from 'bun:test'; +import { favouritesFirst, groupByFamily, rotateToSelected } from './rotate'; + +const ids = (list: Array<{ id: string }>) => list.map((t) => t.id); +const list = [{ id: 'a' }, { id: 'b' }, { id: 'c' }, { id: 'd' }]; + +describe('rotating to the selected theme', () => { + test('puts the selected one first and wraps the rest behind it', () => { + expect(ids(rotateToSelected(list, 'c'))).toEqual(['c', 'd', 'a', 'b']); + }); + + // The property that makes this a rotation rather than a sort: nobody loses a neighbour. + test('every adjacency survives, only the starting point moves', () => { + const rotated = ids(rotateToSelected(list, 'c')); + const doubled = [...ids(list), ...ids(list)].join(','); + expect(doubled).toContain(rotated.join(',')); + }); + + test('leaves the order alone when the selected one is already first', () => { + expect(ids(rotateToSelected(list, 'a'))).toEqual(['a', 'b', 'c', 'd']); + }); + + // An id from an uninstalled theme, or none chosen yet. Rotating by a guess would be worse than not. + test('leaves the order alone for an unknown or missing id', () => { + expect(ids(rotateToSelected(list, 'gone'))).toEqual(['a', 'b', 'c', 'd']); + expect(ids(rotateToSelected(list, undefined))).toEqual(['a', 'b', 'c', 'd']); + }); + + test('does not mutate the input', () => { + const original = [...ids(list)]; + rotateToSelected(list, 'd'); + expect(ids(list)).toEqual(original); + }); +}); + +describe('favourites first', () => { + test('lifts favourites, keeping the order within each half', () => { + expect(ids(favouritesFirst(list, new Set(['d', 'b'])))).toEqual(['b', 'd', 'a', 'c']); + }); + + test('changes nothing when there are none', () => { + expect(ids(favouritesFirst(list, new Set()))).toEqual(['a', 'b', 'c', 'd']); + }); + + // Composed with the rotation: the theme you are wearing leads its half. + test('composes with the rotation', () => { + expect(ids(favouritesFirst(rotateToSelected(list, 'c'), new Set(['c', 'a'])))).toEqual(['c', 'a', 'd', 'b']); + }); +}); + +describe('grouping by family', () => { + const themed = [ + { id: 'duck-dark', name: 'Duck Dark', family: 'Duck' }, + { id: 'cat', name: 'Catppuccin', family: 'Catppuccin' }, + { id: 'duck-light', name: 'Duck Light', family: 'Duck' }, + ] as never as Parameters[0]; + + test('keeps families in first-seen order, so a rotation decides them', () => { + expect(groupByFamily(themed).map(([family]) => family)).toEqual(['Duck', 'Catppuccin']); + }); + + // Members of a family separated by the rotation still collect together — the alternative is the same + // family heading appearing twice. + test('collects a family split by the rotation', () => { + const groups = groupByFamily(themed); + expect(groups[0]![1].map((t) => t.id)).toEqual(['duck-dark', 'duck-light']); + }); +}); diff --git a/src/workspaces/theme/src/rotate.ts b/src/workspaces/theme/src/rotate.ts new file mode 100644 index 00000000..0e440b56 --- /dev/null +++ b/src/workspaces/theme/src/rotate.ts @@ -0,0 +1,49 @@ +import type { Theme, ThemeId } from './themes'; + +/** + * The theme list, rotated so the one you are wearing is first. + * + * With 200+ themes the list is far taller than any container, and the theme you have on is the anchor for + * every decision you are about to make — "what did I just come from", "what is next to this". Sorting the + * selected one to the top would answer the first question and destroy the second, because it takes that + * theme out of its family and leaves a hole where it was. + * + * A ROTATION keeps every neighbour: the order is unchanged, the starting point moves. Duck Dark's + * neighbours are still Duck Light and whatever followed it; they have simply wrapped around behind you. + * + * The registry order is the ordering to preserve — families sit together in it, and that grouping is the + * only structure a list of two hundred names has. + */ +export function rotateToSelected(list: readonly T[], selected: ThemeId | undefined): T[] { + if (!selected) return [...list]; + const at = list.findIndex((entry) => entry.id === selected); + // Not present: an id from a theme that has been uninstalled, or none set yet. Leave the order alone + // rather than rotate by a guess. + if (at <= 0) return [...list]; + return [...list.slice(at), ...list.slice(0, at)]; +} + +/** + * Favourites first, then everything else — each half keeping the rotation it was given. + * + * Two-tier rather than a filter, because hiding the rest makes the list unable to answer "what else is + * there". A filter is a separate, explicit act; this is the default shape. + */ +export function favouritesFirst(list: readonly T[], favourites: ReadonlySet): T[] { + if (favourites.size === 0) return [...list]; + const starred: T[] = []; + const rest: T[] = []; + for (const entry of list) (favourites.has(entry.id) ? starred : rest).push(entry); + return [...starred, ...rest]; +} + +/** Grouped by family for the dropdown, over an already-ordered list. Families appear in first-seen order. */ +export function groupByFamily(list: readonly Theme[]): Array<[string, Theme[]]> { + const groups = new Map(); + for (const theme of list) { + const existing = groups.get(theme.family); + if (existing) existing.push(theme); + else groups.set(theme.family, [theme]); + } + return [...groups.entries()]; +}