feat: Add onboarding modal with explainer video (#145)#146
Merged
rdmueller merged 11 commits intoMar 8, 2026
Conversation
- Show onboarding modal automatically on first visit - Logo at top, slogan highlighted in primary color - Desktop: YouTube Shorts embed left, text right - Mobile: text + YouTube link (no embed) - Language-aware: DE/EN video and text based on site language - Closes via X button, CTA button, ESC key, or click outside - Saves dismissal in localStorage (not shown again) - Info button (i) in header to reopen at any time - 13 new unit tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughDiese PR führt ein Onboarding-Modal für Erstbesucher ein, das automatisch angezeigt wird und über einen Info-Button in der Kopfzeile erneut geöffnet werden kann. Das Modal enthält ein erklärendes Video, ein Logo und mehrsprachige Texte. Zusätzlich werden Anker-Dokumente synced, Übersetzungen aktualisiert und Layout-Komponenten für Responsive Design angepasst. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Header
participant OnboardingModal
participant LocalStorage
participant DOM
User->>Header: Seite laden oder Info-Button klicken
Header->>OnboardingModal: createOnboardingModal()
OnboardingModal->>DOM: Modal-Element mit ARIA-Attributen erstellen
OnboardingModal->>LocalStorage: shouldShowOnboarding() überprüfen
alt Erstbesucher
OnboardingModal->>OnboardingModal: buildModalContent() mit Sprache & Video
OnboardingModal->>DOM: Modal-Inhalt rendern (Logo, Slogan, Text, Video/Link)
OnboardingModal->>DOM: Body-Scroll deaktivieren
OnboardingModal->>DOM: Focus setzen
User->>OnboardingModal: ESC drücken oder CTA/Close-Button klicken
OnboardingModal->>DOM: Modal verstecken, Body-Scroll aktivieren
OnboardingModal->>LocalStorage: onboarding-seen Flag setzen
else Rückkehrer
OnboardingModal->>OnboardingModal: Überspringe Auto-Show
User->>Header: Info-Button klicken
Header->>OnboardingModal: showOnboarding()
OnboardingModal->>DOM: Modal anzeigen (wie oben)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Logo already contains "Semantic Anchors" text, so remove the redundant slogan1 line. Center the video vertically in its column. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.gitignore (1)
4-4:icon.pngnur ausnehmen, wenn die Datei auch wirklich referenziert wird.Für
website/public/logo.pnggibt es im PR-Kontext einen klaren Consumer (website/src/components/onboarding-modal.js, dort wird${baseUrl}logo.pnggeladen). Fürwebsite/public/icon.pngfindet sich dagegen kein Verweis im gesamten Repository. Der HTML-Head verwendet einfavicon.svg, nichticon.png. Daher sollte die Ausnahme auf Line 4 entfernt werden, um zu vermeiden, dass eine ungenutzte Binärdatei bewusst im Repo landen.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 4, Die Ausnahme für website/public/icon.png in .gitignore sollte entfernt, also die Zeile "!website/public/icon.png" gelöscht werden, weil es im Repo keinen Verweis auf icon.png gibt (im Gegensatz zu website/public/logo.png, das in website/src/components/onboarding-modal.js verwendet wird) und der HTML-Head favicon.svg nutzt; entferne die Negation, sodass ungenutzte Binärdateien nicht versehentlich committet werden.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/2026-03-08-onboarding-modal-design.md`:
- Around line 30-33: Update the i18n key list so it matches the implementation
and translation files: replace the incorrect `onboarding.slogan` entry with the
actual keys `onboarding.slogan1` and `onboarding.slogan2` (and ensure
`onboarding.text1`–`text4`, `onboarding.cta`, `onboarding.watchVideo`,
`onboarding.infoButton` remain listed); alternatively, if you prefer a single
slogan key, change the implementation and en.json/de.json to use
`onboarding.slogan` everywhere—make the docs, translation files
(en.json/de.json) and code (places referencing `onboarding.slogan1`/`slogan2`)
consistent.
In `@website/src/components/onboarding-modal.js`:
- Around line 9-10: Wrap all direct localStorage accesses in try/catch and
provide safe fallbacks: modify shouldShowOnboarding to catch exceptions from
localStorage.getItem(STORAGE_KEY) and return the explicit fallback (e.g., treat
as not shown or true/false per current logic) when access fails; likewise locate
the code that writes to storage around the 146-153 region (calls to
localStorage.setItem with STORAGE_KEY) and wrap those in try/catch so failures
don’t abort app initialization, optionally logging the error and silently
skipping persistence. Ensure you reference STORAGE_KEY and preserve current
return semantics while avoiding uncaught exceptions from blocked storage.
- Around line 85-107: The iframe is merely hidden with CSS ("hidden sm:block")
but still present in the DOM so the embed request fires on mobile; change the
markup so the <iframe> (using embedUrl, title=watchVideo) is only rendered when
appropriate — either when the viewport is desktop-size (detect via a JS media
query like matchMedia for the "sm" breakpoint) or after an explicit user action
(e.g., set a showEmbed state when the user clicks the mobile link/button that
currently uses youtubeUrl/watchVideo); keep the mobile anchor for a no-embed
fallback and ensure all iframe attributes (frameborder, allow, allowfullscreen)
are preserved when conditionally rendering.
- Around line 27-31: The modal currently only sets initial focus and closes on
Escape but lacks a focus trap; implement a Tab/Shift+Tab focus trap inside the
onboarding modal so focus cannot move into the header/footer or iframe. In the
document keydown handler (where you check e.key === 'Escape' and call
closeOnboarding()), also intercept Tab presses, compute the list of focusable
elements within the modal element (modal), prevent default when Tab would move
focus out, and manually move focus to the first/last focusable element depending
on e.shiftKey; ensure this logic is used for both the modal open path and any
existing handlers referenced around closeOnboarding/openOnboarding so the trap
covers the sections mentioned (including the other block around lines 131-143).
---
Nitpick comments:
In @.gitignore:
- Line 4: Die Ausnahme für website/public/icon.png in .gitignore sollte
entfernt, also die Zeile "!website/public/icon.png" gelöscht werden, weil es im
Repo keinen Verweis auf icon.png gibt (im Gegensatz zu website/public/logo.png,
das in website/src/components/onboarding-modal.js verwendet wird) und der
HTML-Head favicon.svg nutzt; entferne die Negation, sodass ungenutzte
Binärdateien nicht versehentlich committet werden.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 30fffb8d-26ca-4d34-8104-c4c466c58227
⛔ Files ignored due to path filters (1)
website/public/logo.pngis excluded by!**/*.png
📒 Files selected for processing (8)
.gitignoredocs/plans/2026-03-08-onboarding-modal-design.mdwebsite/src/components/header.jswebsite/src/components/onboarding-modal.jswebsite/src/components/onboarding-modal.test.jswebsite/src/main.jswebsite/src/translations/de.jsonwebsite/src/translations/en.json
- Fix Chatham House Rule: change listing block (----) to quote block so example invocation renders as text, not source code (DE+EN) - Lighten modal overlays: bg-black/30 + backdrop-blur-sm instead of bg-black/50 for both anchor-modal and onboarding-modal - Add slogan to header: "One word, and the AI gets the rest." (lg+) - Add icon to header next to title (32x32px) - Replace SVG favicon with icon.png - Add .gitignore exceptions for logo.png and icon.png Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use the full logo image instead of "Semantic Anchors" text in the header. Show slogan as subtitle below the logo (sm+). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split header into two rows for better layout: - Row 1: Logo (full size), info button, language/theme toggles - Row 2: Navigation links (larger text), search input and role filter with visible borders Search and role filter in header sync bidirectionally with main content equivalents. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Logo spans both rows on desktop, nav in row 1, search/filter in row 2 - Move anchor count (56/56) into header row 2 - Hide duplicate search/filter in main content on desktop - Replace info (i) icon with play/video icon for onboarding button - Add sync-anchors.js script to reliably copy .adoc files from docs/anchors/ to website/public/docs/anchors/ before dev and build - Support mobile-specific IDs for theme, lang, and onboarding buttons Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
website/src/components/onboarding-modal.js (3)
9-10:⚠️ Potential issue | 🟠 Major
localStoragehier fehlertolerant behandeln.
getItemundsetItemkönnen in blockierten Storage-Kontexten werfen. WeilshouldShowOnboarding()im Startpfad liegt, kann das die App-Initialisierung abbrechen; beim Schließen würde zudem die Persistenz den Dismiss-Flow sprengen. Bitte beide Zugriffe intry/catchkapseln und einen expliziten Fallback definieren.As per coding guidelines
website/src/**/*.js: Check for: security issues, unused variables, proper error handling.Also applies to: 144-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.js` around lines 9 - 10, Wrap all direct localStorage accesses in try/catch and provide an explicit fallback: in shouldShowOnboarding() catch errors from localStorage.getItem(STORAGE_KEY) and return a safe default (e.g., true to show onboarding) on failure; likewise, locate the code path that writes dismissal (the corresponding setItem call) and wrap localStorage.setItem(STORAGE_KEY, 'true') in try/catch and silently fail or fallback to an in-memory flag so the dismiss flow still proceeds without throwing. Update both shouldShowOnboarding and the dismiss/save function to use these guarded accesses so blocked storage won't abort initialization or the dismiss flow.
83-105:⚠️ Potential issue | 🟠 MajorDer Mobile-Pfad rendert das Embed weiterhin mit.
hidden sm:blockversteckt dasiframenur visuell. Auf Mobile bleibt es trotzdem im DOM und kann den YouTube-Request direkt beim Auto-Open auslösen, obwohl#145dort explizit den Link-Fallback ohne Embed verlangt. Rendert dasiframebitte nur auf Desktop bzw. erst nach expliziter Nutzeraktion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.js` around lines 83 - 105, The iframe is still present in the DOM because it uses "hidden sm:block" which only visually hides it; change the component to render the iframe element conditionally (e.g., render iframe only when a "isIframeVisible" state or an "isDesktop" detection is true) instead of relying on CSS; add a user action (click handler on the play link/button that sets isIframeVisible = true) so the iframe with embedUrl and title watchVideo is created only after explicit user interaction (keep the youtubeUrl anchor fallback for mobile by preserving the sm:hidden link).
27-31:⚠️ Potential issue | 🟠 MajorDie Fokusfalle fehlt noch.
Aktuell setzt ihr nur den Initialfokus und schließt per
Escape. MitTab/Shift+Tabkann der Fokus aus dem Dialog herauswandern; sobald er im eingebetteten Player landet, istEscapezudem nicht mehr zuverlässig beim Modal. Damit ist das Accessibility-Kriterium aus#145noch nicht erfüllt.Also applies to: 129-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.js` around lines 27 - 31, Der Modal hat noch keine Focus-Trap: implementiere im onboarding-modal.js beim Öffnen das Speichern des vorherigen Fokus (z. B. prevActiveElement), setze Fokus auf das erste fokussierbare Element im modal und füge einen keydown-Handler (auf document oder modal) der Tab/Shift+Tab abfängt und den Fokus zyklisch innerhalb aller fokussierbaren Elemente des modal hält; beim Schließen (closeOnboarding) stelle den vorherigen Fokus wieder her und entferne die Handler; behalte den bestehenden Escape-Listener (document.addEventListener('keydown', ...)) aber stelle sicher, dass er weiterhin funktioniert, indem du die Focus-Trap so implementierst, dass der Fokus nie in den eingebetteten Player entweicht.
🧹 Nitpick comments (2)
website/package.json (1)
7-10: Die neuen Build-Skripte fallen weiter durch Linting und Formatting.Mit
sync-anchors.jskommt jetzt zusätzlicher JS-Build-Code dazu, aberlint,lint:fix,formatundformat:checkprüfen weiterhin nursrc/. Damit bleibt genau der neue Pre-Step außerhalb der regulären Code-Hygiene.🧰 Mögliche Anpassung
- "lint": "eslint src/", - "lint:fix": "eslint src/ --fix", - "format": "prettier --write src/", - "format:check": "prettier --check src/" + "lint": "eslint src/ ../scripts/sync-anchors.js ../scripts/render-docs.js", + "lint:fix": "eslint src/ ../scripts/sync-anchors.js ../scripts/render-docs.js --fix", + "format": "prettier --write src/ ../scripts/sync-anchors.js ../scripts/render-docs.js", + "format:check": "prettier --check src/ ../scripts/sync-anchors.js ../scripts/render-docs.js"As per coding guidelines, "JavaScript and Python build scripts should include ESLint and Prettier configuration for code formatting and linting".
Also applies to: 21-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/package.json` around lines 7 - 10, The new build script sync-anchors.js (invoked by the npm scripts "sync-anchors", "predev", and "prebuild") lives outside src/ but your npm tasks lint, lint:fix, format and format:check only target src/, so the build script escapes ESLint/Prettier. Update the package.json task globs for "lint", "lint:fix", "format" and "format:check" to also include the build script path (e.g. ../scripts/**/*.js or the specific sync-anchors.js) and ensure your ESLint/Prettier configs apply to those files (or add an overrides entry) so sync-anchors.js is linted/formatted with the same rules.website/src/main.js (1)
288-310: Die Header-spezifischen Binder duplizieren inzwischen die allgemeine Sync-Logik.
populateHeaderRoleFilter()/bindHeaderSearchInput()undbindRoleFilter()/bindSearchInput()pflegen dieselben Elemente mit leicht abweichendem Verhalten. Das erhöht die Drift-Gefahr bei weiteren Änderungen an Sync, Placeholdern und Initialzustand. Ich würde das auf einen gemeinsamen Initialisierungs-/Binding-Pfad reduzieren.Also applies to: 313-330
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/main.js` around lines 288 - 310, populateHeaderRoleFilter and the header-specific bindHeaderSearchInput duplicate logic from bindRoleFilter/bindSearchInput; extract a single helper (e.g., bindRoleAndSearch or initFilters) that accepts the role select element and search input element (and an optional syncTarget element id) and performs: populating options from appData.roles, setting placeholder/initial value, wiring onchange/oninput to call applyCardFilters and to sync the paired control (main ↔ header). Replace populateHeaderRoleFilter and bindHeaderSearchInput calls with calls to this new helper for both header and main controls so all sync, placeholder, and initial-state logic lives in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/sync-anchors.js`:
- Around line 27-47: Das Script kopiert nur neue/aktualisierte .adoc aus SRC
nach DEST (siehe srcFiles, SRC, DEST, srcPath, destPath, fs.copyFileSync) und
entfernt nicht mehr existierende Dateien; ergänze nach dem Einlesen von srcFiles
und vor/ oder nach der Kopierschleife eine Bereinigung: liste alle .adoc in DEST
(z.B. destFiles = fs.readdirSync(DEST).filter(...)) und für jede Datei, die
nicht in srcFiles vorkommt, rufe fs.unlinkSync(path.join(DEST, file)) auf,
sodass entfernte/umbenannte Anchors aus DEST gelöscht werden; beibehalte die
bestehende mtime-Vergleichslogik und die Zähler (copied/skipped) unverändert.
In `@website/src/components/header.js`:
- Around line 17-29: Replace the incorrect "play" SVG used in the header control
with the required info icon for the element with id="onboarding-info-btn" so
desktop matches the requested Info-Icon from issue `#145`; update the SVG inside
the button (and the corresponding mobile instance referenced around the other
block at lines 92-103) to the consistent info icon SVG (preserve classes,
attributes like aria-label/title using i18n.t('onboarding.infoButton'), and
styling/hover behavior) so desktop and mobile show the same icon and
accessibility labels.
In `@website/src/main.js`:
- Around line 86-89: The placeholder suffix " (full-text)" is hardcoded in
main.js causing mixed-language placeholders; change the concatenation to use an
i18n key instead (e.g. replace the literal suffix in the loop that sets
placeholders for 'search-input' and 'header-search-input' to use
i18n.t('search.fulltext_suffix') or a similar key), update or add that
translation key for all locales (de, en, etc.), and compose the final
placeholder via i18n.t('search.placeholder') + ' ' +
i18n.t('search.fulltext_suffix') (or use a single combined key like
i18n.t('search.placeholder_fulltext')) so the suffix is localized.
- Around line 229-286: Before rebinding controls, capture the current header
state (document.getElementById('header-role-filter')?.value and
document.getElementById('header-search-input')?.value) and use those values to
initialize both control sets inside bindRoleFilter and bindSearchInput; when
creating options in bindRoleFilter (roleFilterIds / appData.roles) set the
select.value to the captured header role and also write that value into the
other select, and in bindSearchInput (searchInputIds) set each input.value to
the captured header query and sync both inputs; after finishing the bindings
call applyCardFilters(capturedRole, capturedQuery) once (and still call
triggerSearchIndexBuild if the capturedQuery is non-empty) so filters and UI
stay consistent across route changes.
---
Duplicate comments:
In `@website/src/components/onboarding-modal.js`:
- Around line 9-10: Wrap all direct localStorage accesses in try/catch and
provide an explicit fallback: in shouldShowOnboarding() catch errors from
localStorage.getItem(STORAGE_KEY) and return a safe default (e.g., true to show
onboarding) on failure; likewise, locate the code path that writes dismissal
(the corresponding setItem call) and wrap localStorage.setItem(STORAGE_KEY,
'true') in try/catch and silently fail or fallback to an in-memory flag so the
dismiss flow still proceeds without throwing. Update both shouldShowOnboarding
and the dismiss/save function to use these guarded accesses so blocked storage
won't abort initialization or the dismiss flow.
- Around line 83-105: The iframe is still present in the DOM because it uses
"hidden sm:block" which only visually hides it; change the component to render
the iframe element conditionally (e.g., render iframe only when a
"isIframeVisible" state or an "isDesktop" detection is true) instead of relying
on CSS; add a user action (click handler on the play link/button that sets
isIframeVisible = true) so the iframe with embedUrl and title watchVideo is
created only after explicit user interaction (keep the youtubeUrl anchor
fallback for mobile by preserving the sm:hidden link).
- Around line 27-31: Der Modal hat noch keine Focus-Trap: implementiere im
onboarding-modal.js beim Öffnen das Speichern des vorherigen Fokus (z. B.
prevActiveElement), setze Fokus auf das erste fokussierbare Element im modal und
füge einen keydown-Handler (auf document oder modal) der Tab/Shift+Tab abfängt
und den Fokus zyklisch innerhalb aller fokussierbaren Elemente des modal hält;
beim Schließen (closeOnboarding) stelle den vorherigen Fokus wieder her und
entferne die Handler; behalte den bestehenden Escape-Listener
(document.addEventListener('keydown', ...)) aber stelle sicher, dass er
weiterhin funktioniert, indem du die Focus-Trap so implementierst, dass der
Fokus nie in den eingebetteten Player entweicht.
---
Nitpick comments:
In `@website/package.json`:
- Around line 7-10: The new build script sync-anchors.js (invoked by the npm
scripts "sync-anchors", "predev", and "prebuild") lives outside src/ but your
npm tasks lint, lint:fix, format and format:check only target src/, so the build
script escapes ESLint/Prettier. Update the package.json task globs for "lint",
"lint:fix", "format" and "format:check" to also include the build script path
(e.g. ../scripts/**/*.js or the specific sync-anchors.js) and ensure your
ESLint/Prettier configs apply to those files (or add an overrides entry) so
sync-anchors.js is linted/formatted with the same rules.
In `@website/src/main.js`:
- Around line 288-310: populateHeaderRoleFilter and the header-specific
bindHeaderSearchInput duplicate logic from bindRoleFilter/bindSearchInput;
extract a single helper (e.g., bindRoleAndSearch or initFilters) that accepts
the role select element and search input element (and an optional syncTarget
element id) and performs: populating options from appData.roles, setting
placeholder/initial value, wiring onchange/oninput to call applyCardFilters and
to sync the paired control (main ↔ header). Replace populateHeaderRoleFilter and
bindHeaderSearchInput calls with calls to this new helper for both header and
main controls so all sync, placeholder, and initial-state logic lives in one
place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: cc273764-e327-4e6a-8d38-93f89df9267b
⛔ Files ignored due to path filters (1)
website/public/icon.pngis excluded by!**/*.png
📒 Files selected for processing (14)
docs/anchors/chatham-house-rule.adocdocs/anchors/chatham-house-rule.de.adocscripts/sync-anchors.jswebsite/index.htmlwebsite/package.jsonwebsite/src/components/anchor-modal.jswebsite/src/components/card-grid.jswebsite/src/components/header.jswebsite/src/components/main-content.jswebsite/src/components/onboarding-modal.jswebsite/src/components/onboarding-modal.test.jswebsite/src/main.jswebsite/src/translations/de.jsonwebsite/src/translations/en.json
✅ Files skipped from review due to trivial changes (2)
- docs/anchors/chatham-house-rule.de.adoc
- website/index.html
Comment on lines
+27
to
+47
| const srcFiles = fs.readdirSync(SRC).filter((f) => f.endsWith('.adoc')) | ||
| let copied = 0 | ||
| let skipped = 0 | ||
|
|
||
| for (const file of srcFiles) { | ||
| const srcPath = path.join(SRC, file) | ||
| const destPath = path.join(DEST, file) | ||
|
|
||
| const srcStat = fs.statSync(srcPath) | ||
|
|
||
| if (fs.existsSync(destPath)) { | ||
| const destStat = fs.statSync(destPath) | ||
| if (srcStat.mtimeMs <= destStat.mtimeMs) { | ||
| skipped++ | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| fs.copyFileSync(srcPath, destPath) | ||
| copied++ | ||
| } |
Contributor
There was a problem hiding this comment.
Das ist noch kein vollständiger Sync.
Wenn ein Anchor in docs/anchors gelöscht oder umbenannt wird, bleibt die alte .adoc in website/public/docs/anchors erhalten, weil hier nur kopiert und nie bereinigt wird. Dadurch kann die Website veraltete Dokumente weiter ausliefern.
🧹 Mögliche Ergänzung
const srcFiles = fs.readdirSync(SRC).filter((f) => f.endsWith('.adoc'))
+ const destFiles = fs.readdirSync(DEST).filter((f) => f.endsWith('.adoc'))
let copied = 0
let skipped = 0
+
+ for (const file of destFiles) {
+ if (!srcFiles.includes(file)) {
+ fs.unlinkSync(path.join(DEST, file))
+ }
+ }
for (const file of srcFiles) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sync-anchors.js` around lines 27 - 47, Das Script kopiert nur
neue/aktualisierte .adoc aus SRC nach DEST (siehe srcFiles, SRC, DEST, srcPath,
destPath, fs.copyFileSync) und entfernt nicht mehr existierende Dateien; ergänze
nach dem Einlesen von srcFiles und vor/ oder nach der Kopierschleife eine
Bereinigung: liste alle .adoc in DEST (z.B. destFiles =
fs.readdirSync(DEST).filter(...)) und für jede Datei, die nicht in srcFiles
vorkommt, rufe fs.unlinkSync(path.join(DEST, file)) auf, sodass
entfernte/umbenannte Anchors aus DEST gelöscht werden; beibehalte die bestehende
mtime-Vergleichslogik und die Zähler (copied/skipped) unverändert.
Comment on lines
17
to
29
| <button | ||
| id="mobile-menu-toggle" | ||
| class="sm:hidden rounded-md p-2 text-[var(--color-text-secondary)] hover:text-[var(--color-text)] hover:bg-[var(--color-bg-secondary)] transition-colors" | ||
| aria-label="Toggle menu" | ||
| aria-expanded="false" | ||
| id="onboarding-info-btn" | ||
| class="self-start mt-1 rounded-full p-1 text-[var(--color-text-secondary)] hover:text-[var(--color-primary)] hover:bg-[var(--color-bg-secondary)] transition-colors" | ||
| data-i18n-aria="onboarding.infoButton" | ||
| data-i18n-title="onboarding.infoButton" | ||
| aria-label="${i18n.t('onboarding.infoButton')}" | ||
| title="${i18n.t('onboarding.infoButton')}" | ||
| > | ||
| <svg class="h-6 w-6" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" d="M3.75 6.75h16.5M3.75 12h16.5m-16.5 5.25h16.5" /> | ||
| <svg class="h-5 w-5" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" d="M21 12a9 9 0 11-18 0 9 9 0 0118 0z" /> | ||
| <path stroke-linecap="round" stroke-linejoin="round" d="M15.91 11.672a.375.375 0 010 .656l-5.603 3.113a.375.375 0 01-.557-.328V8.887c0-.286.307-.466.557-.327l5.603 3.112z" /> | ||
| </svg> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Desktop-Trigger weicht vom geforderten Info-Icon ab.
Das Control neben dem Logo zeigt hier ein Play-Symbol, obwohl #145 explizit ein Info-Icon im Header fordert. Zusätzlich ist das Verhalten damit zwischen Desktop und Mobile inkonsistent.
Also applies to: 92-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/components/header.js` around lines 17 - 29, Replace the incorrect
"play" SVG used in the header control with the required info icon for the
element with id="onboarding-info-btn" so desktop matches the requested Info-Icon
from issue `#145`; update the SVG inside the button (and the corresponding mobile
instance referenced around the other block at lines 92-103) to the consistent
info icon SVG (preserve classes, attributes like aria-label/title using
i18n.t('onboarding.infoButton'), and styling/hover behavior) so desktop and
mobile show the same icon and accessibility labels.
Comment on lines
229
to
+286
| function bindRoleFilter() { | ||
| const roleFilter = document.getElementById('role-filter') | ||
| const roleFilterIds = ['role-filter', 'header-role-filter'] | ||
|
|
||
| roleFilterIds.forEach((id) => { | ||
| const roleFilter = document.getElementById(id) | ||
| if (!roleFilter || !appData?.roles) return | ||
|
|
||
| while (roleFilter.options.length > 1) { | ||
| roleFilter.remove(1) | ||
| } | ||
|
|
||
| appData.roles.forEach((role) => { | ||
| const option = document.createElement('option') | ||
| option.value = role.id | ||
| option.textContent = role.name | ||
| roleFilter.appendChild(option) | ||
| }) | ||
|
|
||
| roleFilter.onchange = (e) => { | ||
| // Sync the other dropdown | ||
| roleFilterIds.forEach((otherId) => { | ||
| if (otherId !== id) { | ||
| const other = document.getElementById(otherId) | ||
| if (other) other.value = e.target.value | ||
| } | ||
| }) | ||
| const searchQuery = document.getElementById('header-search-input')?.value | ||
| || document.getElementById('search-input')?.value || '' | ||
| applyCardFilters(e.target.value, searchQuery) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| function bindSearchInput() { | ||
| const searchInputIds = ['search-input', 'header-search-input'] | ||
|
|
||
| searchInputIds.forEach((id) => { | ||
| const searchInput = document.getElementById(id) | ||
| if (!searchInput) return | ||
|
|
||
| searchInput.oninput = (e) => { | ||
| const query = e.target.value | ||
| // Sync the other search input | ||
| searchInputIds.forEach((otherId) => { | ||
| if (otherId !== id) { | ||
| const other = document.getElementById(otherId) | ||
| if (other) other.value = query | ||
| } | ||
| }) | ||
| if (query.trim()) { | ||
| triggerSearchIndexBuild() | ||
| } | ||
| const roleId = document.getElementById('header-role-filter')?.value | ||
| || document.getElementById('role-filter')?.value || '' | ||
| applyCardFilters(roleId, query) | ||
| } | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Der Filterzustand wird beim Zurückkehren zur Startseite nicht sauber rehydratisiert.
Die Header-Controls überleben den Routenwechsel, die Main-Controls werden aber neu gerendert. bindSearchInput() übernimmt den bestehenden Header-Query dabei nicht initial, und bindRoleFilter() baut den Header-Select neu auf, wodurch Auswahl und Ergebnisliste wieder auseinanderlaufen. Bitte den aktuellen Header-State vor dem Rebinding sichern, auf beide Control-Sets zurückschreiben und danach einmal applyCardFilters() ausführen.
Möglicher Fix-Sketch
function initCardGridVisualization() {
if (!appData) return
+ const initialRoleId = document.getElementById('header-role-filter')?.value || ''
+ const initialQuery = document.getElementById('header-search-input')?.value || ''
+
const container = document.getElementById('main-content')
if (container) {
container.innerHTML = renderCardGrid(appData.categories, appData.anchors)
}
initCardGrid()
updateAnchorCount(appData.anchors.length, appData.anchors.length)
- bindRoleFilter()
- bindSearchInput()
+ bindRoleFilter(initialRoleId)
+ bindSearchInput(initialQuery)
+ applyCardFilters(initialRoleId, initialQuery)
}
-function bindRoleFilter() {
+function bindRoleFilter(initialRoleId = '') {
const roleFilterIds = ['role-filter', 'header-role-filter']
roleFilterIds.forEach((id) => {
const roleFilter = document.getElementById(id)
if (!roleFilter || !appData?.roles) return
while (roleFilter.options.length > 1) {
roleFilter.remove(1)
}
appData.roles.forEach((role) => {
const option = document.createElement('option')
option.value = role.id
option.textContent = role.name
roleFilter.appendChild(option)
})
+
+ roleFilter.value = initialRoleId
})
}
-function bindSearchInput() {
+function bindSearchInput(initialQuery = '') {
const searchInputIds = ['search-input', 'header-search-input']
searchInputIds.forEach((id) => {
const searchInput = document.getElementById(id)
if (!searchInput) return
+
+ searchInput.value = initialQuery🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/main.js` around lines 229 - 286, Before rebinding controls,
capture the current header state
(document.getElementById('header-role-filter')?.value and
document.getElementById('header-search-input')?.value) and use those values to
initialize both control sets inside bindRoleFilter and bindSearchInput; when
creating options in bindRoleFilter (roleFilterIds / appData.roles) set the
select.value to the captured header role and also write that value into the
other select, and in bindSearchInput (searchInputIds) set each input.value to
the captured header query and sync both inputs; after finishing the bindings
call applyCardFilters(capturedRole, capturedQuery) once (and still call
triggerSearchIndexBuild if the capturedQuery is non-empty) so filters and UI
stay consistent across route changes.
- Add try/catch around localStorage in shouldShowOnboarding() and closeOnboarding() - Use window.matchMedia for conditional iframe rendering (no iframe on mobile) - Add focus trap (Tab/Shift+Tab) in modal keydown handler - Fix i18n key names in design doc (slogan1/slogan2) - Add matchMedia mock in tests for jsdom compatibility Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
Review-Kommentare adressiert (Commit 3e396ca)Behoben:
Nicht geändert (bewusst):
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
website/src/components/onboarding-modal.js (2)
31-60: Globaler Event-Listener wird bei jedemcreateOnboardingModal()-Aufruf hinzugefügt.Der
document.addEventListener('keydown', ...)wird innerhalb voncreateOnboardingModal()registriert. Zwar verhindert der Singleton-Check (Zeile 18) mehrfache Aufrufe im normalen Flow, aber wenn das Modal je programmatisch entfernt und neu erstellt würde, würden sich Listener akkumulieren.Da dies im aktuellen Design unwahrscheinlich ist (Singleton-Pattern), ist dies nur ein geringes Risiko – aber für zukünftige Robustheit könnte der Listener außerhalb der Funktion oder mit einem Guard registriert werden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.js` around lines 31 - 60, The keydown listener is being added inside createOnboardingModal via document.addEventListener('keydown', ...) which can accumulate if the modal is recreated; move registration out of createOnboardingModal or add a guard to ensure the listener is only added once (e.g., a module-scoped boolean like onboardingKeydownBound) and/or remove the listener in closeOnboarding; update references to modal and closeOnboarding so the shared listener still checks modal.classList.contains('hidden') and manages focus trapping without duplicating handlers.
114-137:matchMedia-Check ist nicht reaktiv bei Resize.Der
window.matchMedia('(min-width: 640px)').matches-Check (Zeile 114) wird einmalig beim Rendern des Modal-Inhalts ausgeführt. Wenn ein Benutzer das Modal auf Desktop öffnet und dann das Fenster verkleinert (oder umgekehrt), bleibt der ursprünglich gerenderte Inhalt (iframe vs. Link) unverändert.Dies ist wahrscheinlich akzeptabel für einen Modal, der typischerweise schnell geschlossen wird, aber für eine vollständig responsive Lösung könnte ein
resize-Listener oder CSS-only Ansatz verwendet werden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.js` around lines 114 - 137, The one-time window.matchMedia('(min-width: 640px)').matches check causes non-reactive rendering of the iframe vs link; replace it by deriving a reactive boolean (e.g., isWide) from window.matchMedia and subscribing to its 'change' event (or window resize) so the component re-renders when viewport crosses 640px, then use isWide to choose between the iframe (embedUrl, watchVideo) and the fallback link (youtubeUrl, watchVideo); alternatively, remove the JS media check entirely and always render the iframe but toggle its visibility/responsiveness with CSS utility classes so the layout adapts without JS.website/src/components/onboarding-modal.test.js (2)
39-48: Fehlender Test für localStorage-Fehlerbehandlung.Die Implementierung von
shouldShowOnboarding()hat einen try/catch-Block, der bei Speicherfehlernfalsezurückgibt. Ein Test für diesen Edge-Case würde die Robustheit der Testsuite verbessern.💡 Vorgeschlagener zusätzlicher Test
it('returns false when localStorage throws an error', () => { const originalGetItem = localStorage.getItem localStorage.getItem = () => { throw new Error('Storage blocked') } expect(shouldShowOnboarding()).toBe(false) localStorage.getItem = originalGetItem })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.test.js` around lines 39 - 48, Add a unit test to cover the localStorage error path for shouldShowOnboarding(): simulate localStorage.getItem throwing (save original, replace with a function that throws, call shouldShowOnboarding() and expect false, then restore original) so the try/catch branch in shouldShowOnboarding() is exercised and the test suite remains robust.
158-168: Fokus-Trap-Test fehlt.Der Code implementiert eine Fokus-Falle für Tab/Shift+Tab (gemäß Issue
#145), aber es gibt keinen Test dafür. Dies ist ein wichtiges Accessibility-Feature.💡 Vorgeschlagene Fokus-Trap-Tests
it('traps focus within modal on Tab', () => { createOnboardingModal() showOnboarding() const modal = document.getElementById('onboarding-modal') const focusableElements = modal.querySelectorAll('button, [href], iframe') const lastElement = focusableElements[focusableElements.length - 1] lastElement.focus() document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab' })) // Focus should wrap to first element expect(document.activeElement).toBe(focusableElements[0]) }) it('traps focus within modal on Shift+Tab', () => { createOnboardingModal() showOnboarding() const modal = document.getElementById('onboarding-modal') const focusableElements = modal.querySelectorAll('button, [href], iframe') const firstElement = focusableElements[0] firstElement.focus() document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', shiftKey: true })) // Focus should wrap to last element expect(document.activeElement).toBe(focusableElements[focusableElements.length - 1]) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/onboarding-modal.test.js` around lines 158 - 168, Add two tests to verify the focus-trap behavior for Tab and Shift+Tab using the existing helpers createOnboardingModal() and showOnboarding(): (1) create the modal, show it, query focusable elements inside document.getElementById('onboarding-modal'), focus the last element, dispatch a KeyboardEvent('keydown', { key: 'Tab' }) and assert document.activeElement is the first focusable element; (2) create/show modal, focus the first focusable element, dispatch KeyboardEvent('keydown', { key: 'Tab', shiftKey: true }) and assert document.activeElement is the last focusable element; ensure you select focusable elements similarly to the suggested query (e.g., 'button, [href], iframe') and place both tests alongside the existing keyboard interaction describe block to cover Issue `#145`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/components/onboarding-modal.js`:
- Around line 31-60: The keydown listener is being added inside
createOnboardingModal via document.addEventListener('keydown', ...) which can
accumulate if the modal is recreated; move registration out of
createOnboardingModal or add a guard to ensure the listener is only added once
(e.g., a module-scoped boolean like onboardingKeydownBound) and/or remove the
listener in closeOnboarding; update references to modal and closeOnboarding so
the shared listener still checks modal.classList.contains('hidden') and manages
focus trapping without duplicating handlers.
- Around line 114-137: The one-time window.matchMedia('(min-width:
640px)').matches check causes non-reactive rendering of the iframe vs link;
replace it by deriving a reactive boolean (e.g., isWide) from window.matchMedia
and subscribing to its 'change' event (or window resize) so the component
re-renders when viewport crosses 640px, then use isWide to choose between the
iframe (embedUrl, watchVideo) and the fallback link (youtubeUrl, watchVideo);
alternatively, remove the JS media check entirely and always render the iframe
but toggle its visibility/responsiveness with CSS utility classes so the layout
adapts without JS.
In `@website/src/components/onboarding-modal.test.js`:
- Around line 39-48: Add a unit test to cover the localStorage error path for
shouldShowOnboarding(): simulate localStorage.getItem throwing (save original,
replace with a function that throws, call shouldShowOnboarding() and expect
false, then restore original) so the try/catch branch in shouldShowOnboarding()
is exercised and the test suite remains robust.
- Around line 158-168: Add two tests to verify the focus-trap behavior for Tab
and Shift+Tab using the existing helpers createOnboardingModal() and
showOnboarding(): (1) create the modal, show it, query focusable elements inside
document.getElementById('onboarding-modal'), focus the last element, dispatch a
KeyboardEvent('keydown', { key: 'Tab' }) and assert document.activeElement is
the first focusable element; (2) create/show modal, focus the first focusable
element, dispatch KeyboardEvent('keydown', { key: 'Tab', shiftKey: true }) and
assert document.activeElement is the last focusable element; ensure you select
focusable elements similarly to the suggested query (e.g., 'button, [href],
iframe') and place both tests alongside the existing keyboard interaction
describe block to cover Issue `#145`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6a5cf300-7128-4679-9f5a-6c5b88590ed6
📒 Files selected for processing (3)
docs/plans/2026-03-08-onboarding-modal-design.mdwebsite/src/components/onboarding-modal.jswebsite/src/components/onboarding-modal.test.js
- .gitignore: keep both logo.png and icon.png exceptions - header.js: keep redesigned two-row desktop layout Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused slogan1 variable (logo already shows brand name) - Fix Prettier formatting in onboarding-modal.js and main.js Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace hardcoded '(full-text)' with i18n key 'search.fullText' so it displays '(Volltext)' in German. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #145
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit