fix(test): NotificationDropdown "view-all link" test causes iframe navigation crash in CI #545

New Issue

marcel · 2026-05-12T10:20:20+02:00

marcel commented

2026-05-12 10:20:20 +02:00

Problem

CI frontend tests fail with an unhandled error in src/lib/notification/NotificationDropdown.svelte.test.ts:

Error: Failed to run the test …/NotificationDropdown.svelte.test.ts.
Caused by: Error: Cannot connect to the iframe. Did you change the location or submitted a form?
Received URL: http://localhost:63315/aktivitaeten
Expected: http://localhost:63315/?sessionId=…

The test 'calls onClose when the view-all link is clicked' does:

await page.getByRole('link').click();

The component renders:

<a href="/aktivitaeten" onclick={onClose} …>

In the vitest-browser Playwright iframe there is no SvelteKit router, so clicking the anchor triggers a real full-page navigation to /aktivitaeten. This disconnects the orchestrator iframe and crashes the entire test file (not just the one test case).

Secondary noise (no test failures, but logged)

@tailwindcss/vite:generate:serve logs pre-transform errors when it processes Svelte virtual CSS modules for ChronikFuerDichBox.svelte and hilfe/transkription/+page.svelte — it sees script/template content instead of CSS. These do not cause test failures but are noisy in CI output.

Fix

Two-part change following the existing pattern used in src/routes/admin/page.svelte.test.ts:

1. NotificationDropdown.svelte — prevent default href navigation and delegate to SvelteKit's goto():

import { goto } from '$app/navigation';
…
<a
  href="/aktivitaeten"
  onclick={(e) => { e.preventDefault(); goto('/aktivitaeten'); onClose(); }}
  …
>

The href stays for accessibility (right-click / open-in-new-tab); goto() handles client-side navigation in the real app.

2. NotificationDropdown.svelte.test.ts — mock $app/navigation so goto is a no-op spy:

vi.mock('$app/navigation', () => ({
  goto: vi.fn()
}));

Acceptance criteria

npm run test passes locally with no unhandled errors in NotificationDropdown.svelte.test.ts
All 11 existing tests in the file continue to pass
CI frontend test job is green

## Problem CI frontend tests fail with an unhandled error in `src/lib/notification/NotificationDropdown.svelte.test.ts`: ``` Error: Failed to run the test …/NotificationDropdown.svelte.test.ts. Caused by: Error: Cannot connect to the iframe. Did you change the location or submitted a form? Received URL: http://localhost:63315/aktivitaeten Expected: http://localhost:63315/?sessionId=… ``` The test `'calls onClose when the view-all link is clicked'` does: ```typescript await page.getByRole('link').click(); ``` The component renders: ```svelte <a href="/aktivitaeten" onclick={onClose} …> ``` In the vitest-browser Playwright iframe there is no SvelteKit router, so clicking the anchor triggers a **real full-page navigation** to `/aktivitaeten`. This disconnects the orchestrator iframe and crashes the entire test file (not just the one test case). ## Secondary noise (no test failures, but logged) `@tailwindcss/vite:generate:serve` logs pre-transform errors when it processes Svelte virtual CSS modules for `ChronikFuerDichBox.svelte` and `hilfe/transkription/+page.svelte` — it sees script/template content instead of CSS. These do not cause test failures but are noisy in CI output. ## Fix Two-part change following the existing pattern used in `src/routes/admin/page.svelte.test.ts`: **1. `NotificationDropdown.svelte`** — prevent default href navigation and delegate to SvelteKit's `goto()`: ```svelte import { goto } from '$app/navigation'; … <a href="/aktivitaeten" onclick={(e) => { e.preventDefault(); goto('/aktivitaeten'); onClose(); }} … > ``` The `href` stays for accessibility (right-click / open-in-new-tab); `goto()` handles client-side navigation in the real app. **2. `NotificationDropdown.svelte.test.ts`** — mock `$app/navigation` so `goto` is a no-op spy: ```typescript vi.mock('$app/navigation', () => ({ goto: vi.fn() })); ``` ## Acceptance criteria - [ ] `npm run test` passes locally with no unhandled errors in `NotificationDropdown.svelte.test.ts` - [ ] All 11 existing tests in the file continue to pass - [ ] CI frontend test job is green

marcel added the P1-high bug test labels 2026-05-12 10:20:23 +02:00

marcel commented

2026-05-12 10:23:48 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

Root cause confirmed. The onclick prop currently bound to onClose alone has no e.preventDefault(), so clicking the <a> follows the href for real. In the vitest-browser Playwright iframe there is no router — the real navigation disconnects the orchestrator iframe and crashes the entire test file, not just the one test case. That's why CI shows all 11 tests failed.
Pattern is already established. src/routes/admin/page.svelte.test.ts:5-17 is the exact reference: vi.mock('$app/navigation', ...) at the top, then import the component. The issue correctly identifies this as the template.
Minimal mock is correct here. The admin page mocks 12 navigation functions because its component uses more of them. NotificationDropdown.svelte will only import goto, so { goto: vi.fn() } is the right-sized mock — don't wholesale copy the admin page's full mock object.
Static import is fine. Vitest hoists vi.mock() before any imports run. The existing static import of NotificationDropdown at the top of the test file does not need to be converted to await import().
Fragile link selector. Line 167 of the test uses page.getByRole('link') with no name filter. This works while there is exactly one <a> in the dropdown, but is implicitly coupled to that assumption. Adding { name: /alle aktivitäten|view all/i } (matching m.chronik_view_all()) makes the intent explicit.
Missing navigation assertion. The test asserts expect(onClose).toHaveBeenCalledOnce() but never checks that goto('/aktivitaeten') was actually called. Since goto becomes a vi.fn() spy, the test should also assert the navigation call — otherwise it verifies the callback fires but not that the user ends up anywhere.

Recommendations

Reorder the handler: e.preventDefault(); onClose(); goto('/aktivitaeten'). Close the dropdown before navigation begins. goto() returns a Promise that isn't awaited in the handler, so order determines which side-effect the user perceives first.

Assert on the spy in the updated test:

import { goto } from '$app/navigation';
// ...
await page.getByRole('link', { name: /alle aktivitäten|view all/i }).click();
expect(onClose).toHaveBeenCalledOnce();
expect(goto).toHaveBeenCalledWith('/aktivitaeten');

Tighten the getByRole selector with a name filter as shown above.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - **Root cause confirmed.** The `onclick` prop currently bound to `onClose` alone has no `e.preventDefault()`, so clicking the `<a>` follows the `href` for real. In the vitest-browser Playwright iframe there is no router — the real navigation disconnects the orchestrator iframe and crashes the *entire test file*, not just the one test case. That's why CI shows all 11 tests failed. - **Pattern is already established.** `src/routes/admin/page.svelte.test.ts:5-17` is the exact reference: `vi.mock('$app/navigation', ...)` at the top, then import the component. The issue correctly identifies this as the template. - **Minimal mock is correct here.** The admin page mocks 12 navigation functions because its component uses more of them. `NotificationDropdown.svelte` will only import `goto`, so `{ goto: vi.fn() }` is the right-sized mock — don't wholesale copy the admin page's full mock object. - **Static import is fine.** Vitest hoists `vi.mock()` before any imports run. The existing static import of `NotificationDropdown` at the top of the test file does not need to be converted to `await import()`. - **Fragile link selector.** Line 167 of the test uses `page.getByRole('link')` with no name filter. This works while there is exactly one `<a>` in the dropdown, but is implicitly coupled to that assumption. Adding `{ name: /alle aktivitäten|view all/i }` (matching `m.chronik_view_all()`) makes the intent explicit. - **Missing navigation assertion.** The test asserts `expect(onClose).toHaveBeenCalledOnce()` but never checks that `goto('/aktivitaeten')` was actually called. Since `goto` becomes a `vi.fn()` spy, the test should also assert the navigation call — otherwise it verifies the callback fires but not that the user ends up anywhere. ### Recommendations - **Reorder the handler**: `e.preventDefault(); onClose(); goto('/aktivitaeten')`. Close the dropdown *before* navigation begins. `goto()` returns a Promise that isn't awaited in the handler, so order determines which side-effect the user perceives first. - **Assert on the spy** in the updated test: ```typescript import { goto } from '$app/navigation'; // ... await page.getByRole('link', { name: /alle aktivitäten|view all/i }).click(); expect(onClose).toHaveBeenCalledOnce(); expect(goto).toHaveBeenCalledWith('/aktivitaeten'); ``` - Tighten the `getByRole` selector with a `name` filter as shown above.

marcel commented

2026-05-12 10:23:53 +02:00

🏛️ Markus Keller — Application Architect

Observations

No structural concerns. The fix is correctly scoped to the notification module. No new cross-domain dependencies, no service boundary crossings, no infrastructure changes.
goto() is the idiomatic SvelteKit navigation primitive for this case: the component needs both a real href (for right-click / middle-click / open-in-new-tab browser behavior) and client-side routing (no full-page reload). e.preventDefault() + goto() is the canonical solution across the SvelteKit ecosystem.
Framework import, not a domain service. goto from $app/navigation is a SvelteKit framework dependency, not a cross-domain service call. No module boundary is crossed.
The pattern already exists in the codebase — the admin page component set the precedent. This fix makes the notification module consistent with it.

Recommendations

No architectural changes needed. Ship the fix as described in the issue.

## 🏛️ Markus Keller — Application Architect ### Observations - **No structural concerns.** The fix is correctly scoped to the `notification` module. No new cross-domain dependencies, no service boundary crossings, no infrastructure changes. - **`goto()` is the idiomatic SvelteKit navigation primitive** for this case: the component needs both a real `href` (for right-click / middle-click / open-in-new-tab browser behavior) and client-side routing (no full-page reload). `e.preventDefault()` + `goto()` is the canonical solution across the SvelteKit ecosystem. - **Framework import, not a domain service.** `goto` from `$app/navigation` is a SvelteKit framework dependency, not a cross-domain service call. No module boundary is crossed. - **The pattern already exists in the codebase** — the admin page component set the precedent. This fix makes the notification module consistent with it. ### Recommendations - No architectural changes needed. Ship the fix as described in the issue.

marcel commented

2026-05-12 10:24:05 +02:00

🧪 Sara Holt — QA Engineer

Observations

Crash mechanism confirmed. Iframe navigation in vitest-browser is unrecoverable — once the orchestrator iframe disconnects, all remaining tests in the file are reported as failed. The fix correctly prevents the navigation event from propagating, which eliminates the crash entirely.
Missing behavioral assertion. The test asserts expect(onClose).toHaveBeenCalledOnce() but does not verify that goto('/aktivitaeten') was called. Since goto is a vi.fn() spy after the mock, the test can and should assert both sides: the callback and the navigation. Without the navigation assertion, the test would pass even if goto were never invoked — which is the core behavioral guarantee of this fix.
Fragile selector. page.getByRole('link') at line 167 has no name filter. It passes today because the dropdown renders exactly one <a>. Add { name: /alle aktivitäten|view all/i } (the rendered text of m.chronik_view_all()) so the test fails meaningfully if the link is renamed or removed, rather than selecting an unintended element.
Tailwind noise is out of AC scope. The secondary issue (pre-transform errors for ChronikFuerDichBox.svelte and hilfe/transkription/+page.svelte) is described in the problem statement but has no acceptance criterion and no proposed fix. This creates review ambiguity — a reviewer cannot tell whether a PR that silences the warnings is "more done" or out of scope. Resolve this explicitly (see Open Decisions).
Coverage impact. The new goto() call in the component is a new branch. Asserting it in the test keeps branch coverage meaningful rather than inflating it with untested paths.

Recommendations

Import goto from the mocked module in the test and assert expect(goto).toHaveBeenCalledWith('/aktivitaeten') alongside the onClose assertion.
Add a name filter to the getByRole('link') selector.
Clarify the Tailwind noise scope before the PR lands.

Open Decisions

Tailwind CSS noise scope: the secondary issue is mentioned in the issue body but not in the acceptance criteria. Options: (a) add an explicit AC and fix it in this PR, (b) remove it from this issue and open a dedicated follow-up, or (c) leave it with a comment "tracked separately as #XYZ." One of these must be chosen — the current ambiguous state leaves reviewers guessing.

## 🧪 Sara Holt — QA Engineer ### Observations - **Crash mechanism confirmed.** Iframe navigation in vitest-browser is unrecoverable — once the orchestrator iframe disconnects, all remaining tests in the file are reported as failed. The fix correctly prevents the navigation event from propagating, which eliminates the crash entirely. - **Missing behavioral assertion.** The test asserts `expect(onClose).toHaveBeenCalledOnce()` but does not verify that `goto('/aktivitaeten')` was called. Since `goto` is a `vi.fn()` spy after the mock, the test can and should assert both sides: the callback *and* the navigation. Without the navigation assertion, the test would pass even if `goto` were never invoked — which is the core behavioral guarantee of this fix. - **Fragile selector.** `page.getByRole('link')` at line 167 has no name filter. It passes today because the dropdown renders exactly one `<a>`. Add `{ name: /alle aktivitäten|view all/i }` (the rendered text of `m.chronik_view_all()`) so the test fails meaningfully if the link is renamed or removed, rather than selecting an unintended element. - **Tailwind noise is out of AC scope.** The secondary issue (pre-transform errors for `ChronikFuerDichBox.svelte` and `hilfe/transkription/+page.svelte`) is described in the problem statement but has no acceptance criterion and no proposed fix. This creates review ambiguity — a reviewer cannot tell whether a PR that silences the warnings is "more done" or out of scope. Resolve this explicitly (see Open Decisions). - **Coverage impact.** The new `goto()` call in the component is a new branch. Asserting it in the test keeps branch coverage meaningful rather than inflating it with untested paths. ### Recommendations - Import `goto` from the mocked module in the test and assert `expect(goto).toHaveBeenCalledWith('/aktivitaeten')` alongside the `onClose` assertion. - Add a `name` filter to the `getByRole('link')` selector. - Clarify the Tailwind noise scope before the PR lands. ### Open Decisions - **Tailwind CSS noise scope**: the secondary issue is mentioned in the issue body but not in the acceptance criteria. Options: (a) add an explicit AC and fix it in this PR, (b) remove it from this issue and open a dedicated follow-up, or (c) leave it with a comment "tracked separately as #XYZ." One of these must be chosen — the current ambiguous state leaves reviewers guessing.

marcel commented

2026-05-12 10:24:10 +02:00

🔐 Nora "NullX" Steiner — Security Engineer

Observations

No security concerns with this fix.
href preservation is correct. Keeping href="/aktivitaeten" on the anchor element is the right call — without it the element loses its link semantics and browsers cannot offer right-click / open-in-new-tab behavior. The path is a static internal route with no user-controlled input, so there is no open-redirect risk.
goto() routes through SvelteKit's internal router. Any guards defined in +layout.server.ts or hooks.server.ts still execute on navigation. No auth check is bypassed.
e.preventDefault() is narrow in scope. It suppresses the browser's default href-follow action only — it does not suppress any security-relevant event behavior such as CSRF tokens or session handling.
Test mock is test-infrastructure only. { goto: vi.fn() } replaces the navigation function in the test environment. No production code is weakened.

Recommendations

No security changes required. Fix is clean.

## 🔐 Nora "NullX" Steiner — Security Engineer ### Observations - **No security concerns with this fix.** - **`href` preservation is correct.** Keeping `href="/aktivitaeten"` on the anchor element is the right call — without it the element loses its link semantics and browsers cannot offer right-click / open-in-new-tab behavior. The path is a static internal route with no user-controlled input, so there is no open-redirect risk. - **`goto()` routes through SvelteKit's internal router.** Any guards defined in `+layout.server.ts` or `hooks.server.ts` still execute on navigation. No auth check is bypassed. - **`e.preventDefault()` is narrow in scope.** It suppresses the browser's default href-follow action only — it does not suppress any security-relevant event behavior such as CSRF tokens or session handling. - **Test mock is test-infrastructure only.** `{ goto: vi.fn() }` replaces the navigation function in the test environment. No production code is weakened. ### Recommendations - No security changes required. Fix is clean.

marcel commented

2026-05-12 10:24:17 +02:00

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

Keeping href="/aktivitaeten" is the correct UX decision. Without a real href, users who right-click or middle-click the "view all" link lose native browser link behavior (open in new tab, copy link address). The proposed fix correctly preserves href for these cases while overriding left-click with client-side navigation. This is good for all users, especially those accustomed to link conventions.
Dropdown close order matters for perceived responsiveness. The issue proposes e.preventDefault(); goto('/aktivitaeten'); onClose(). From a UX standpoint, onClose() should come before goto(). The dropdown should close immediately on click — if navigation is slow or delayed, leaving the dropdown open while the page transitions creates a disorienting visual state. Recommended order: e.preventDefault(); onClose(); goto('/aktivitaeten').
Accessible name is sufficient. The link's visible text is the output of m.chronik_view_all() — a real text label, not an icon alone. No aria-label addition is needed. Screen readers will announce the link text correctly.
No visual changes. All existing styling on the <a> element is preserved. No brand or layout concerns.

Recommendations

Reorder the handler to e.preventDefault(); onClose(); goto('/aktivitaeten') for better perceived responsiveness.
No other UX changes needed.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations - **Keeping `href="/aktivitaeten"` is the correct UX decision.** Without a real `href`, users who right-click or middle-click the "view all" link lose native browser link behavior (open in new tab, copy link address). The proposed fix correctly preserves `href` for these cases while overriding left-click with client-side navigation. This is good for all users, especially those accustomed to link conventions. - **Dropdown close order matters for perceived responsiveness.** The issue proposes `e.preventDefault(); goto('/aktivitaeten'); onClose()`. From a UX standpoint, `onClose()` should come *before* `goto()`. The dropdown should close immediately on click — if navigation is slow or delayed, leaving the dropdown open while the page transitions creates a disorienting visual state. Recommended order: `e.preventDefault(); onClose(); goto('/aktivitaeten')`. - **Accessible name is sufficient.** The link's visible text is the output of `m.chronik_view_all()` — a real text label, not an icon alone. No `aria-label` addition is needed. Screen readers will announce the link text correctly. - **No visual changes.** All existing styling on the `<a>` element is preserved. No brand or layout concerns. ### Recommendations - Reorder the handler to `e.preventDefault(); onClose(); goto('/aktivitaeten')` for better perceived responsiveness. - No other UX changes needed.

marcel commented

2026-05-12 10:24:25 +02:00

⚙️ Tobias Wendt — DevOps & Platform Engineer

Observations

CI impact is confirmed and correctly prioritized P1-high. The iframe navigation crash takes down the entire NotificationDropdown.svelte.test.ts file — all 11 tests fail in CI, not just the offending one. The frontend test job fails and blocks PR merges until this is fixed.
No CI pipeline changes needed. The fix touches two source files (NotificationDropdown.svelte and NotificationDropdown.svelte.test.ts). The existing ci.yml frontend job runs npm run test — no changes to the workflow definition are required.
Fix eliminates non-determinism. The current failure is deterministic (clicking the link always crashes the iframe), but it presents as a flaky-looking failure in CI because the test file summary shows 11 failures for what appears to be one root cause. After the fix, the test file will be stable.
Tailwind CSS log noise. The @tailwindcss/vite:generate:serve pre-transform errors for ChronikFuerDichBox.svelte and hilfe/transkription/+page.svelte don't cause CI failures today, but log noise can obscure real errors — a future real failure in these components could be buried under the existing noise. Worth a dedicated follow-up issue to suppress or fix, but not a blocker for this PR.

Recommendations

Ship the fix immediately — it's a contained two-file change with no pipeline impact.
Open a follow-up issue specifically for the Tailwind CSS pre-transform warnings once this is resolved, so the CI log is clean.

## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Observations - **CI impact is confirmed and correctly prioritized P1-high.** The iframe navigation crash takes down the *entire* `NotificationDropdown.svelte.test.ts` file — all 11 tests fail in CI, not just the offending one. The frontend test job fails and blocks PR merges until this is fixed. - **No CI pipeline changes needed.** The fix touches two source files (`NotificationDropdown.svelte` and `NotificationDropdown.svelte.test.ts`). The existing `ci.yml` frontend job runs `npm run test` — no changes to the workflow definition are required. - **Fix eliminates non-determinism.** The current failure is deterministic (clicking the link always crashes the iframe), but it presents as a flaky-looking failure in CI because the test file summary shows 11 failures for what appears to be one root cause. After the fix, the test file will be stable. - **Tailwind CSS log noise.** The `@tailwindcss/vite:generate:serve` pre-transform errors for `ChronikFuerDichBox.svelte` and `hilfe/transkription/+page.svelte` don't cause CI failures today, but log noise can obscure real errors — a future real failure in these components could be buried under the existing noise. Worth a dedicated follow-up issue to suppress or fix, but not a blocker for this PR. ### Recommendations - Ship the fix immediately — it's a contained two-file change with no pipeline impact. - Open a follow-up issue specifically for the Tailwind CSS pre-transform warnings once this is resolved, so the CI log is clean.

marcel commented

2026-05-12 10:24:34 +02:00

📋 Elicit — Requirements Engineer

Observations

Issue is well-formed overall. Root cause, affected behavior, proposed fix, reference pattern, and acceptance criteria are all present. The issue is small and focused — good.
Acceptance criteria gap. The three ACs verify: (1) tests pass locally, (2) 11 existing tests still pass, (3) CI is green. None of them verify the new behavioral guarantee being introduced: that clicking the link triggers goto('/aktivitaeten'). A PR that calls onClose() but replaces the link with a <button> would satisfy all three ACs while breaking the navigation entirely. Add a fourth AC: "Clicking the view-all link calls goto('/aktivitaeten'), verified by a vi.fn() spy assertion in the updated test."
Secondary noise scope is undefined. The "Secondary noise" section describes a Tailwind CSS pre-transform error affecting two components. It has no AC, no proposed fix, and no "won't fix in this issue" declaration. As written, it is in scope of the problem description but out of scope of the acceptance criteria. Reviewers cannot tell whether a PR that silences these warnings is "more complete" or "out of scope." This ambiguity should be resolved before implementation.
The issue is otherwise implementation-ready for its primary goal once the two gaps above are addressed.

Recommendations

Add AC: "Clicking the view-all link calls goto('/aktivitaeten') — verified by spy assertion in the updated test."
Resolve the Tailwind noise scope explicitly: either add an AC for it, file it as a separate issue and reference the number, or mark it explicitly as "out of scope for this fix."

Open Decisions

Tailwind CSS pre-transform noise scope: currently in the issue body but absent from acceptance criteria. Should it be fixed in this PR (add AC + fix), tracked as a separate issue (reference #XYZ here), or explicitly declared out of scope? The current ambiguous state leaves implementers and reviewers without a clear boundary.

## 📋 Elicit — Requirements Engineer ### Observations - **Issue is well-formed overall.** Root cause, affected behavior, proposed fix, reference pattern, and acceptance criteria are all present. The issue is small and focused — good. - **Acceptance criteria gap.** The three ACs verify: (1) tests pass locally, (2) 11 existing tests still pass, (3) CI is green. None of them verify the *new behavioral guarantee* being introduced: that clicking the link triggers `goto('/aktivitaeten')`. A PR that calls `onClose()` but replaces the link with a `<button>` would satisfy all three ACs while breaking the navigation entirely. Add a fourth AC: "Clicking the view-all link calls `goto('/aktivitaeten')`, verified by a `vi.fn()` spy assertion in the updated test." - **Secondary noise scope is undefined.** The "Secondary noise" section describes a Tailwind CSS pre-transform error affecting two components. It has no AC, no proposed fix, and no "won't fix in this issue" declaration. As written, it is in scope of the problem description but out of scope of the acceptance criteria. Reviewers cannot tell whether a PR that silences these warnings is "more complete" or "out of scope." This ambiguity should be resolved before implementation. - **The issue is otherwise implementation-ready** for its primary goal once the two gaps above are addressed. ### Recommendations - Add AC: *"Clicking the view-all link calls `goto('/aktivitaeten')` — verified by spy assertion in the updated test."* - Resolve the Tailwind noise scope explicitly: either add an AC for it, file it as a separate issue and reference the number, or mark it explicitly as "out of scope for this fix." ### Open Decisions - **Tailwind CSS pre-transform noise scope**: currently in the issue body but absent from acceptance criteria. Should it be fixed in this PR (add AC + fix), tracked as a separate issue (reference #XYZ here), or explicitly declared out of scope? The current ambiguous state leaves implementers and reviewers without a clear boundary.

marcel commented

2026-05-12 10:24:45 +02:00

🗳️ Decision Queue — Action Required

1 decision needs your input before implementation starts.

Scope

Tailwind CSS pre-transform log noise — The issue body describes @tailwindcss/vite:generate:serve errors for ChronikFuerDichBox.svelte and hilfe/transkription/+page.svelte as a "secondary noise" problem, but the acceptance criteria are silent on it. Three options:
- (a) Fix in this PR — add an AC and include the fix in the same branch.
- (b) Track separately — open a new issue, reference it here, and explicitly mark the noise section as "out of scope for this fix."
- (c) Declare out of scope — add a sentence to the issue body saying "the Tailwind noise is known and will not be addressed here."
Option (b) is recommended by both Sara and Tobias — it keeps this fix small and CI-unblocking, while ensuring the noise doesn't get forgotten. (Raised by: Sara, Elicit, Tobias)

## 🗳️ Decision Queue — Action Required _1 decision needs your input before implementation starts._ ### Scope - **Tailwind CSS pre-transform log noise** — The issue body describes `@tailwindcss/vite:generate:serve` errors for `ChronikFuerDichBox.svelte` and `hilfe/transkription/+page.svelte` as a "secondary noise" problem, but the acceptance criteria are silent on it. Three options: - **(a) Fix in this PR** — add an AC and include the fix in the same branch. - **(b) Track separately** — open a new issue, reference it here, and explicitly mark the noise section as "out of scope for this fix." - **(c) Declare out of scope** — add a sentence to the issue body saying "the Tailwind noise is known and will not be addressed here." Option (b) is recommended by both Sara and Tobias — it keeps this fix small and CI-unblocking, while ensuring the noise doesn't get forgotten. _(Raised by: Sara, Elicit, Tobias)_

marcel commented

2026-05-12 10:26:08 +02:00

A) Fix in this PR

marcel referenced a pull request that will close this issue

2026-05-12 10:52:52 +02:00

fix(test): NotificationDropdown iframe navigation crash + Tailwind CI noise #548

marcel commented

2026-05-12 10:53:04 +02:00

Implementation complete — PR #548

All acceptance criteria met. Four atomic commits on feat/issue-545-notification-dropdown-iframe-fix:

Primary fix

NotificationDropdown.svelte — added import { goto } from '$app/navigation' and replaced onclick={onClose} on the footer link with:

onclick={(e) => { e.preventDefault(); onClose(); goto('/aktivitaeten'); }}

href="/aktivitaeten" is preserved for right-click / open-in-new-tab. onClose() runs before goto() so the dropdown disappears immediately on click.

NotificationDropdown.svelte.test.ts — three test changes:

vi.mock('$app/navigation', () => ({ goto: vi.fn() })) — minimal mock (only goto) to prevent real navigation in the Playwright iframe
getByRole('link', { name: /alle aktivitäten|view all/i }) — explicit selector, no longer implicitly coupled to "exactly one link"
expect(goto).toHaveBeenCalledWith('/aktivitaeten') — asserts the behavioural guarantee introduced by this fix

Secondary fix (Tailwind CI noise — option A)

ChronikFuerDichBox.svelte and hilfe/transkription/+page.svelte were the only two Svelte components with non-empty <style> blocks, causing the Svelte vite plugin to create virtual CSS modules that @tailwindcss/vite:generate:serve tried (and failed) to process as CSS. Both <style> blocks have been extracted into layout.css. The animation class was renamed fade-in → chronik-fade-in for safe global scope.

Commits

f6a0d7aa test(notification): add goto mock and tighten selector in NotificationDropdown spec
0387d51e fix(notification): prevent iframe navigation — use goto instead of href follow
97bb1ee6 fix(style): move ChronikFuerDichBox animation to global CSS to suppress Tailwind noise
0a6a3fd0 fix(style): move transkription print styles to global CSS to suppress Tailwind noise

All 13 NotificationDropdown tests pass. 274 test files, 2588 tests total — zero failures.

## Implementation complete — PR #548 All acceptance criteria met. Four atomic commits on `feat/issue-545-notification-dropdown-iframe-fix`: ### Primary fix **`NotificationDropdown.svelte`** — added `import { goto } from '$app/navigation'` and replaced `onclick={onClose}` on the footer link with: ```svelte onclick={(e) => { e.preventDefault(); onClose(); goto('/aktivitaeten'); }} ``` `href="/aktivitaeten"` is preserved for right-click / open-in-new-tab. `onClose()` runs before `goto()` so the dropdown disappears immediately on click. **`NotificationDropdown.svelte.test.ts`** — three test changes: - `vi.mock('$app/navigation', () => ({ goto: vi.fn() }))` — minimal mock (only `goto`) to prevent real navigation in the Playwright iframe - `getByRole('link', { name: /alle aktivitäten|view all/i })` — explicit selector, no longer implicitly coupled to "exactly one link" - `expect(goto).toHaveBeenCalledWith('/aktivitaeten')` — asserts the behavioural guarantee introduced by this fix ### Secondary fix (Tailwind CI noise — option A) `ChronikFuerDichBox.svelte` and `hilfe/transkription/+page.svelte` were the only two Svelte components with non-empty `<style>` blocks, causing the Svelte vite plugin to create virtual CSS modules that `@tailwindcss/vite:generate:serve` tried (and failed) to process as CSS. Both `<style>` blocks have been extracted into `layout.css`. The animation class was renamed `fade-in` → `chronik-fade-in` for safe global scope. ### Commits - `f6a0d7aa` test(notification): add goto mock and tighten selector in NotificationDropdown spec - `0387d51e` fix(notification): prevent iframe navigation — use goto instead of href follow - `97bb1ee6` fix(style): move ChronikFuerDichBox animation to global CSS to suppress Tailwind noise - `0a6a3fd0` fix(style): move transkription print styles to global CSS to suppress Tailwind noise All 13 NotificationDropdown tests pass. 274 test files, 2588 tests total — zero failures.

marcel referenced this issue

2026-05-12 11:13:26 +02:00

fix(test): NotificationDropdown iframe navigation crash + Tailwind CI noise #548

marcel referenced this issue

2026-05-12 11:32:00 +02:00

fix(test): NotificationDropdown iframe navigation crash + Tailwind CI noise #548

marcel closed this issue

2026-05-12 11:35:41 +02:00

marcel referenced this issue

2026-05-12 16:40:27 +02:00

fix(notification): replace view-all anchor with button to prevent iframe navigation #552

marcel referenced this issue