From 76b546c8f6dea74f289da134dc988007fc574720 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 25 Apr 2026 17:10:54 -0700 Subject: [PATCH] fix: honor subtitle annotation style priority --- ...-subtitle-tokens-to-honor-subtitleStyle.md | 32 ++++ ...color-priority-after-typography-cleanup.md | 32 ++++ changes/297-subtitle-annotation-colors.md | 4 + src/renderer/style.css | 118 +++++++++------ src/renderer/subtitle-render.test.ts | 140 ++++++++++++++++-- 5 files changed, 271 insertions(+), 55 deletions(-) create mode 100644 backlog/tasks/task-294 - Fix-annotated-subtitle-tokens-to-honor-subtitleStyle.md create mode 100644 backlog/tasks/task-297 - Fix-subtitle-annotation-color-priority-after-typography-cleanup.md create mode 100644 changes/297-subtitle-annotation-colors.md diff --git a/backlog/tasks/task-294 - Fix-annotated-subtitle-tokens-to-honor-subtitleStyle.md b/backlog/tasks/task-294 - Fix-annotated-subtitle-tokens-to-honor-subtitleStyle.md new file mode 100644 index 00000000..c07f6a85 --- /dev/null +++ b/backlog/tasks/task-294 - Fix-annotated-subtitle-tokens-to-honor-subtitleStyle.md @@ -0,0 +1,32 @@ +--- +id: TASK-294 +title: Fix annotated subtitle tokens to honor subtitleStyle +status: Done +assignee: [] +created_date: '2026-04-25 23:04' +updated_date: '2026-04-25 23:07' +labels: + - subtitles + - renderer +dependencies: [] +priority: medium +--- + +## Description + + +Annotated token spans should inherit the configured subtitleStyle typography and only use annotation metadata to change token color. + + +## Acceptance Criteria + +- [x] #1 Tokenized/annotated subtitles preserve configured base subtitle typography such as font family, size, weight, line height, letter spacing, text rendering, and text shadow. +- [x] #2 Known/N+1/JLPT/frequency/name-match annotations affect token color only, plus existing token metadata/hover affordances. +- [x] #3 A renderer regression test covers annotated token rendering with custom subtitleStyle. + + +## Final Summary + + +Updated renderer subtitle annotation CSS so known/N+1/name/JLPT/frequency classes no longer override typography with token-specific shadows, underlines, padding, or hover font-weight. Added regression coverage using the user's custom subtitleStyle shape to verify annotated token spans inherit base typography and annotation CSS changes token color only. Verified with `bun test src/renderer/subtitle-render.test.ts`, `bun run typecheck`, and `bun run test:fast`. + diff --git a/backlog/tasks/task-297 - Fix-subtitle-annotation-color-priority-after-typography-cleanup.md b/backlog/tasks/task-297 - Fix-subtitle-annotation-color-priority-after-typography-cleanup.md new file mode 100644 index 00000000..52e503bc --- /dev/null +++ b/backlog/tasks/task-297 - Fix-subtitle-annotation-color-priority-after-typography-cleanup.md @@ -0,0 +1,32 @@ +--- +id: TASK-297 +title: Fix subtitle annotation color priority after typography cleanup +status: Done +assignee: [] +created_date: '2026-04-25 23:44' +updated_date: '2026-04-25 23:46' +labels: + - subtitles + - renderer +dependencies: [] +priority: high +--- + +## Description + + +Known-word and frequency subtitle token colors should keep their configured priority after annotation CSS stopped using JLPT underlines. + + +## Acceptance Criteria + +- [x] #1 Known-word token color takes priority over JLPT and frequency color classes. +- [x] #2 Frequency single-mode token color takes priority over JLPT color classes when frequency annotation is active. +- [x] #3 Regression coverage verifies CSS selectors do not allow JLPT color rules to override higher-priority annotation colors. + + +## Final Summary + + +Scoped JLPT token color selectors so they only apply when no higher-priority known-word, N+1, name-match, or frequency class is present. This keeps known words green and frequency single-mode tokens using the single frequency color instead of being visually overridden by JLPT colors. Added CSS regression assertions for this priority behavior. Verified with `bun test src/renderer/subtitle-render.test.ts`, `bun run typecheck`, and `bun run test:fast`. + diff --git a/changes/297-subtitle-annotation-colors.md b/changes/297-subtitle-annotation-colors.md new file mode 100644 index 00000000..81b59c13 --- /dev/null +++ b/changes/297-subtitle-annotation-colors.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Fixed annotated subtitle token colors so `subtitleStyle` typography is preserved and higher-priority known-word/frequency colors are not overridden by JLPT colors. diff --git a/src/renderer/style.css b/src/renderer/style.css index cbf7202d..5d608162 100644 --- a/src/renderer/style.css +++ b/src/renderer/style.css @@ -783,88 +783,121 @@ body.settings-modal-open [data-subminer-yomitan-popup-host='true'] { #subtitleRoot .word.word-known { color: var(--subtitle-known-word-color, #a6da95); - text-shadow: 0 0 6px rgba(166, 218, 149, 0.35); } #subtitleRoot .word.word-n-plus-one { color: var(--subtitle-n-plus-one-color, #c6a0f6); - text-shadow: 0 0 6px rgba(198, 160, 246, 0.35); } #subtitleRoot .word.word-name-match { color: var(--subtitle-name-match-color, #f5bde6); - text-shadow: 0 0 6px rgba(245, 189, 230, 0.35); } -#subtitleRoot .word.word-jlpt-n1 { - --subtitle-jlpt-underline-color: var(--subtitle-jlpt-n1-color, #ed8796); - border-bottom: 2px solid var(--subtitle-jlpt-underline-color); - padding-bottom: 1px; - box-decoration-break: clone; - -webkit-box-decoration-break: clone; +#subtitleRoot + .word.word-jlpt-n1:not( + :is( + .word-known, + .word-n-plus-one, + .word-name-match, + .word-frequency-single, + .word-frequency-band-1, + .word-frequency-band-2, + .word-frequency-band-3, + .word-frequency-band-4, + .word-frequency-band-5 + ) + ) { + color: var(--subtitle-jlpt-n1-color, #ed8796); } #subtitleRoot .word.word-jlpt-n1[data-jlpt-level]::after { color: var(--subtitle-jlpt-n1-color, #ed8796); } -#subtitleRoot .word.word-jlpt-n2 { - --subtitle-jlpt-underline-color: var(--subtitle-jlpt-n2-color, #f5a97f); - border-bottom: 2px solid var(--subtitle-jlpt-underline-color); - padding-bottom: 1px; - box-decoration-break: clone; - -webkit-box-decoration-break: clone; +#subtitleRoot + .word.word-jlpt-n2:not( + :is( + .word-known, + .word-n-plus-one, + .word-name-match, + .word-frequency-single, + .word-frequency-band-1, + .word-frequency-band-2, + .word-frequency-band-3, + .word-frequency-band-4, + .word-frequency-band-5 + ) + ) { + color: var(--subtitle-jlpt-n2-color, #f5a97f); } #subtitleRoot .word.word-jlpt-n2[data-jlpt-level]::after { color: var(--subtitle-jlpt-n2-color, #f5a97f); } -#subtitleRoot .word.word-jlpt-n3 { - --subtitle-jlpt-underline-color: var(--subtitle-jlpt-n3-color, #f9e2af); - border-bottom: 2px solid var(--subtitle-jlpt-underline-color); - padding-bottom: 1px; - box-decoration-break: clone; - -webkit-box-decoration-break: clone; +#subtitleRoot + .word.word-jlpt-n3:not( + :is( + .word-known, + .word-n-plus-one, + .word-name-match, + .word-frequency-single, + .word-frequency-band-1, + .word-frequency-band-2, + .word-frequency-band-3, + .word-frequency-band-4, + .word-frequency-band-5 + ) + ) { + color: var(--subtitle-jlpt-n3-color, #f9e2af); } #subtitleRoot .word.word-jlpt-n3[data-jlpt-level]::after { color: var(--subtitle-jlpt-n3-color, #f9e2af); } -#subtitleRoot .word.word-jlpt-n4 { - --subtitle-jlpt-underline-color: var(--subtitle-jlpt-n4-color, #a6e3a1); - border-bottom: 2px solid var(--subtitle-jlpt-underline-color); - padding-bottom: 1px; - box-decoration-break: clone; - -webkit-box-decoration-break: clone; +#subtitleRoot + .word.word-jlpt-n4:not( + :is( + .word-known, + .word-n-plus-one, + .word-name-match, + .word-frequency-single, + .word-frequency-band-1, + .word-frequency-band-2, + .word-frequency-band-3, + .word-frequency-band-4, + .word-frequency-band-5 + ) + ) { + color: var(--subtitle-jlpt-n4-color, #a6e3a1); } #subtitleRoot .word.word-jlpt-n4[data-jlpt-level]::after { color: var(--subtitle-jlpt-n4-color, #a6e3a1); } -#subtitleRoot .word.word-jlpt-n5 { - --subtitle-jlpt-underline-color: var(--subtitle-jlpt-n5-color, #8aadf4); - border-bottom: 2px solid var(--subtitle-jlpt-underline-color); - padding-bottom: 1px; - box-decoration-break: clone; - -webkit-box-decoration-break: clone; +#subtitleRoot + .word.word-jlpt-n5:not( + :is( + .word-known, + .word-n-plus-one, + .word-name-match, + .word-frequency-single, + .word-frequency-band-1, + .word-frequency-band-2, + .word-frequency-band-3, + .word-frequency-band-4, + .word-frequency-band-5 + ) + ) { + color: var(--subtitle-jlpt-n5-color, #8aadf4); } #subtitleRoot .word.word-jlpt-n5[data-jlpt-level]::after { color: var(--subtitle-jlpt-n5-color, #8aadf4); } -#subtitleRoot .word.word-frequency-single, -#subtitleRoot .word.word-frequency-band-1, -#subtitleRoot .word.word-frequency-band-2, -#subtitleRoot .word.word-frequency-band-3, -#subtitleRoot .word.word-frequency-band-4, -#subtitleRoot .word.word-frequency-band-5 { - text-shadow: 0 0 6px rgba(255, 255, 255, 0.3); -} - #subtitleRoot .word.word-frequency-single { color: var(--subtitle-frequency-single-color, #f5a97f); } @@ -912,7 +945,6 @@ body.settings-modal-open [data-subminer-yomitan-popup-host='true'] { #subtitleRoot .word.word-frequency-band-5:hover { background: var(--subtitle-hover-token-background-color, rgba(54, 58, 79, 0.84)); border-radius: 3px; - font-weight: 800; } #subtitleRoot .word.word-known .c:hover, diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index 04fc6262..c12c8ff2 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -220,6 +220,22 @@ function normalizeCssSelector(selector: string): string { .trim(); } +function buildJlptColorSelector(level: number): string { + const higherPriorityClasses = [ + '.word-known', + '.word-n-plus-one', + '.word-name-match', + '.word-frequency-single', + '.word-frequency-band-1', + '.word-frequency-band-2', + '.word-frequency-band-3', + '.word-frequency-band-4', + '.word-frequency-band-5', + ].join(', '); + + return `#subtitleRoot .word.word-jlpt-n${level}:not(:is(${higherPriorityClasses}))`; +} + test('computeWordClass preserves known and n+1 classes while adding JLPT classes', () => { const knownJlpt = createToken({ isKnown: true, @@ -410,6 +426,96 @@ test('applySubtitleStyle stores secondary background styles in hover-aware css v } }); +test('annotated subtitle tokens inherit configured base subtitle typography', () => { + const restoreDocument = installFakeDocument(); + try { + const subtitleRoot = new FakeElement('div'); + const subtitleContainer = new FakeElement('div'); + const secondarySubRoot = new FakeElement('div'); + const secondarySubContainer = new FakeElement('div'); + const ctx = { + state: createRendererState(), + dom: { + subtitleRoot, + subtitleContainer, + secondarySubRoot, + secondarySubContainer, + }, + } as never; + + const renderer = createSubtitleRenderer(ctx); + renderer.applySubtitleStyle({ + fontFamily: 'M PLUS 1 Medium, Source Han Sans JP, Noto Sans CJK JP', + fontSize: 35, + fontColor: '#cad3f5', + fontWeight: 700, + lineHeight: 1.35, + letterSpacing: '-0.01em', + textRendering: 'geometricPrecision', + textShadow: '3px 0 0 #000, -3px 0 0 #000, 0 3px 0 #000, 0 -3px 0 #000, 2px 2px 0 #000', + frequencyDictionary: { + enabled: true, + topX: 10000, + mode: 'single', + singleColor: '#f5a97f', + }, + enableJlpt: true, + jlptColors: { + N1: '#ed8796', + N2: '#f5a97f', + N3: '#f9e2af', + N4: '#a6e3a1', + N5: '#8aadf4', + }, + nPlusOneColor: '#c6a0f6', + knownWordColor: '#a6da95', + } as never); + + renderer.renderSubtitle({ + text: 'お礼をされるようなことしてない', + tokens: [ + createToken({ surface: 'お礼', isKnown: true }), + createToken({ surface: 'を' }), + createToken({ surface: 'される', jlptLevel: 'N4' }), + createToken({ surface: 'ような', frequencyRank: 15 }), + ], + }); + + const rootStyle = subtitleRoot.style as unknown as Record; + assert.equal(rootStyle.fontFamily, 'M PLUS 1 Medium, Source Han Sans JP, Noto Sans CJK JP'); + assert.equal(rootStyle.fontSize, '35px'); + assert.equal(rootStyle.color, '#cad3f5'); + assert.equal(rootStyle.fontWeight, '700'); + assert.equal(rootStyle.lineHeight, '1.35'); + assert.equal(rootStyle.letterSpacing, '-0.01em'); + assert.equal(rootStyle.textRendering, 'geometricPrecision'); + assert.match(rootStyle.textShadow ?? '', /3px 0 0 #000/); + + const wordNodes = collectWordNodes(subtitleRoot); + assert.deepEqual( + wordNodes.map((node) => [node.textContent, node.className]), + [ + ['お礼', 'word word-known'], + ['を', 'word'], + ['される', 'word word-jlpt-n4'], + ['ような', 'word word-frequency-single'], + ], + ); + for (const wordNode of wordNodes) { + const tokenStyle = wordNode.style as unknown as Record; + assert.equal(tokenStyle.fontFamily, undefined); + assert.equal(tokenStyle.fontSize, undefined); + assert.equal(tokenStyle.fontWeight, undefined); + assert.equal(tokenStyle.lineHeight, undefined); + assert.equal(tokenStyle.letterSpacing, undefined); + assert.equal(tokenStyle.textRendering, undefined); + assert.equal(tokenStyle.textShadow, undefined); + } + } finally { + restoreDocument(); + } +}); + test('computeWordClass adds frequency class for single mode when rank is within topX', () => { const token = createToken({ surface: '猫', @@ -749,7 +855,7 @@ test('shouldRenderTokenizedSubtitle enables token rendering when tokens exist', assert.equal(shouldRenderTokenizedSubtitle(0), false); }); -test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { +test('subtitle annotation CSS changes token color without overriding typography', () => { const distCssPath = path.join(process.cwd(), 'dist', 'renderer', 'style.css'); const srcCssPath = path.join(process.cwd(), 'src', 'renderer', 'style.css'); @@ -763,17 +869,27 @@ test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { const cssText = fs.readFileSync(cssPath, 'utf-8'); for (let level = 1; level <= 5; level += 1) { - const block = extractClassBlock(cssText, `#subtitleRoot .word.word-jlpt-n${level}`); + const plainJlptBlock = extractClassBlock(cssText, `#subtitleRoot .word.word-jlpt-n${level}`); + assert.doesNotMatch(plainJlptBlock, /(?:^|\n)\s*color\s*:/m); + + const block = extractClassBlock(cssText, buildJlptColorSelector(level)); assert.ok(block.length > 0, `word-jlpt-n${level} class should exist`); - assert.match( - block, - new RegExp(`--subtitle-jlpt-underline-color:\\s*var\\(--subtitle-jlpt-n${level}-color,`), - ); - assert.match(block, /border-bottom:\s*2px solid var\(--subtitle-jlpt-underline-color\);/); - assert.match(block, /padding-bottom:\s*1px;/); - assert.match(block, /box-decoration-break:\s*clone;/); - assert.match(block, /-webkit-box-decoration-break:\s*clone;/); - assert.doesNotMatch(block, /(?:^|\n)\s*color\s*:/m); + assert.match(block, new RegExp(`color:\\s*var\\(--subtitle-jlpt-n${level}-color,`)); + assert.doesNotMatch(block, /border-bottom\s*:/); + assert.doesNotMatch(block, /padding-bottom\s*:/); + assert.doesNotMatch(block, /box-decoration-break\s*:/); + assert.doesNotMatch(block, /-webkit-box-decoration-break\s*:/); + assert.doesNotMatch(block, /text-shadow\s*:/); + } + + for (const selector of [ + '#subtitleRoot .word.word-known', + '#subtitleRoot .word.word-n-plus-one', + '#subtitleRoot .word.word-name-match', + ]) { + const block = extractClassBlock(cssText, selector); + assert.match(block, /color:\s*var\(/); + assert.doesNotMatch(block, /text-shadow\s*:/); } for (let band = 1; band <= 5; band += 1) { @@ -873,7 +989,7 @@ test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { /background:\s*var\(--subtitle-hover-token-background-color,\s*rgba\(54,\s*58,\s*79,\s*0\.84\)\);/, ); assert.match(coloredWordHoverBlock, /border-radius:\s*3px;/); - assert.match(coloredWordHoverBlock, /font-weight:\s*800;/); + assert.doesNotMatch(coloredWordHoverBlock, /font-weight\s*:/); assert.doesNotMatch(coloredWordHoverBlock, /color:\s*var\(--subtitle-hover-token-color/); assert.doesNotMatch( coloredWordHoverBlock,