fix(ui): restore highlight options layout and clean up color name editing (#3776)

* fix(ui): restore highlight options layout and clean up color name editing

Restore the pre-#3741 layout for both the AnnotationPopup highlight
options row and the HighlightColorsEditor settings panel.

HighlightOptions: re-add `justify-between` on the outer container and
remove `flex-1` from the color strip so the gap between the style box
and color strip responds to the number of colors.

HighlightColorsEditor: restore `grid-cols-3 sm:grid-cols-5` grid,
remove always-visible name inputs, and add a click-to-edit popover on
each color circle with hover tooltip for the label.

Add a browser screenshot test that renders the real AnnotationPopup
component with Tailwind CSS and compares against baseline PNGs for
5, 10, and 15 colors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

apps/readest-app/.claude/memory/MEMORY.md

@@ -29,6 +29,7 @@
 - Dropdown menus use `DropdownContext` (not blur-based) for screen reader compat
 
 ## Workflow
+- [Test file filter](feedback_test_file_filter.md) — use `pnpm test <path>` without `--` to run a single file
 - [Always rebase before PR](feedback_pr_rebase.md) — rebase onto origin/main before creating PRs
 - [New branch per PR](feedback_pr_new_branch.md) — always create a fresh branch from main for each new PR/issue
 - [Upgrade gstack locally](feedback_gstack_upgrade.md) — always upgrade from the project's .claude/skills/gstack, not global

apps/readest-app/.claude/memory/feedback_test_file_filter.md

@@ -0,0 +1,11 @@
+---
+name: test-file-filter
+description: Use pnpm test/test:browser with path directly (no --) to run a single test file
+type: feedback
+---
+
+Run a specific test file with `pnpm test <path>` or `pnpm test:browser <path>` — no `--` separator.
+
+**Why:** Adding `--` before the path (e.g. `pnpm test:browser -- <path>`) causes vitest to ignore the file filter and run all test files. Without `--`, pnpm appends the path directly to the vitest command, which correctly filters to that file only.
+
+**How to apply:** Always use `pnpm test src/__tests__/foo.test.ts` or `pnpm test:browser src/__tests__/foo.browser.test.tsx` when verifying a specific test file.

apps/readest-app/.gitignore

@@ -9,6 +9,7 @@
 # testing
 /coverage
 .test-sandbox-node/
+.vitest-attachments/
 
 # next.js
 /.next/

apps/readest-app/package.json

@@ -16,9 +16,9 @@
     "build-tauri": "dotenv -e .env.tauri -- next build",
     "i18n:extract": "i18next-scanner --config i18next-scanner.config.cjs",
     "lint": "tsgo --noEmit && biome check .",
-    "test": "dotenv -e .env -e .env.test.local vitest",
+    "test": "dotenv -e .env -e .env.test.local -- vitest",
     "test:coverage": "dotenv -e .env -e .env.test.local -- vitest run --coverage",
-    "test:browser": "vitest --config vitest.browser.config.mts --watch=false",
+    "test:browser": "dotenv -e .env -e .env.test.local -- vitest run --config vitest.browser.config.mts",
     "test:tauri": "bash scripts/test-tauri.sh",
     "test:pr:web": "pnpm test -- --watch=false && pnpm test:browser",
     "test:pr:tauri": "bash scripts/test-tauri.sh",

apps/readest-app/src/__tests__/components/annotation-popup-layout.browser.test.tsx

@@ -0,0 +1,210 @@
+/**
+ * Visual regression test for the AnnotationPopup component.
+ *
+ * Renders the *real* AnnotationPopup + HighlightOptions with actual
+ * annotationToolButtons, DEFAULT_HIGHLIGHT_COLORS, and optional user
+ * colors.  Tailwind CSS is loaded so the screenshot matches the live app.
+ *
+ * Guards against the layout regression from PR #3741 (missing
+ * `justify-between`, unwanted `flex-1` on the color strip).
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest';
+import { render, cleanup } from '@testing-library/react';
+import { page } from 'vitest/browser';
+import type { UserHighlightColor } from '@/types/book';
+
+// ── Tailwind / DaisyUI styles ───────────────────────────────────────────
+import '@/styles/globals.css';
+
+// ── Per-test state read by mocks ────────────────────────────────────────
+let mockUserColors: UserHighlightColor[] = [];
+
+// ── Mocks (must be before component imports) ────────────────────────────
+
+vi.mock('@/context/EnvContext', () => ({
+  useEnv: () => ({ envConfig: {}, appService: null }),
+}));
+
+vi.mock('@/store/themeStore', () => ({
+  useThemeStore: () => ({ isDarkMode: false }),
+}));
+
+vi.mock('@/hooks/useTranslation', () => ({
+  useTranslation: () => (s: string) => s,
+}));
+
+vi.mock('@/store/settingsStore', () => ({
+  useSettingsStore: () => ({
+    settings: {
+      globalReadSettings: {
+        highlightStyle: 'highlight' as const,
+        highlightStyles: {
+          highlight: 'yellow',
+          underline: 'red',
+          squiggly: 'blue',
+        },
+        customHighlightColors: {} as Record<string, string>,
+        get userHighlightColors() {
+          return mockUserColors;
+        },
+        defaultHighlightLabels: {},
+      },
+      globalViewSettings: {
+        isEink: false,
+        isColorEink: false,
+      },
+    },
+  }),
+}));
+
+vi.mock('@/hooks/useResponsiveSize', () => ({
+  useResponsiveSize: (n: number) => n,
+  useDefaultIconSize: () => 20,
+}));
+
+vi.mock('@/hooks/useKeyDownActions', () => ({
+  useKeyDownActions: () => {},
+}));
+
+vi.mock('@/helpers/settings', () => ({
+  saveSysSettings: vi.fn(),
+}));
+
+vi.mock('@/app/reader/utils/annotatorUtil', () => ({
+  getHighlightColorLabel: () => undefined,
+}));
+
+// ── Real component imports ──────────────────────────────────────────────
+
+import AnnotationPopup from '@/app/reader/components/annotator/AnnotationPopup';
+import { annotationToolButtons } from '@/app/reader/components/annotator/AnnotationTools';
+
+// ── Constants ───────────────────────────────────────────────────────────
+
+const POPUP_W = 300;
+const POPUP_H = 44;
+
+// Highlight options float above the popup by (28 + 16) = 44px
+const OPTIONS_OFFSET = 28 + 16;
+
+// Position the popup so both it and the floating options are visible:
+//   y=0..OPTIONS_OFFSET: highlight-options row
+//   y=OPTIONS_OFFSET..OPTIONS_OFFSET+POPUP_H: toolbar
+const POPUP_Y = OPTIONS_OFFSET;
+const POPUP_X = 0;
+const WRAPPER_H = POPUP_Y + POPUP_H + 14; // +14 for triangle below
+
+const toolButtons = annotationToolButtons.map(({ label, Icon }) => ({
+  tooltipText: label,
+  Icon,
+  onClick: vi.fn(),
+}));
+
+// Browser-mode matcher types are unavailable to tsgo; cast once here.
+const expectElement = (locator: unknown) =>
+  // @ts-expect-error -- expect.element() exists in vitest browser mode
+  expect.element(locator) as { toMatchScreenshot: (name: string) => Promise<void> };
+
+/**
+ * Fixed-size wrapper that contains both the popup and the absolutely
+ * positioned highlight-options row above it, matching the real app
+ * where the triangle points up and highlight options float above.
+ */
+const Wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => (
+  <div
+    data-theme='dark'
+    style={{
+      position: 'relative',
+      width: POPUP_W,
+      height: WRAPPER_H,
+      overflow: 'visible',
+    }}
+  >
+    {children}
+  </div>
+);
+
+const renderPopup = (userColors: UserHighlightColor[] = []) => {
+  mockUserColors = userColors;
+  return render(
+    <Wrapper>
+      <AnnotationPopup
+        bookKey='test'
+        dir='ltr'
+        isVertical={false}
+        buttons={toolButtons}
+        notes={[]}
+        position={{ dir: 'up', point: { x: POPUP_X, y: POPUP_Y } }}
+        trianglePosition={{ dir: 'up', point: { x: POPUP_X + POPUP_W / 2, y: POPUP_Y + POPUP_H } }}
+        highlightOptionsVisible
+        selectedStyle='highlight'
+        selectedColor='yellow'
+        popupWidth={POPUP_W}
+        popupHeight={POPUP_H}
+        onHighlight={vi.fn()}
+        onDismiss={vi.fn()}
+      />
+    </Wrapper>,
+  );
+};
+
+// ── Lifecycle ───────────────────────────────────────────────────────────
+
+beforeAll(async () => {
+  await page.viewport(800, 600);
+});
+
+beforeEach(() => {
+  mockUserColors = [];
+});
+
+afterEach(() => {
+  cleanup();
+});
+
+// ── Tests ───────────────────────────────────────────────────────────────
+
+describe('AnnotationPopup layout screenshot', () => {
+  it('default 5 colors — compact color strip, large gap', async () => {
+    const { container } = renderPopup();
+    const wrapper = container.firstElementChild as HTMLElement;
+    await expectElement(page.elementLocator(wrapper)).toMatchScreenshot(
+      'annotation-popup-5-colors',
+    );
+  });
+
+  it('5+5 user colors — color strip grows, gap shrinks', async () => {
+    const { container } = renderPopup([
+      { hex: '#f97316' },
+      { hex: '#06b6d4' },
+      { hex: '#ec4899' },
+      { hex: '#14b8a6' },
+      { hex: '#f43f5e' },
+    ]);
+    const wrapper = container.firstElementChild as HTMLElement;
+    await expectElement(page.elementLocator(wrapper)).toMatchScreenshot(
+      'annotation-popup-10-colors',
+    );
+  });
+
+  it('5+10 user colors — color strip at max, overflow scrolls', async () => {
+    const { container } = renderPopup([
+      { hex: '#f97316' },
+      { hex: '#06b6d4' },
+      { hex: '#ec4899' },
+      { hex: '#14b8a6' },
+      { hex: '#f43f5e' },
+      { hex: '#a855f7' },
+      { hex: '#84cc16' },
+      { hex: '#0ea5e9' },
+      { hex: '#e11d48' },
+      { hex: '#6366f1' },
+    ]);
+    const wrapper = container.firstElementChild as HTMLElement;
+    await expectElement(page.elementLocator(wrapper)).toMatchScreenshot(
+      'annotation-popup-15-colors',
+    );
+  });
+});

apps/readest-app/src/app/reader/components/annotator/HighlightOptions.tsx

@@ -173,7 +173,7 @@ const HighlightOptions: React.FC<HighlightOptionsProps> = ({
   return (
     <div
       className={clsx(
-        'highlight-options absolute flex items-center gap-4',
+        'highlight-options absolute flex items-center justify-between gap-4',
         isVertical ? 'flex-col' : 'flex-row',
       )}
       style={{
@@ -249,9 +249,7 @@ const HighlightOptions: React.FC<HighlightOptionsProps> = ({
         {...stripPointerHandlers}
         className={clsx(
           'not-eink:bg-gray-700 eink-bordered flex items-center gap-2 rounded-3xl',
-          isVertical
-            ? 'flex-col overflow-y-auto py-2'
-            : 'min-w-0 flex-1 flex-row overflow-x-auto px-2',
+          isVertical ? 'flex-col overflow-y-auto py-2' : 'min-w-0 flex-row overflow-x-auto px-2',
           !isVertical && 'cursor-grab',
           !isVertical && isDraggingColorStrip && 'cursor-grabbing',
         )}

apps/readest-app/src/components/settings/color/HighlightColorsEditor.tsx

@@ -1,4 +1,5 @@
-import React, { useEffect, useState } from 'react';
+import clsx from 'clsx';
+import React, { useEffect, useRef, useState } from 'react';
 import { MdClose } from 'react-icons/md';
 import {
   DEFAULT_HIGHLIGHT_COLORS,
@@ -25,40 +26,90 @@ interface HighlightColorsEditorProps {
 }
 
 /**
- * Text input that commits on blur instead of on every keystroke, so we don't
- * thrash the settings store while the user is typing a label.
+ * Popover that appears on click of a color circle, allowing the user to
+ * view and edit the label for that color.
  */
-const LabelInput: React.FC<{
+const LabelPopover: React.FC<{
   label: string;
   onCommit: (next: string) => void;
-  placeholder: string;
-  className: string;
-}> = ({ label, onCommit, placeholder, className }) => {
+  onClose: () => void;
+}> = ({ label, onCommit, onClose }) => {
+  const _ = useTranslation();
   const [draft, setDraft] = useState(label);
+  const inputRef = useRef<HTMLInputElement>(null);
+  const popoverRef = useRef<HTMLDivElement>(null);
 
   useEffect(() => {
-    setDraft(label);
-  }, [label]);
+    inputRef.current?.focus();
+    inputRef.current?.select();
+  }, []);
+
+  useEffect(() => {
+    const handleClickOutside = (e: MouseEvent) => {
+      if (popoverRef.current && !popoverRef.current.contains(e.target as Node)) {
+        commit();
+      }
+    };
+    document.addEventListener('mousedown', handleClickOutside);
+    return () => document.removeEventListener('mousedown', handleClickOutside);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [draft]);
 
   const commit = () => {
     const trimmed = draft.trim();
     if (trimmed !== label) onCommit(trimmed);
+    onClose();
   };
 
   return (
-    <input
-      type='text'
-      value={draft}
-      onChange={(e) => setDraft(e.target.value)}
-      onBlur={commit}
-      onKeyDown={(e) => {
-        if (e.key === 'Enter') (e.currentTarget as HTMLInputElement).blur();
-      }}
-      placeholder={placeholder}
-      maxLength={20}
-      className={className}
-      title={draft}
-    />
+    <div
+      ref={popoverRef}
+      className='bg-base-100 border-base-300 absolute -top-9 left-1/2 z-50 -translate-x-1/2 rounded-md border px-1 py-0.5 shadow-lg'
+    >
+      <input
+        ref={inputRef}
+        type='text'
+        value={draft}
+        onChange={(e) => setDraft(e.target.value)}
+        onKeyDown={(e) => {
+          if (e.key === 'Enter') commit();
+          if (e.key === 'Escape') onClose();
+        }}
+        placeholder={_('Name')}
+        maxLength={20}
+        className='bg-base-100 w-20 text-center text-xs outline-none'
+      />
+    </div>
+  );
+};
+
+/**
+ * A color circle that shows label on hover and opens a label editor on click.
+ */
+const ColorCircle: React.FC<{
+  hex: string;
+  label: string;
+  highlightPreviewStyle: React.CSSProperties;
+  onLabelCommit: (next: string) => void;
+}> = ({ hex, label, highlightPreviewStyle, onLabelCommit }) => {
+  const [editing, setEditing] = useState(false);
+
+  return (
+    <div className='relative flex flex-col items-center'>
+      {editing && (
+        <LabelPopover label={label} onCommit={onLabelCommit} onClose={() => setEditing(false)} />
+      )}
+      <div
+        className='border-base-300 h-8 w-8 cursor-pointer rounded-full border-2 shadow-sm'
+        title={label || undefined}
+        onClick={() => setEditing(true)}
+      >
+        <div
+          className='h-full w-full rounded-full'
+          style={{ backgroundColor: hex, ...highlightPreviewStyle }}
+        />
+      </div>
+    </div>
   );
 };
 
@@ -77,7 +128,6 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
 }) => {
   const _ = useTranslation();
   const [newColor, setNewColor] = useState('#808080');
-  const [newColorLabel, setNewColorLabel] = useState('');
 
   const highlightPreviewStyle: React.CSSProperties = {
     opacity: highlightOpacity,
@@ -113,9 +163,7 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
     const hex = normalizeHex(newColor);
     if (!hex.startsWith('#')) return;
     if (userHighlightColors.some((entry) => entry.hex === hex)) return;
-    const label = newColorLabel.trim();
-    onUserHighlightColorsChange([...userHighlightColors, label ? { hex, label } : { hex }]);
-    setNewColorLabel('');
+    onUserHighlightColorsChange([...userHighlightColors, { hex }]);
   };
 
   const handleDeleteUserColor = (hex: string) => {
@@ -127,7 +175,6 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
     const oldKey = normalizeHex(oldHex);
     const newKey = normalizeHex(newHex);
     if (oldKey === newKey) return;
-    // Drop the rename if it collides with another existing color.
     if (userHighlightColors.some((entry) => entry.hex === newKey)) return;
     onUserHighlightColorsChange(
       userHighlightColors.map((entry) =>
@@ -144,26 +191,17 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
     <div>
       <h2 className='mb-2 font-medium'>{_('Highlight Colors')}</h2>
       <div className='card border-base-200 bg-base-100 overflow-visible border shadow'>
-        <div className='grid grid-cols-2 gap-3 p-4 sm:grid-cols-3 md:grid-cols-4 lg:grid-cols-5'>
+        <div className='grid grid-cols-3 gap-3 p-4 sm:grid-cols-5'>
           {DEFAULT_HIGHLIGHT_COLORS.map((color, index, array) => {
             const position = index === 0 ? 'left' : index === array.length - 1 ? 'right' : 'center';
             return (
-              <div key={color} className='flex min-w-0 flex-col items-center gap-2'>
-                <LabelInput
+              <div key={color} className='flex flex-col items-center gap-2'>
+                <ColorCircle
+                  hex={customHighlightColors[color]!}
                   label={defaultHighlightLabels[color] ?? ''}
-                  onCommit={(next) => handleDefaultLabelChange(color, next)}
-                  placeholder={_('Name')}
-                  className='input input-xs bg-base-100 border-base-200/75 h-6 w-full min-w-0 max-w-24 text-center text-xs'
+                  highlightPreviewStyle={highlightPreviewStyle}
+                  onLabelCommit={(next) => handleDefaultLabelChange(color, next)}
                 />
-                <div className='border-base-300 h-8 w-8 rounded-full border-2 shadow-sm'>
-                  <div
-                    className='h-full w-full rounded-full'
-                    style={{
-                      backgroundColor: customHighlightColors[color],
-                      ...highlightPreviewStyle,
-                    }}
-                  />
-                </div>
                 <ColorInput
                   label=''
                   value={customHighlightColors[color]!}
@@ -177,11 +215,16 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
         </div>
 
         <div className='border-base-200 border-t p-4'>
-          <div className='mb-2 flex items-center justify-between'>
+          <div
+            className={clsx(
+              'flex items-center justify-between',
+              userHighlightColors.length > 0 && 'mb-4',
+            )}
+          >
             <span className='font-normal'>
               {_('Custom Colors')} ({userHighlightColors.length}/{MAX_USER_HIGHLIGHT_COLORS})
             </span>
-            <div className='flex flex-wrap items-center gap-2'>
+            <div className='flex items-center gap-2'>
               <div className='border-base-300 h-6 w-6 rounded-full border-2 shadow-sm'>
                 <div
                   className='h-full w-full rounded-full'
@@ -195,14 +238,6 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
                 pickerPosition='right'
                 onChange={setNewColor}
               />
-              <input
-                type='text'
-                value={newColorLabel}
-                onChange={(e) => setNewColorLabel(e.target.value)}
-                placeholder={_('Name')}
-                maxLength={20}
-                className='input input-xs bg-base-100 border-base-200/75 h-6 w-24 text-center text-xs'
-              />
               <button
                 onClick={handleAddUserColor}
                 disabled={
@@ -218,19 +253,13 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
           {userHighlightColors.length > 0 && (
             <div className='grid grid-cols-3 gap-3 sm:grid-cols-5'>
               {userHighlightColors.map(({ hex, label }, index) => (
-                <div key={hex} className='group relative flex min-w-0 flex-col items-center gap-2'>
-                  <LabelInput
+                <div key={hex} className='group relative flex flex-col items-center gap-2'>
+                  <ColorCircle
+                    hex={hex}
                     label={label ?? ''}
-                    onCommit={(next) => handleUserLabelChange(hex, next)}
-                    placeholder={_('Name')}
-                    className='input input-xs bg-base-100 border-base-200/75 h-6 w-full min-w-0 max-w-24 text-center text-xs'
+                    highlightPreviewStyle={highlightPreviewStyle}
+                    onLabelCommit={(next) => handleUserLabelChange(hex, next)}
                   />
-                  <div className='border-base-300 h-8 w-8 rounded-full border-2 shadow-sm'>
-                    <div
-                      className='h-full w-full rounded-full'
-                      style={{ backgroundColor: hex, ...highlightPreviewStyle }}
-                    />
-                  </div>
                   <ColorInput
                     label=''
                     value={hex}
@@ -251,15 +280,17 @@ const HighlightColorsEditor: React.FC<HighlightColorsEditorProps> = ({
           )}
         </div>
 
-        <NumberInput
-          label={_('Opacity')}
-          value={highlightOpacity}
-          onChange={onOpacityChange}
-          disabled={isEink}
-          min={0}
-          max={1}
-          step={0.1}
-        />
+        <div className='border-base-200 border-t'>
+          <NumberInput
+            label={_('Opacity')}
+            value={highlightOpacity}
+            onChange={onOpacityChange}
+            disabled={isEink}
+            min={0}
+            max={1}
+            step={0.1}
+          />
+        </div>
       </div>
     </div>
   );

apps/readest-app/vitest.browser.config.mts

@@ -1,4 +1,5 @@
 import tsconfigPaths from 'vite-tsconfig-paths';
+import react from '@vitejs/plugin-react';
 import { defineConfig } from 'vitest/config';
 import { playwright } from '@vitest/browser-playwright';
 import { loadEnvFile } from './vitest.env.mts';
@@ -7,7 +8,7 @@ import { loadEnvFile } from './vitest.env.mts';
 const env = { ...loadEnvFile('.env'), ...loadEnvFile('.env.web') };
 
 export default defineConfig({
-  plugins: [tsconfigPaths()],
+  plugins: [tsconfigPaths(), react()],
   define: {
     'process.env': JSON.stringify(env),
   },
@@ -42,14 +43,33 @@ export default defineConfig({
     },
   },
   test: {
-    include: ['src/**/*.browser.test.ts'],
+    include: ['src/**/*.browser.test.ts', 'src/**/*.browser.test.tsx'],
     onConsoleLog(_log, type) {
       if (type === 'stdout') return false;
     },
     browser: {
       enabled: true,
-      provider: playwright(),
+      headless: true,
+      provider: playwright({
+        contextOptions: {
+          viewport: { width: 1920, height: 1080 },
+          deviceScaleFactor: 2,
+        },
+      }),
       instances: [{ browser: 'chromium' }],
+      expect: {
+        toMatchScreenshot: {
+          comparatorName: 'pixelmatch',
+          comparatorOptions: {
+            threshold: 0.1,
+            allowedMismatchedPixelRatio: 0.02,
+          },
+          // Strip platform from the path so one baseline works on macOS and Linux.
+          // The path is relative to the project root (not the test file).
+          resolveScreenshotPath: ({ arg, browserName, ext, testFileDirectory, testFileName }) =>
+            `${testFileDirectory}/__screenshots__/${testFileName}/${arg}-${browserName}${ext}`,
+        },
+      },
     },
   },
 });

apps/readest-app/vitest.config.mts

@@ -21,6 +21,7 @@ export default defineConfig({
       '**/dist/**',
       '**/.claude/**',
       '**/*.browser.test.ts',
+      '**/*.browser.test.tsx',
       '**/*.tauri.test.ts',
     ],
     coverage: {