diff --git a/apps/readest-app/.claude/memory/MEMORY.md b/apps/readest-app/.claude/memory/MEMORY.md index 3236ed6f..30c1379b 100644 --- a/apps/readest-app/.claude/memory/MEMORY.md +++ b/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 ` 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 diff --git a/apps/readest-app/.claude/memory/feedback_test_file_filter.md b/apps/readest-app/.claude/memory/feedback_test_file_filter.md new file mode 100644 index 00000000..5833b3fb --- /dev/null +++ b/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 ` or `pnpm test:browser ` — no `--` separator. + +**Why:** Adding `--` before the path (e.g. `pnpm test:browser -- `) 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. diff --git a/apps/readest-app/.gitignore b/apps/readest-app/.gitignore index d4a8ec07..40b75e84 100644 --- a/apps/readest-app/.gitignore +++ b/apps/readest-app/.gitignore @@ -9,6 +9,7 @@ # testing /coverage .test-sandbox-node/ +.vitest-attachments/ # next.js /.next/ diff --git a/apps/readest-app/package.json b/apps/readest-app/package.json index 5d58ab0e..5af77a23 100644 --- a/apps/readest-app/package.json +++ b/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", diff --git a/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-10-colors-chromium.png b/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-10-colors-chromium.png new file mode 100644 index 00000000..9425895f Binary files /dev/null and b/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-10-colors-chromium.png differ diff --git a/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-15-colors-chromium.png b/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-15-colors-chromium.png new file mode 100644 index 00000000..9425895f Binary files /dev/null and b/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-15-colors-chromium.png differ diff --git a/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-5-colors-chromium.png b/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-5-colors-chromium.png new file mode 100644 index 00000000..321a52a3 Binary files /dev/null and b/apps/readest-app/src/__tests__/components/__screenshots__/annotation-popup-layout.browser.test.tsx/annotation-popup-5-colors-chromium.png differ diff --git a/apps/readest-app/src/__tests__/components/annotation-popup-layout.browser.test.tsx b/apps/readest-app/src/__tests__/components/annotation-popup-layout.browser.test.tsx new file mode 100644 index 00000000..65d682a6 --- /dev/null +++ b/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, + 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 }; + +/** + * 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 }) => ( +
+ {children} +
+); + +const renderPopup = (userColors: UserHighlightColor[] = []) => { + mockUserColors = userColors; + return render( + + + , + ); +}; + +// ── 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', + ); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/annotator/HighlightOptions.tsx b/apps/readest-app/src/app/reader/components/annotator/HighlightOptions.tsx index 442ed0ec..850ea93b 100644 --- a/apps/readest-app/src/app/reader/components/annotator/HighlightOptions.tsx +++ b/apps/readest-app/src/app/reader/components/annotator/HighlightOptions.tsx @@ -173,7 +173,7 @@ const HighlightOptions: React.FC = ({ return (
= ({ {...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', )} diff --git a/apps/readest-app/src/components/settings/color/HighlightColorsEditor.tsx b/apps/readest-app/src/components/settings/color/HighlightColorsEditor.tsx index 537fe487..a2bfb287 100644 --- a/apps/readest-app/src/components/settings/color/HighlightColorsEditor.tsx +++ b/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(null); + const popoverRef = useRef(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 ( - 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} - /> +
+ 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' + /> +
+ ); +}; + +/** + * 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 ( +
+ {editing && ( + setEditing(false)} /> + )} +
setEditing(true)} + > +
+
+
); }; @@ -77,7 +128,6 @@ const HighlightColorsEditor: React.FC = ({ }) => { 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 = ({ 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 = ({ 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 = ({

{_('Highlight Colors')}

-
+
{DEFAULT_HIGHLIGHT_COLORS.map((color, index, array) => { const position = index === 0 ? 'left' : index === array.length - 1 ? 'right' : 'center'; return ( -
- + 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)} /> -
-
-
= ({
-
+
0 && 'mb-4', + )} + > {_('Custom Colors')} ({userHighlightColors.length}/{MAX_USER_HIGHLIGHT_COLORS}) -
+
= ({ pickerPosition='right' onChange={setNewColor} /> - 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' - />