Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -265,6 +268,91 @@ const ConfigFieldEditor = memo<ConfigFieldEditorProps>(
);
}

case 'color': {
const colorText = displayedStringValue(field, value);
const note = colorInputMessage(colorText);
return (
<div className="mb-4">
<label
htmlFor={fullPath}
className="block text-sm font-medium text-gray-200 mb-2 font-sans"
>
{field.label}
</label>
{field.description && (
<p className="text-xs text-gray-400 mb-2 font-sans">
{field.description}
</p>
)}
<div className="flex items-center gap-2">
{/*
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.
*/}
<input
type="color"
value={colorPickerValue(colorText)}
onChange={(e) => 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`}
/>
<input
id={fullPath}
type="text"
value={colorText}
maxLength={field.maxLength}
onChange={(e) => 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 !== '' && (
<button
type="button"
onClick={() => handleChange('')}
className="shrink-0 px-2 py-2 text-xs text-gray-400 hover:text-gray-200 font-sans"
// The way back to the template's own colour, which is what
// the font preset spells out as "Theme default". Without it
// the first colour an owner picks is permanent.
title="Clear, and use the style template's color"
>
Clear
</button>
)}
</div>
{/*
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.
*/}
<div id={`${fullPath}-note`}>
<LiveRegion
className="block text-xs mt-1 font-sans text-gray-400"
message={note && !note.invalid ? note.message : null}
/>
<LiveRegion
urgency="assertive"
className="block text-xs mt-1 font-sans text-red-400"
message={note?.invalid ? note.message : null}
/>
</div>
</div>
);
}

default: {
const stringValue = displayedStringValue(field, value);
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
6 changes: 5 additions & 1 deletion apps/self-hosted/src/features/floating-menu/config-fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -488,7 +492,7 @@ export const configFieldsMap: Record<string, ConfigField> = {
*/
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,
Expand Down
143 changes: 143 additions & 0 deletions apps/self-hosted/src/features/floating-menu/field-display.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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', () => {
/** `<input type="color">` 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<string> {
const file = ts.createSourceFile(
'config-editor.tsx',
source,
ts.ScriptTarget.Latest,
true,
ts.ScriptKind.TSX,
);
const called = new Set<string>();
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');
});
});
Comment on lines +500 to +541

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files matching target:"
fd 'field-display\.test\.ts$|config-editor\.tsx$|components' apps/self-hosted/src/features/floating-menu || true

target_test="apps/self-hosted/src/features/floating-menu/field-display.test.ts"
target_editor="apps/self-hosted/src/features/floating-menu/components/config-editor.tsx"

if [ -f "$target_test" ]; then
  echo "--- test file size ---"
  wc -l "$target_test"
  echo "--- lines 460-560 ---"
  sed -n '460,560p' "$target_test" | nl -ba -v460
else
  echo "test file missing"
fi

if [ -f "$target_editor" ]; then
  echo "--- editor file size ---"
  wc -l "$target_editor"
  echo "--- outline ---"
  ast-grep outline "$target_editor" || true
  echo "--- color-related snippets ---"
  rg -n "color|colour|colorInputMessage|colorPickerValue|Clear|onUpdate" "$target_editor" -C 3
fi

echo "--- package/test framework indicators ---"
fd -a 'package\.json$' . | xargs -r -I{} sh -c 'echo "--- {}"; jq -r ".scripts // {}, .devDependencies // {}, .dependencies // {} | {vitest: .vitest, "`@testing-library/react`": .["`@testing-library/react`"], react: .react, vitest?: .[]? // empty}" {} 2>/dev/null || true'

Repository: ecency/vision-web

Length of output: 529


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target_test="apps/self-hosted/src/features/floating-menu/field-display.test.ts"
target_editor="apps/self-hosted/src/features/floating-menu/components/config-editor.tsx"

echo "--- target editor size ---"
wc -l "$target_editor"
echo "--- target test size/lines 440-542 ---"
python3 - <<'PY'
from pathlib import Path
p = Path("apps/self-hosted/src/features/floating-menu/field-display.test.ts")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
    if 440 <= i <= 542:
        print(f"{i:4}: {line}")
PY

echo "--- outline ---"
ast-grep outline "$target_editor" || true

echo "--- all color-related snippets in config-editor.tsx ---"
rg -n "color|colour|colorInputMessage|colorPickerValue|Clear|onUpdate" "$target_editor" -C 3 || true

echo "--- relevant test helpers ---"
python3 - <<'PY'
from pathlib import Path
text = Path("apps/self-hosted/src/features/floating-menu/field-display.test.ts").read_text()
for i, line in enumerate(text.splitlines(), 1):
    if 1 <= i <= 80 or 400 <= i <= 555:
        print(f"{i:4}: {line}")
PY

Repository: ecency/vision-web

Length of output: 17910


Replace the source-inspection test with a rendered editor test.

This test only reads config-editor.tsx and checks that colorInputMessage and colorPickerValue appear in exported/invoked identifiers. It does not verify that the editor renders the color text input, native color swatch, and Clear button, or that those controls update onUpdate. Use React Testing Library to render the editor and assert visible validation state and onUpdate values after text, swatch, and Clear actions.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/self-hosted/src/features/floating-menu/field-display.test.ts` around
lines 500 - 541, Replace the source-inspection test around calledFunctions with
a React Testing Library test that renders the config editor’s color-field path.
Assert the text input, native color swatch, and Clear button are visible, then
simulate text entry, swatch changes, and clearing; verify the rendered
validation state and the corresponding values passed to onUpdate.

Source: Coding guidelines

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not taking this one here: the runner is environment 'node' with include limited to *.test.ts, and @testing-library is not a dependency anywhere in this app, so a rendered-editor test is an infrastructure decision rather than a fix for this PR. The AST guard stays as the stopgap; it provably catches the deleted-case mutation.

46 changes: 46 additions & 0 deletions apps/self-hosted/src/features/floating-menu/field-display.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { formatHexColor, parseHexColor } from '@/core/theme-appearance';
import type { ConfigField } from './config-fields';
import type { ConfigValue } from './types';

Expand Down Expand Up @@ -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.
*
* `<input type="color">` 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;
}
Loading