bug(viewer): "Annotieren anzeigen" text has insufficient contrast in light mode #341

New Issue

marcel · 2026-04-26T19:56:30+02:00

marcel commented

2026-04-26 19:56:30 +02:00

Bug

The "Annotieren anzeigen" toggle label in the PDF viewer is unreadable in light mode — the text color is too close to the background.

Steps to reproduce

Open any document with a PDF viewer.
Ensure light mode is active.
Observe the "Annotieren anzeigen" label.

Expected behavior

Text contrast ratio ≥ 4.5:1 against its background (WCAG 2.1 AA).

Actual behavior

Label appears washed out / nearly invisible in light mode.

Acceptance criteria

Given light mode, the "Annotieren anzeigen" label meets WCAG 2.1 AA contrast (≥ 4.5:1).
Given dark mode, the label remains legible.
Fix verified with browser DevTools accessibility contrast checker.

## Bug The "Annotieren anzeigen" toggle label in the PDF viewer is unreadable in light mode — the text color is too close to the background. ## Steps to reproduce 1. Open any document with a PDF viewer. 2. Ensure light mode is active. 3. Observe the "Annotieren anzeigen" label. ## Expected behavior Text contrast ratio ≥ 4.5:1 against its background (WCAG 2.1 AA). ## Actual behavior Label appears washed out / nearly invisible in light mode. ## Acceptance criteria - Given light mode, the "Annotieren anzeigen" label meets WCAG 2.1 AA contrast (≥ 4.5:1). - Given dark mode, the label remains legible. - Fix verified with browser DevTools accessibility contrast checker.

marcel added this to the (deleted) milestone 2026-04-26 19:56:30 +02:00

marcel added the P1-high bug ui labels 2026-04-26 19:56:54 +02:00

marcel commented

2026-04-26 19:59:04 +02:00

🎨 Leonie Voss — UI/UX Design Lead & Accessibility Advocate

Observations

Root cause is in PdfControls.svelte line 94: when showAnnotations is false, the inactive-state label gets text-accent. In light mode, --c-accent resolves to #a1dcd8, which the CSS file itself already documents as a WCAG failure: /* ⚠ accent — decorative use only ... Light mode: #a1dcd8 on white = 1.52:1 — WCAG FAIL. Never use as text colour. */. The codebase has the rule; the component broke it.
When showAnnotations is true, the label uses text-ink-2 (#4b5563, 7.6:1 on white — AA pass). Only the inactive/off state is broken.
The PDF control bar background is --c-pdf-ctrl = #d8d8d8 in light mode, not white. #a1dcd8 on #d8d8d8 is still below 1.8:1 — no improvement.
The button also uses bg-surface/10 as its background when inactive, which is essentially transparent. That doesn't rescue the contrast either.
Dark mode is fine: --c-accent becomes #00c7b1 in dark, which is a visible turquoise on the dark --c-pdf-ctrl surface (#011526). Contrast is approximately 6:1 — AA pass.
WCAG criterion violated: 1.4.3 Contrast (Minimum), Level AA — requires ≥ 4.5:1 for normal text.

Recommendations

Replace text-accent in the inactive branch with text-primary (--c-primary = #012851 in light, #a1dcd8 in dark — both pass AA). This aligns with the token's documented purpose: "Primary interactive."

The corrected class string for the button:

class="flex items-center gap-1.5 rounded px-2 py-1 font-sans text-xs transition {showAnnotations
    ? 'text-ink-2 hover:bg-surface/10'
    : 'bg-surface/10 text-primary'}"

Add an axe-playwright assertion to the PDF viewer page E2E test to catch future regressions in both light and dark mode (see Sara's comment on the testing gap).
Touch target: the button is px-2 py-1 (approximately 28×24px) — below the 44×44px minimum for the senior audience. While this is a pre-existing issue not introduced by this bug, the fix is a good moment to raise it. Consider px-3 py-2 min-h-[44px].

Open Decisions

Should the inactive state use text-primary (navy in light, mint in dark) for stronger visual distinction from the active state, or text-ink-2 (same as active) for visual consistency — and rely on the background fill alone to convey state? The current design pairs the fill (bg-surface/10) with the color change, but removing the color difference risks unclear state communication for color-blind users.

## 🎨 Leonie Voss — UI/UX Design Lead & Accessibility Advocate ### Observations - **Root cause is in `PdfControls.svelte` line 94**: when `showAnnotations` is `false`, the inactive-state label gets `text-accent`. In light mode, `--c-accent` resolves to `#a1dcd8`, which the CSS file itself already documents as a WCAG failure: `/* ⚠ accent — decorative use only ... Light mode: #a1dcd8 on white = 1.52:1 — WCAG FAIL. Never use as text colour. */`. The codebase has the rule; the component broke it. - **When `showAnnotations` is `true`**, the label uses `text-ink-2` (`#4b5563`, 7.6:1 on white — AA pass). Only the inactive/off state is broken. - The PDF control bar background is `--c-pdf-ctrl` = `#d8d8d8` in light mode, not white. `#a1dcd8` on `#d8d8d8` is still below 1.8:1 — no improvement. - The button also uses `bg-surface/10` as its background when inactive, which is essentially transparent. That doesn't rescue the contrast either. - **Dark mode is fine**: `--c-accent` becomes `#00c7b1` in dark, which is a visible turquoise on the dark `--c-pdf-ctrl` surface (`#011526`). Contrast is approximately 6:1 — AA pass. - WCAG criterion violated: **1.4.3 Contrast (Minimum), Level AA** — requires ≥ 4.5:1 for normal text. ### Recommendations - Replace `text-accent` in the inactive branch with `text-primary` (`--c-primary` = `#012851` in light, `#a1dcd8` in dark — both pass AA). This aligns with the token's documented purpose: "Primary interactive." - The corrected class string for the button: ```svelte class="flex items-center gap-1.5 rounded px-2 py-1 font-sans text-xs transition {showAnnotations ? 'text-ink-2 hover:bg-surface/10' : 'bg-surface/10 text-primary'}" ``` - Add an axe-playwright assertion to the PDF viewer page E2E test to catch future regressions in both light and dark mode (see Sara's comment on the testing gap). - Touch target: the button is `px-2 py-1` (approximately 28×24px) — below the 44×44px minimum for the senior audience. While this is a pre-existing issue not introduced by this bug, the fix is a good moment to raise it. Consider `px-3 py-2 min-h-[44px]`. ### Open Decisions - Should the inactive state use `text-primary` (navy in light, mint in dark) for **stronger visual distinction from the active state**, or `text-ink-2` (same as active) for **visual consistency** — and rely on the background fill alone to convey state? The current design pairs the fill (`bg-surface/10`) with the color change, but removing the color difference risks unclear state communication for color-blind users.

marcel commented

2026-04-26 19:59:19 +02:00

👨‍💻 Felix Brandt — Fullstack Developer

Observations

The offending line is in /frontend/src/lib/components/PdfControls.svelte at line 92–94:
```
class="flex items-center gap-1.5 rounded px-2 py-1 font-sans text-xs transition {showAnnotations
    ? 'text-ink-2 hover:bg-surface/10'
    : 'bg-surface/10 text-accent'}"
```
The text-accent token maps to --c-accent which is #a1dcd8 in light mode — explicitly flagged in layout.css as decorative-only and a WCAG fail on any light surface. The fix is a one-token swap.
The existing PdfViewer.svelte.spec.ts has three tests covering navigation and zoom controls, but zero tests for the annotation toggle. The toggle is conditionally rendered ({#if annotationCount > 0}) so it has never been exercised in the test suite.
PdfControls.svelte is a lean 125-line file — well within bounds. No splitting needed.
The component correctly uses $props() and passes specific typed props. No architecture issues.

Recommendations

One-line fix: replace text-accent with text-primary in PdfControls.svelte. --c-primary is navy in light mode (14.5:1 on white) and mint in dark mode (9:1 on dark surface) — passes AA in both modes with no additional work.

Write the failing test first (TDD red phase) in PdfViewer.svelte.spec.ts before touching the production code:

it('shows annotation toggle when annotations are present', async () => {
    render(PdfControls, {
        currentPage: 1, totalPages: 2, isLoaded: true,
        showAnnotations: false, annotationCount: 3,
        onPrev: vi.fn(), onNext: vi.fn(), onZoomIn: vi.fn(),
        onZoomOut: vi.fn(), onToggleAnnotations: vi.fn()
    });
    const btn = page.getByRole('button', { name: /annotierungen anzeigen/i });
    await expect.element(btn).toBeInTheDocument();
});

This test currently passes vacuously because the toggle is never rendered (no annotationCount prop is passed in existing tests). Add a dedicated PdfControls.svelte.spec.ts that mounts the isolated control component — it doesn't need the full PDF rendering machinery.

The test file name suggests it tests PdfViewer (the whole rendering component), but the annotation toggle lives in PdfControls. Consider extracting a PdfControls.svelte.spec.ts for control-bar logic independent of PDF rendering.

## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - The offending line is in `/frontend/src/lib/components/PdfControls.svelte` at line 92–94: ```svelte class="flex items-center gap-1.5 rounded px-2 py-1 font-sans text-xs transition {showAnnotations ? 'text-ink-2 hover:bg-surface/10' : 'bg-surface/10 text-accent'}" ``` The `text-accent` token maps to `--c-accent` which is `#a1dcd8` in light mode — explicitly flagged in `layout.css` as decorative-only and a WCAG fail on any light surface. The fix is a one-token swap. - The existing `PdfViewer.svelte.spec.ts` has three tests covering navigation and zoom controls, but **zero tests for the annotation toggle**. The toggle is conditionally rendered (`{#if annotationCount > 0}`) so it has never been exercised in the test suite. - `PdfControls.svelte` is a lean 125-line file — well within bounds. No splitting needed. - The component correctly uses `$props()` and passes specific typed props. No architecture issues. ### Recommendations - **One-line fix**: replace `text-accent` with `text-primary` in `PdfControls.svelte`. `--c-primary` is navy in light mode (14.5:1 on white) and mint in dark mode (9:1 on dark surface) — passes AA in both modes with no additional work. - **Write the failing test first** (TDD red phase) in `PdfViewer.svelte.spec.ts` before touching the production code: ```typescript it('shows annotation toggle when annotations are present', async () => { render(PdfControls, { currentPage: 1, totalPages: 2, isLoaded: true, showAnnotations: false, annotationCount: 3, onPrev: vi.fn(), onNext: vi.fn(), onZoomIn: vi.fn(), onZoomOut: vi.fn(), onToggleAnnotations: vi.fn() }); const btn = page.getByRole('button', { name: /annotierungen anzeigen/i }); await expect.element(btn).toBeInTheDocument(); }); ``` This test currently passes vacuously because the toggle is never rendered (no `annotationCount` prop is passed in existing tests). Add a dedicated `PdfControls.svelte.spec.ts` that mounts the isolated control component — it doesn't need the full PDF rendering machinery. - The test file name suggests it tests `PdfViewer` (the whole rendering component), but the annotation toggle lives in `PdfControls`. Consider extracting a `PdfControls.svelte.spec.ts` for control-bar logic independent of PDF rendering.

marcel commented

2026-04-26 19:59:28 +02:00

🏗️ Markus Keller — Application Architect

Observations

This is a purely presentational bug with no architectural surface — the fix is a single token swap in one component. No module boundary violations, no layering issues, no structural debt.
The CSS token system in layout.css is architecturally sound: raw palette constants are isolated in --palette-*, semantic tokens in --c-*, and Tailwind utilities generated from @theme inline. The bug is a misuse of the decorative --c-accent token for text — a usage violation, not a structural flaw.
What is noteworthy: the token file already documents the constraint correctly with an explicit warning comment. The violation happened at the point of use, not in the token definition. This is exactly the kind of gap that automated linting would close permanently.

Recommendations

Fix is trivial: text-accent → text-primary in PdfControls.svelte. Do not over-engineer.
After fixing, consider adding a Stylelint or custom ESLint rule to catch future text-accent usage in text-color context. A comment-only convention in a CSS file is a weak guardrail; a lint rule is a hard one. This is a one-time investment that prevents the same class of mistake from recurring on any future component.
The layout.css token comment is good documentation. No ADR needed for this level of change — the existing comment is sufficient.

Open Decisions

Whether to enforce the text-accent decorative-only constraint via linting is worth discussing: it converts a convention into a machine-checked rule, but requires maintaining the lint configuration. Low effort, high leverage.

## 🏗️ Markus Keller — Application Architect ### Observations - This is a purely presentational bug with no architectural surface — the fix is a single token swap in one component. No module boundary violations, no layering issues, no structural debt. - The CSS token system in `layout.css` is architecturally sound: raw palette constants are isolated in `--palette-*`, semantic tokens in `--c-*`, and Tailwind utilities generated from `@theme inline`. The bug is a misuse of the decorative `--c-accent` token for text — a usage violation, not a structural flaw. - What is noteworthy: the token file already documents the constraint correctly with an explicit warning comment. The violation happened at the point of use, not in the token definition. This is exactly the kind of gap that automated linting would close permanently. ### Recommendations - Fix is trivial: `text-accent` → `text-primary` in `PdfControls.svelte`. Do not over-engineer. - After fixing, consider adding a Stylelint or custom ESLint rule to catch future `text-accent` usage in text-color context. A comment-only convention in a CSS file is a weak guardrail; a lint rule is a hard one. This is a one-time investment that prevents the same class of mistake from recurring on any future component. - The `layout.css` token comment is good documentation. No ADR needed for this level of change — the existing comment is sufficient. ### Open Decisions - Whether to enforce the `text-accent` decorative-only constraint via linting is worth discussing: it converts a convention into a machine-checked rule, but requires maintaining the lint configuration. Low effort, high leverage.

marcel commented

2026-04-26 19:59:37 +02:00

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

This is an accessibility compliance bug, not a security vulnerability. No attack surface is affected.
From a security-adjacent perspective: text visibility failures on UI controls matter when the control in question changes what data is displayed to the user. The "Annotieren anzeigen" toggle controls annotation overlay rendering — a user who cannot read the label may not realize they are toggling visibility of annotation data. For a document archive with sensitive family records, invisible state labels on display controls are worth taking seriously.
No injection, authentication, or authorization surface is implicated. No OWASP Top 10 concern.

Recommendations

The fix (token swap) is safe and carries no security implications. No security review required for this change.
Confirm the annotation display is server-fetched data (not client-side secrets exposed in the DOM regardless of toggle state). A quick check of PdfViewer.svelte shows annotations are rendered only when showAnnotations is true — the toggle does control DOM presence, not just CSS visibility. This means the toggle is a functional control, not a cosmetic one, which reinforces why its label must be readable.
No regression tests are needed from a security standpoint beyond what Felix and Sara propose for the UI behavior.

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations - This is an accessibility compliance bug, not a security vulnerability. No attack surface is affected. - From a security-adjacent perspective: text visibility failures on UI controls matter when the control in question changes what data is displayed to the user. The "Annotieren anzeigen" toggle controls annotation overlay rendering — a user who cannot read the label may not realize they are toggling visibility of annotation data. For a document archive with sensitive family records, invisible state labels on display controls are worth taking seriously. - No injection, authentication, or authorization surface is implicated. No OWASP Top 10 concern. ### Recommendations - The fix (token swap) is safe and carries no security implications. No security review required for this change. - Confirm the annotation display is server-fetched data (not client-side secrets exposed in the DOM regardless of toggle state). A quick check of `PdfViewer.svelte` shows annotations are rendered only when `showAnnotations` is true — the toggle does control DOM presence, not just CSS visibility. This means the toggle is a functional control, not a cosmetic one, which reinforces why its label must be readable. - No regression tests are needed from a security standpoint beyond what Felix and Sara propose for the UI behavior.

marcel commented

2026-04-26 19:59:51 +02:00

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

No test covers the annotation toggle in any layer. The three existing tests in PdfViewer.svelte.spec.ts cover navigation and zoom; none pass annotationCount > 0 to trigger the conditional branch. This is a coverage gap that allowed the contrast regression to ship undetected.
The acceptance criteria in the issue require verification with browser DevTools. That is a manual process — it will not block a future regression. A wcag2aa axe-playwright assertion on the PDF viewer page will catch color contrast failures automatically.
The issue is P1-high in the "Demo Day" milestone. The fix should be verified before Demo Day — a gap in automated test coverage means another manual check cycle is needed after every subsequent change to the controls bar.

Recommendations

Add a PdfControls component test (separate spec file, not inside PdfViewer.svelte.spec.ts) with the following cases:
1. Annotation toggle renders when annotationCount > 0 — verifies conditional branch.
2. Annotation toggle is absent when annotationCount === 0 — verifies it stays hidden.
3. Toggle label changes between showAnnotations: true and false — verifies both i18n strings.
  This test runs in Vitest browser mode and takes under 1 second per case.

Add an axe-playwright assertion to the document detail E2E test (or a dedicated PDF viewer E2E test):

import AxeBuilder from '@axe-core/playwright';

test('pdf controls pass WCAG 2.1 AA contrast in light mode', async ({ page }) => {
    await page.goto('/documents/<id-with-annotations>');
    const results = await new AxeBuilder({ page })
        .withTags(['wcag2a', 'wcag2aa'])
        .analyze();
    expect(results.violations).toEqual([]);
});

Run the same check after toggling to dark mode (page.evaluate(() => document.documentElement.setAttribute('data-theme', 'dark'))).

Acceptance criteria for this issue should be tightened: add "No axe WCAG2AA violations on the PDF viewer page in light mode" as a verifiable, automatable criterion — not just DevTools manual verification.

Open Decisions

Should PdfControls.svelte.spec.ts be a standalone file, or should the annotation toggle tests be added to PdfViewer.svelte.spec.ts? Standalone is cleaner (tests the control in isolation without the heavy PDF mocking), but it adds a new file. The tests are independent either way.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations - **No test covers the annotation toggle** in any layer. The three existing tests in `PdfViewer.svelte.spec.ts` cover navigation and zoom; none pass `annotationCount > 0` to trigger the conditional branch. This is a coverage gap that allowed the contrast regression to ship undetected. - The acceptance criteria in the issue require verification with browser DevTools. That is a manual process — it will not block a future regression. A `wcag2aa` axe-playwright assertion on the PDF viewer page will catch color contrast failures automatically. - The issue is P1-high in the "Demo Day" milestone. The fix should be verified before Demo Day — a gap in automated test coverage means another manual check cycle is needed after every subsequent change to the controls bar. ### Recommendations - **Add a `PdfControls` component test** (separate spec file, not inside `PdfViewer.svelte.spec.ts`) with the following cases: 1. Annotation toggle renders when `annotationCount > 0` — verifies conditional branch. 2. Annotation toggle is absent when `annotationCount === 0` — verifies it stays hidden. 3. Toggle label changes between `showAnnotations: true` and `false` — verifies both i18n strings. This test runs in Vitest browser mode and takes under 1 second per case. - **Add an axe-playwright assertion** to the document detail E2E test (or a dedicated PDF viewer E2E test): ```typescript import AxeBuilder from '@axe-core/playwright'; test('pdf controls pass WCAG 2.1 AA contrast in light mode', async ({ page }) => { await page.goto('/documents/<id-with-annotations>'); const results = await new AxeBuilder({ page }) .withTags(['wcag2a', 'wcag2aa']) .analyze(); expect(results.violations).toEqual([]); }); ``` Run the same check after toggling to dark mode (`page.evaluate(() => document.documentElement.setAttribute('data-theme', 'dark'))`). - **Acceptance criteria for this issue should be tightened**: add "No axe WCAG2AA violations on the PDF viewer page in light mode" as a verifiable, automatable criterion — not just DevTools manual verification. ### Open Decisions - Should `PdfControls.svelte.spec.ts` be a standalone file, or should the annotation toggle tests be added to `PdfViewer.svelte.spec.ts`? Standalone is cleaner (tests the control in isolation without the heavy PDF mocking), but it adds a new file. The tests are independent either way.

marcel commented

2026-04-26 19:59:58 +02:00

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

This is a frontend styling bug with no infrastructure or CI implications. No Docker Compose, pipeline, or deployment configuration is affected.
The fix does not require a backend build, OpenAPI type regeneration, or any environment change.
The CI pipeline currently runs ESLint and svelte-check but has no automated accessibility gate (no axe-playwright job). This bug would have passed CI undetected.

Recommendations

The fix is a one-line change to a .svelte file — no infrastructure involvement needed. Fast to ship.
If Sara's recommendation to add an axe-playwright E2E test is accepted, confirm the CI pipeline has a Playwright-capable runner. The existing E2E job configuration should already handle this (Playwright browsers are installed), but verify the test can be added to the existing test:e2e npm script without requiring a separate runner or job.
No new infrastructure, services, or Docker images are required for this fix.

Open Decisions

None — purely a frontend concern.

## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations - This is a frontend styling bug with no infrastructure or CI implications. No Docker Compose, pipeline, or deployment configuration is affected. - The fix does not require a backend build, OpenAPI type regeneration, or any environment change. - The CI pipeline currently runs ESLint and `svelte-check` but has no automated accessibility gate (no axe-playwright job). This bug would have passed CI undetected. ### Recommendations - The fix is a one-line change to a `.svelte` file — no infrastructure involvement needed. Fast to ship. - If Sara's recommendation to add an axe-playwright E2E test is accepted, confirm the CI pipeline has a Playwright-capable runner. The existing E2E job configuration should already handle this (Playwright browsers are installed), but verify the test can be added to the existing `test:e2e` npm script without requiring a separate runner or job. - No new infrastructure, services, or Docker images are required for this fix. ### Open Decisions - None — purely a frontend concern.

marcel commented

2026-04-26 20:00:08 +02:00

📋 Elicit — Requirements Engineer

Observations

The issue body is clear and well-structured: steps to reproduce, expected vs. actual, and one WCAG-referenced acceptance criterion. This meets the Definition of Ready.
Gap in acceptance criteria: the criterion "Fix verified with browser DevTools accessibility contrast checker" is manual and non-repeatable. It will not block a future regression.
The title says "Annotieren anzeigen" but the actual i18n key renders as "Annotierungen anzeigen" — a minor discrepancy. Not a blocker, but the title could be updated for precision.
The issue correctly identifies the light mode failure but leaves the dark mode criterion as "remains legible" — which is untestable as written. Dark mode currently passes (axe would confirm it), but "legible" should be expressed as a measurable threshold to be consistent.

Recommendations

Strengthen the acceptance criteria to include an automatable criterion:

FR-A11Y-341-01 (Event): When a user views the PDF viewer in light mode with annotations present, the "Annotierungen anzeigen / verbergen" label shall have a contrast ratio ≥ 4.5:1 against its background in both toggle states.

FR-A11Y-341-02 (Event): When a user views the PDF viewer in dark mode with annotations present, the label shall have a contrast ratio ≥ 4.5:1 in both toggle states.

NFR-ACC-341-01: The PDF viewer page shall produce zero axe WCAG2AA violations (automated gate, not manual check).
Add "verified by automated axe-playwright assertion" to the Definition of Done for this issue so the fix cannot be closed on a manual check alone.
The milestone ("Demo Day — family get-together") is appropriate for a P1-high visible bug. No milestone change needed.

Open Decisions

None — requirements are clear once the automatable criterion is added.

## 📋 Elicit — Requirements Engineer ### Observations - The issue body is clear and well-structured: steps to reproduce, expected vs. actual, and one WCAG-referenced acceptance criterion. This meets the Definition of Ready. - **Gap in acceptance criteria**: the criterion "Fix verified with browser DevTools accessibility contrast checker" is manual and non-repeatable. It will not block a future regression. - The title says "Annotieren anzeigen" but the actual i18n key renders as "Annotierungen anzeigen" — a minor discrepancy. Not a blocker, but the title could be updated for precision. - The issue correctly identifies the light mode failure but leaves the dark mode criterion as "remains legible" — which is untestable as written. Dark mode currently passes (axe would confirm it), but "legible" should be expressed as a measurable threshold to be consistent. ### Recommendations - **Strengthen the acceptance criteria** to include an automatable criterion: **FR-A11Y-341-01** (Event): When a user views the PDF viewer in light mode with annotations present, the "Annotierungen anzeigen / verbergen" label shall have a contrast ratio ≥ 4.5:1 against its background in both toggle states. **FR-A11Y-341-02** (Event): When a user views the PDF viewer in dark mode with annotations present, the label shall have a contrast ratio ≥ 4.5:1 in both toggle states. **NFR-ACC-341-01**: The PDF viewer page shall produce zero axe WCAG2AA violations (automated gate, not manual check). - Add "verified by automated axe-playwright assertion" to the Definition of Done for this issue so the fix cannot be closed on a manual check alone. - The milestone ("Demo Day — family get-together") is appropriate for a P1-high visible bug. No milestone change needed. ### Open Decisions - None — requirements are clear once the automatable criterion is added.

marcel commented

2026-04-26 20:00:26 +02:00

🗳️ Decision Queue

Consolidated open decisions from the persona reviews. Each requires a human call — no right answer from the code alone.

Theme A: Inactive-state color choice

From Leonie:
Should the annotation toggle's inactive state use text-primary (navy in light, mint in dark) for stronger visual distinction from the active state (text-ink-2), or should it use text-ink-2 (same color as active) for visual consistency — relying on the background fill (bg-surface/10) alone to convey the on/off state?

text-primary = stronger state signal, but may look "too prominent" for an off state.
text-ink-2 = consistent with active, simpler, but color-blind users would rely solely on the background fill to detect state.

Both options fix the contrast failure. The choice is a design tradeoff, not a correctness issue.

Theme B: Test file structure for annotation toggle tests

From Sara:
Should the new annotation toggle tests live in a standalone PdfControls.svelte.spec.ts (clean isolation, no PDF mock needed), or be added to the existing PdfViewer.svelte.spec.ts (fewer files, but the component boundary is blurred)?

Standalone is slightly cleaner; the existing file is already named for the wrong component. Low stakes either way.

Theme C: Enforce `text-accent` decorative-only rule via linting

From Markus:
The layout.css has a comment warning against using --c-accent as a text color, but this bug proves that a comment is not a guardrail. Should a Stylelint or custom ESLint rule be added to block future text-accent / text-brand-mint usage in text-color positions?

Cost: one-time rule configuration, minor ongoing maintenance.
Benefit: the same class of bug cannot ship again without a CI failure.
Low effort, high leverage — but it is a scope addition beyond this single fix.

## 🗳️ Decision Queue Consolidated open decisions from the persona reviews. Each requires a human call — no right answer from the code alone. --- ### Theme A: Inactive-state color choice **From Leonie:** Should the annotation toggle's inactive state use `text-primary` (navy in light, mint in dark) for stronger visual distinction from the active state (`text-ink-2`), or should it use `text-ink-2` (same color as active) for visual consistency — relying on the background fill (`bg-surface/10`) alone to convey the on/off state? - `text-primary` = stronger state signal, but may look "too prominent" for an off state. - `text-ink-2` = consistent with active, simpler, but color-blind users would rely solely on the background fill to detect state. Both options fix the contrast failure. The choice is a design tradeoff, not a correctness issue. --- ### Theme B: Test file structure for annotation toggle tests **From Sara:** Should the new annotation toggle tests live in a standalone `PdfControls.svelte.spec.ts` (clean isolation, no PDF mock needed), or be added to the existing `PdfViewer.svelte.spec.ts` (fewer files, but the component boundary is blurred)? Standalone is slightly cleaner; the existing file is already named for the wrong component. Low stakes either way. --- ### Theme C: Enforce `text-accent` decorative-only rule via linting **From Markus:** The `layout.css` has a comment warning against using `--c-accent` as a text color, but this bug proves that a comment is not a guardrail. Should a Stylelint or custom ESLint rule be added to block future `text-accent` / `text-brand-mint` usage in text-color positions? - Cost: one-time rule configuration, minor ongoing maintenance. - Benefit: the same class of bug cannot ship again without a CI failure. - Low effort, high leverage — but it is a scope addition beyond this single fix.

marcel commented

2026-04-26 20:14:19 +02:00

A: text-primary
B: new PdfControls
C: yes, new lint ruel

A: text-primary B: new PdfControls C: yes, new lint ruel

marcel referenced this issue from a commit

2026-04-26 21:05:58 +02:00

fix(viewer): replace text-accent with text-primary on annotation toggle inactive state

marcel referenced this issue from a commit

2026-04-26 21:05:59 +02:00

fix(a11y): fix ProgressRing text label contrast and add no-restricted-syntax lint rule for text-accent

marcel commented

2026-04-26 21:06:15 +02:00

Implementation complete — branch `feat/issue-341-annotieren-contrast`

What was done

Commit 1: 110da9b8 — fix(viewer): replace text-accent with text-primary on annotation toggle inactive state

Fixed PdfControls.svelte line 94: replaced text-accent with text-primary in the inactive toggle state class expression.
- text-primary resolves to #012851 in light mode (14.5:1 contrast ratio on white — AA pass) and #a1dcd8 in dark mode (9:1 — AA pass).
Added PdfControls.svelte.spec.ts (new file) with 5 tests:
1. Annotation toggle renders when annotationCount > 0
2. Annotation toggle is absent when annotationCount === 0
3. "Annotierungen anzeigen" label shown when showAnnotations: false
4. "Annotierungen verbergen" label shown when showAnnotations: true
5. Button carries text-primary class (not text-accent) in inactive state

Commit 2: 379bc84e — fix(a11y): fix ProgressRing text label contrast and add no-restricted-syntax lint rule for text-accent

While adding the ESLint guardrail, the lint rule surfaced a second pre-existing violation: ProgressRing.svelte used text-accent on a percentage label in a ternary class expression. Fixed to text-primary.
Updated ProgressRing.svelte.spec.ts: test "renders a primary-colored label when percentage is > 0" now asserts text-primary.
Added no-restricted-syntax rule in eslint.config.js, scoped to *.svelte files only. It catches text-accent in JavaScript string literals (ternary/conditional class expressions) — the exact pattern that caused both violations. Asserting 'text-accent' in .spec.ts test files is unaffected.

Decision Queue outcomes

A (inactive-state color): text-primary — stronger visual distinction from active state ✅
B (test file structure): new standalone PdfControls.svelte.spec.ts ✅
C (lint rule): no-restricted-syntax rule in eslint.config.js scoped to .svelte files ✅

Test results

5/5 PdfControls tests pass
4/4 ProgressRing tests pass
Full suite: 632 passed, 1 pre-existing flaky test (entity-nav.svelte.spec.ts) unrelated to these changes
Lint: 0 errors with the new rule applied

Next step

Open PR: http://heim-nas:3005/marcel/familienarchiv/pulls/new/feat/issue-341-annotieren-contrast

## Implementation complete — branch `feat/issue-341-annotieren-contrast` ### What was done **Commit 1: `110da9b8`** — `fix(viewer): replace text-accent with text-primary on annotation toggle inactive state` - Fixed `PdfControls.svelte` line 94: replaced `text-accent` with `text-primary` in the inactive toggle state class expression. - `text-primary` resolves to `#012851` in light mode (14.5:1 contrast ratio on white — AA pass) and `#a1dcd8` in dark mode (9:1 — AA pass). - Added `PdfControls.svelte.spec.ts` (new file) with 5 tests: 1. Annotation toggle renders when `annotationCount > 0` 2. Annotation toggle is absent when `annotationCount === 0` 3. "Annotierungen anzeigen" label shown when `showAnnotations: false` 4. "Annotierungen verbergen" label shown when `showAnnotations: true` 5. Button carries `text-primary` class (not `text-accent`) in inactive state **Commit 2: `379bc84e`** — `fix(a11y): fix ProgressRing text label contrast and add no-restricted-syntax lint rule for text-accent` - While adding the ESLint guardrail, the lint rule surfaced a second pre-existing violation: `ProgressRing.svelte` used `text-accent` on a percentage label in a ternary class expression. Fixed to `text-primary`. - Updated `ProgressRing.svelte.spec.ts`: test "renders a primary-colored label when percentage is > 0" now asserts `text-primary`. - Added `no-restricted-syntax` rule in `eslint.config.js`, scoped to `*.svelte` files only. It catches `text-accent` in JavaScript string literals (ternary/conditional class expressions) — the exact pattern that caused both violations. Asserting `'text-accent'` in `.spec.ts` test files is unaffected. ### Decision Queue outcomes - **A (inactive-state color)**: `text-primary` — stronger visual distinction from active state ✅ - **B (test file structure)**: new standalone `PdfControls.svelte.spec.ts` ✅ - **C (lint rule)**: `no-restricted-syntax` rule in `eslint.config.js` scoped to `.svelte` files ✅ ### Test results - 5/5 PdfControls tests pass - 4/4 ProgressRing tests pass - Full suite: 632 passed, 1 pre-existing flaky test (`entity-nav.svelte.spec.ts`) unrelated to these changes - Lint: 0 errors with the new rule applied ### Next step Open PR: http://heim-nas:3005/marcel/familienarchiv/pulls/new/feat/issue-341-annotieren-contrast

marcel referenced a pull request that will close this issue

2026-04-26 21:07:31 +02:00

fix(viewer): fix "Annotieren anzeigen" contrast and add lint rule (#341) #349

marcel referenced this issue

2026-04-26 21:09:28 +02:00

fix(viewer): fix "Annotieren anzeigen" contrast and add lint rule (#341) #349

marcel referenced this issue

2026-04-26 21:09:48 +02:00

fix(viewer): fix "Annotieren anzeigen" contrast and add lint rule (#341) #349

marcel referenced this issue

2026-04-26 21:26:43 +02:00

test(a11y): add axe-playwright E2E gate for PDF viewer WCAG 2.1 AA compliance #353