diff --git a/apps/self-hosted/src/features/floating-menu/components/config-editor.tsx b/apps/self-hosted/src/features/floating-menu/components/config-editor.tsx index 5019e556f6..389a4a2733 100644 --- a/apps/self-hosted/src/features/floating-menu/components/config-editor.tsx +++ b/apps/self-hosted/src/features/floating-menu/components/config-editor.tsx @@ -1,8 +1,11 @@ import { memo, useEffect, useMemo, useState } from 'react'; +import { LiveRegion } from '@/features/shared/live-region'; import { validateArrayDraft, validateArrayEntries } from '../array-field'; import type { ConfigField } from '../config-fields'; import { FLOATING_MENU_THEME } from '../constants'; import { + colorInputMessage, + colorPickerValue, displayedBooleanValue, displayedSelectValue, displayedStringValue, @@ -265,6 +268,91 @@ const ConfigFieldEditor = memo( ); } + case 'color': { + const colorText = displayedStringValue(field, value); + const note = colorInputMessage(colorText); + return ( +
+ + {field.description && ( +

+ {field.description} +

+ )} +
+ {/* + A swatch beside the text, not instead of it. The native control + has no empty state, so it can pick a colour but can never say + "use the template's", which is where every instance starts. + */} + handleChange(e.target.value)} + className="h-9 w-12 shrink-0 cursor-pointer rounded bg-transparent" + style={{ + border: `1px solid ${FLOATING_MENU_THEME.borderColorStrong}`, + }} + aria-label={`${field.label} swatch`} + /> + handleChange(e.target.value)} + placeholder="#0969da" + spellCheck={false} + className={`${inputClassName} font-mono`} + style={{ + ...inputStyle, + borderColor: note?.invalid + ? '#ef4444' + : FLOATING_MENU_THEME.borderColorStrong, + }} + aria-invalid={note?.invalid ? true : undefined} + aria-describedby={`${fullPath}-note`} + /> + {colorText !== '' && ( + + )} +
+ {/* + Both regions stay mounted from the first render, because a live + region that appears together with its first message is usually + not announced at all. The id ties whichever message is showing to + the input, so the note is also read when the field gets focus. + */} +
+ + +
+
+ ); + } + default: { const stringValue = displayedStringValue(field, value); return ( diff --git a/apps/self-hosted/src/features/floating-menu/config-fields.test.ts b/apps/self-hosted/src/features/floating-menu/config-fields.test.ts index c99f4e6dcd..0e538eaef1 100644 --- a/apps/self-hosted/src/features/floating-menu/config-fields.test.ts +++ b/apps/self-hosted/src/features/floating-menu/config-fields.test.ts @@ -286,8 +286,15 @@ describe('appearance knobs', () => { * erases the stored section on merge, which would take `background` and the * font preset out with it. */ - it('takes the accent as text', () => { - expect(fieldAt(ACCENT_PATH)?.type).toBe('string'); + it('takes the accent as a color field, which writes text', () => { + // Pinned exactly: a regression to a bare `string` would drop the swatch + // and the inline validation while every other guard stayed green, because + // the orphaned `case 'color'` block would keep the call-site test passing. + // `color` writes strings, so the invariant this test has always been about + // holds too: never `number`, whose input writes null when cleared, and + // null erases the whole stored styles section on merge, taking + // `background` and the font preset with it. + expect(fieldAt(ACCENT_PATH)?.type).toBe('color'); }); it('offers the font preset where the appearance engine reads it', () => { diff --git a/apps/self-hosted/src/features/floating-menu/config-fields.ts b/apps/self-hosted/src/features/floating-menu/config-fields.ts index 1c2533be09..5f4c7be085 100644 --- a/apps/self-hosted/src/features/floating-menu/config-fields.ts +++ b/apps/self-hosted/src/features/floating-menu/config-fields.ts @@ -11,6 +11,10 @@ import { AUTH_METHODS } from '@/features/auth/utils/auth-methods'; export type ConfigFieldType = | 'string' + // A hex colour: a native swatch beside a text input. The swatch alone cannot + // express "unset", which is the value every instance holds today and the one + // an owner has to be able to get back to. + | 'color' | 'number' | 'boolean' | 'array' @@ -488,7 +492,7 @@ export const configFieldsMap: Record = { */ accent: { label: 'Accent color', - type: 'string', + type: 'color', description: "One color, as a hex value such as #0969da. Buttons, links, the active feed tab and focus rings derive from it, and the text that sits on it is corrected automatically to stay readable. Leave empty to keep the style template's own color.", maxLength: 32, diff --git a/apps/self-hosted/src/features/floating-menu/field-display.test.ts b/apps/self-hosted/src/features/floating-menu/field-display.test.ts index 6b7ac57454..ef36ec9fae 100644 --- a/apps/self-hosted/src/features/floating-menu/field-display.test.ts +++ b/apps/self-hosted/src/features/floating-menu/field-display.test.ts @@ -3,9 +3,13 @@ import { join } from 'node:path'; import ts from 'typescript'; import { describe, expect, it } from 'vitest'; import { HIVE_LAYER_CONFIG_DEFAULTS } from '@/core/hive-layer'; +import { parseHexColor } from '@/core/theme-appearance'; import type { ConfigField } from './config-fields'; import { configFieldsMap } from './config-fields'; import { + COLOR_UNSET_HINT, + colorInputMessage, + colorPickerValue, displayedBooleanValue, displayedSelectValue, displayedStringValue, @@ -396,3 +400,142 @@ describe('a select whose resolver normalizes', () => { expect(displayedSelectValue(strict, 'Standard')).toBe('off'); }); }); + +/** + * A bare text field accepted `banana`, saved it, and the site kept the template + * colour with nothing said. The save succeeded, so the natural reading was that + * the feature was broken. + * + * Everything here is checked against `parseHexColor`, the function the + * appearance engine itself uses, rather than against a second regex. A panel + * with its own opinion about the same string is how the panel and the site come + * to disagree. + */ +describe('color input', () => { + it('says nothing about a value the engine accepts', () => { + for (const accepted of ['#0969da', '#abc', '#ABC', ' #0969da ']) { + expect(colorInputMessage(accepted), accepted).toBeNull(); + } + }); + + it('warns about anything the engine will not apply', () => { + for (const rejected of ['banana', '#12', '#0969d', 'rgb(1,2,3)', '0969da']) { + const note = colorInputMessage(rejected); + expect(note?.invalid, rejected).toBe(true); + } + }); + + /** + * Alpha is refused by the engine on purpose: a translucent fill is a + * readability hole no contrast correction can close. The panel has to refuse + * it too, or it silently accepts a colour the site drops. + */ + it('warns about an alpha hex, which the engine refuses', () => { + expect(colorInputMessage('#0969daff')?.invalid).toBe(true); + }); + + /** Empty is a real state, not an error: it means the template's own colour. */ + it('explains empty rather than flagging it', () => { + const note = colorInputMessage(''); + expect(note?.invalid).toBe(false); + expect(note?.message).toBe(COLOR_UNSET_HINT); + expect(colorInputMessage(' ')?.invalid).toBe(false); + }); + + /** + * The warning has to say what happens, because what happens is the confusing + * part: the save succeeds and the site does not change. + */ + it('says what the site will do with a value it refuses', () => { + expect(colorInputMessage('banana')?.message).toMatch(/template/i); + }); + + /** + * Agreement with the engine, asserted as a property over both rather than as + * two lists someone kept in step. + */ + it.each([ + '#0969da', + '#abc', + 'banana', + '', + '#0969daff', + 'rgb(0,0,0)', + '#GGGGGG', + ])('flags %s exactly when the engine refuses it', (text) => { + const engineAccepts = parseHexColor(text) !== null; + const panelFlags = colorInputMessage(text)?.invalid === true; + // Empty is the one value that is neither accepted nor an error. + if (text.trim() === '') { + expect(panelFlags).toBe(false); + return; + } + expect(panelFlags).toBe(!engineAccepts); + }); +}); + +describe('color swatch value', () => { + /** `` only accepts `#rrggbb`, so `#abc` has to expand. */ + it('expands a short hex the native control cannot take', () => { + expect(colorPickerValue('#abc')).toBe('#aabbcc'); + }); + + it('passes a full hex through, lower-cased and trimmed', () => { + expect(colorPickerValue(' #0969DA ')).toBe('#0969da'); + }); + + /** + * The swatch has no empty state, so it needs something concrete. Displaying + * it must not write it: a value reaches the document only through onChange, + * so an owner who opens the panel and saves without touching the swatch + * stores nothing. + */ + it('falls back for unset and unparseable values without inventing one', () => { + expect(colorPickerValue('')).toBe('#888888'); + expect(colorPickerValue('banana')).toBe('#888888'); + expect(colorPickerValue('', '#123456')).toBe('#123456'); + }); +}); + +/** + * That the renderer actually uses the validation above. + * + * Every test in this file passed with the entire `case 'color'` block deleted + * from `config-editor.tsx`, which is the whole point of the change: the helpers + * being correct means nothing if the panel does not call them. Nothing in a + * `.tsx` is renderable under this runner, so the call is what can be asserted, + * and here the call IS the mechanism. + */ +describe('the editor renders colour fields with the shared validation', () => { + const EDITOR = join(__dirname, 'components', 'config-editor.tsx'); + + function calledFunctions(source: string): Set { + const file = ts.createSourceFile( + 'config-editor.tsx', + source, + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TSX, + ); + const called = new Set(); + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) { + called.add(node.expression.text); + } + ts.forEachChild(node, visit); + }; + visit(file); + return called; + } + + it('handles the color type and uses both helpers', () => { + const source = readFileSync(EDITOR, 'utf8'); + const called = calledFunctions(source); + + expect(source).toContain("case 'color'"); + // The message, or an invalid value is stored with nothing said. + expect(called).toContain('colorInputMessage'); + // The swatch value, or the native control gets a string it cannot take. + expect(called).toContain('colorPickerValue'); + }); +}); diff --git a/apps/self-hosted/src/features/floating-menu/field-display.ts b/apps/self-hosted/src/features/floating-menu/field-display.ts index 4b20e18d28..0375033768 100644 --- a/apps/self-hosted/src/features/floating-menu/field-display.ts +++ b/apps/self-hosted/src/features/floating-menu/field-display.ts @@ -1,3 +1,4 @@ +import { formatHexColor, parseHexColor } from '@/core/theme-appearance'; import type { ConfigField } from './config-fields'; import type { ConfigValue } from './types'; @@ -125,3 +126,48 @@ export function displayedStringValue( if (typeof value === 'string') return value; return typeof field.default === 'string' ? field.default : ''; } + +/** Shown under a colour input that is empty, which is a valid, meaningful state. */ +export const COLOR_UNSET_HINT = + "Empty, so the style template's own color is used."; + +/** + * Shown under a colour input holding something the engine will not apply. + * + * Says what happens rather than only that it is wrong, because what happens is + * the confusing part: the save succeeds, the value is stored, and the site does + * not change. + */ +export const COLOR_INVALID_MESSAGE = + "Not a color, so the site keeps the style template's own. Use a hex value like #0969da."; + +/** + * The message under a colour input, or null when there is nothing to say. + * + * Validated with `parseHexColor`, the function the appearance engine itself + * uses, so the panel cannot accept something the site then ignores or warn + * about something the site accepts. Anything else here would be a second + * opinion about the same string. + */ +export function colorInputMessage(text: string): { + message: string; + invalid: boolean; +} | null { + if (text.trim() === '') return { message: COLOR_UNSET_HINT, invalid: false }; + return parseHexColor(text) === null + ? { message: COLOR_INVALID_MESSAGE, invalid: true } + : null; +} + +/** + * What the native colour swatch shows. + * + * `` has no empty state and only accepts `#rrggbb`, so an + * unset or half-typed value needs something concrete, and `#rgb` needs + * expanding. Purely what is displayed: nothing here reaches the document unless + * the owner actually moves the picker. + */ +export function colorPickerValue(text: string, fallback = '#888888'): string { + const rgb = parseHexColor(text); + return rgb ? formatHexColor(rgb) : fallback; +}