Skip to content

fix: Lighthouse a11y + CLS issues on homepage#431

Merged
rdmueller merged 2 commits into
LLM-Coding:mainfrom
raifdmueller:fix/lighthouse-a11y-perf
Apr 14, 2026
Merged

fix: Lighthouse a11y + CLS issues on homepage#431
rdmueller merged 2 commits into
LLM-Coding:mainfrom
raifdmueller:fix/lighthouse-a11y-perf

Conversation

@raifdmueller
Copy link
Copy Markdown
Contributor

@raifdmueller raifdmueller commented Apr 14, 2026

Summary

Real issues surfaced after #430 pointed Lighthouse at the canonical URL. Current scores on llm-coding.github.io/Semantic-Anchors/:

  • Performance: 0.75 (target ≥ 0.9)
  • Accessibility: 0.92 (target = 1.0)

The single failing perf audit with non-zero weight is Cumulative Layout Shift (weight 25, score 0.02). Accessibility has 4 failing audits. This PR addresses each root cause.

Accessibility fixes

Audit Root cause Fix
aria-allowed-role <article role="button"> — article has implicit role "article", overriding to "button" is invalid per ARIA in HTML <article><div role="button">
label-content-name-mismatch aria-label="Open {title} details" doesn't start with the visible h3 text Replaced with aria-labelledby pointing at the h3 id, so the accessible name exactly matches the visible title
select-name #header-role-filter had no label Added aria-label with existing filter.allRoles translation
color-contrast .meta-badge inherited gray-500/400 text on gray-100/700 bg → ~3.9:1 / ~3.1:1 (below WCAG AA 4.5:1) Explicit text-gray-700 dark:text-gray-200 → ~5.3:1 in both themes
label-content-name-mismatch (lang-toggle) Visible text "DE"/"EN", aria-label="Toggle language" — no overlap New header.langToggleAria string that starts with the visible text: "DE — switch to German"

Performance fixes

Audit Root cause Fix
cumulative-layout-shift (weight 25) #page-content empty on first paint, populated async with card grid min-height: calc(100vh - 12rem) reserves viewport space
unsized-images Logo images had no explicit width/height Added width="218" height="96" (desktop) / width="145" height="64" (mobile) from actual 1024×451 aspect ratio, plus w-auto so CSS height constraint still wins

Test plan

  • Unit tests pass (89/89)
  • Vite build completes with 9 pre-rendered routes
  • After merge, Lighthouse CI against canonical URL shows Performance ≥ 0.9 and Accessibility = 1.0

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Verbesserungen für Barrierefreiheit

    • Verbesserte Beschriftungen für Sprachumschalter und Rollenfilter für Screenreader-Nutzer
  • Stil

    • Erhöhter Textkontrast für Badge-Styling zur besseren Lesbarkeit
    • Konsistente Logo-Größen und optimiertes Seitenlayout
  • Bug-Behebung

    • Reduziert unerwünschte Layout-Verschiebung beim Laden von Inhalten

Addresses real failures surfaced after PR LLM-Coding#430 pointed Lighthouse at
the canonical deployed URL instead of the stale fork URL.

## Accessibility (0.92 → target 1.0)

### aria-allowed-role + label-content-name-mismatch
Anchor cards used <article role="button" aria-label="Open {title}
details">, which fails two axe rules:
- <article> has implicit role "article"; overriding to "button" is
  invalid per ARIA in HTML spec
- aria-label started with "Open" instead of the visible h3 text, so
  the accessible name didn't match the visible label

Fixed by changing <article> to <div role="button"> and using
aria-labelledby pointing at the h3 id. Accessible name now exactly
matches the visible title.

Event delegation uses the .anchor-card class, not the tag name, so
nothing breaks.

### select-name
#header-role-filter had no associated label. Added aria-label
referencing the existing "All roles" translation string.

### color-contrast
.meta-badge text inherited .anchor-card-meta's gray-500/400 colors,
which only reach ~3.9:1 (light) / ~3.1:1 (dark) on the badge's
gray-100/700 background — below the WCAG AA 4.5:1 threshold for
normal text. Explicitly set gray-700/200 on .meta-badge for ~5.3:1
in both themes.

## Performance (0.75 → target ≥ 0.9)

### cumulative-layout-shift (weight 25)
#page-content started empty and populated asynchronously with the
card grid, causing the single biggest CLS contributor on the page.
Added min-height: calc(100vh - 12rem) to reserve viewport space so
the browser doesn't shift layout when the content arrives.

### unsized-images
Logo images (header + onboarding modal) had no explicit width/height,
so the browser couldn't reserve space and had to reflow when the
image loaded. Added width/height attributes derived from the actual
logo.png aspect ratio (1024×451) and added w-auto so the CSS height
constraint still wins visually.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 14, 2026

Warning

Rate limit exceeded

@raifdmueller has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 28 minutes and 47 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 47 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 7275af72-f8dd-4797-b673-641781c7a82a

📥 Commits

Reviewing files that changed from the base of the PR and between d275104 and f999826.

📒 Files selected for processing (3)
  • website/src/components/card-grid.js
  • website/src/components/header.js
  • website/src/translations/de.json

Walkthrough

Das PR aktualisiert Website-Komponenten mit verbesserten Barrierefreiheitsattributen und expliziten Bildabmessungen. Das includes Änderungen am <article>-Element zu <div> in der Kartengrid-Komponente, Hinzufügen von i18n-Aria-Labels zur Kopfzeile und Übersetzungen, sowie CSS-Anpassungen für das Seitenlayout und die Badge-Styling.

Changes

Cohort / File(s) Zusammenfassung
Komponenten-Barrierefreiheit
website/src/components/card-grid.js
Ändert äußeres Kartenelement von <article> zu <div>, aktualisiert aria-label zu aria-labelledby mit deterministischer Titel-ID (anchor-card-title-${safeId}) für bessere Barrierefreiheit.
Logo- und ARIA-Attribute
website/src/components/header.js, website/src/components/onboarding-modal.js
Fügt explizite width/height-Attribute zu Logo-<img>-Elementen hinzu; aktualisiert Sprachschalt- und Rollenfilter-aria-label mit lokalisierten Werten und data-i18n-aria-Attributen.
Styling und Layout
website/src/styles/main.css
Ergänzt #page-content-Regel mit min-height: calc(100vh - 12rem) zur Verringerung von Layout-Verschiebungen; aktualisiert .meta-badge-Textfarbe auf höheren Kontrast (text-gray-700 dark:text-gray-200).
Übersetzungen
website/src/translations/en.json, website/src/translations/de.json
Fügt header.langToggleAria-Schlüssel für lokalisierte Aria-Labels der Sprachschalter hinzu ("DE — switch to German" / "EN — switch to English").

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Der Titel präzise zusammenfasst die Hauptänderungen des Pull Requests: Behebung von Lighthouse-Problemen bei Barrierefreiheit und Cumulative Layout Shift auf der Homepage.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
website/src/components/card-grid.js (1)

122-125: aria-labelledby-Ziel-ID sollte pro Karteninstanz eindeutig sein.

Aktuell hängt die ID nur von anchor.id ab. Falls ein Anchor mehrfach gerendert wird, entstehen doppelte IDs im DOM.

♻️ Vorschlag für eindeutige IDs
-        ${categoryAnchors.map((anchor) => renderAnchorCard(anchor, color)).join('')}
+        ${categoryAnchors.map((anchor) => renderAnchorCard(anchor, color, category.id)).join('')}

-function renderAnchorCard(anchor, categoryColor) {
+function renderAnchorCard(anchor, categoryColor, categoryId) {
   const isUmbrella = anchor.subAnchors && anchor.subAnchors.length > 0
@@
   const safeId = escapeHtml(anchor.id)
+  const safeCategoryId = escapeHtml(categoryId)
+  const cardTitleId = `anchor-card-title-${safeCategoryId}-${safeId}`

   return `
     <div
@@
-      aria-labelledby="anchor-card-title-${safeId}"
+      aria-labelledby="${cardTitleId}"
     >
       <div class="anchor-card-header">
-        <h3 id="anchor-card-title-${safeId}" class="anchor-card-title">${escapeHtml(anchor.title)}</h3>
+        <h3 id="${cardTitleId}" class="anchor-card-title">${escapeHtml(anchor.title)}</h3>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/src/components/card-grid.js` around lines 122 - 125, The
aria-labelledby target must be unique per card instance: update the markup that
uses safeId/anchor.id so the generated id for the heading and the
aria-labelledby attribute includes an instance-unique suffix (e.g., a
per-instance counter, timestamp or short UUID) and use that same composed id for
both the aria-labelledby and the h3 id (symbols to change: safeId,
"anchor-card-title-${...}", and the aria-labelledby attribute in the card render
in card-grid.js) so duplicated anchors cannot produce duplicate DOM ids.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@website/src/components/header.js`:
- Around line 57-58: The mobile language toggle still uses a hardcoded
aria-label ("Toggle language"); update the mobile toggle element in header.js
(the mobile language toggle button) to use i18n.t('header.langToggleAria') for
its aria-label and add the same data-i18n-aria="header.langToggleAria" attribute
as the desktop toggle so the accessible name matches localized text. Locate the
mobile toggle button (the element with aria-label="Toggle language") and replace
the static string with the i18n lookup and add the data-i18n-aria attribute.

In `@website/src/translations/de.json`:
- Line 5: The de.json entry for "header.langToggleAria" currently mixes English
explanatory text; update the value so the visible language code remains "EN" but
the explanatory phrase is German (e.g., "EN — zu Englisch wechseln") so
screenreader and UI are fully localized; locate the "header.langToggleAria" key
in website/src/translations/de.json and replace the English phrase with the
German equivalent while keeping the "EN" token intact.

---

Nitpick comments:
In `@website/src/components/card-grid.js`:
- Around line 122-125: The aria-labelledby target must be unique per card
instance: update the markup that uses safeId/anchor.id so the generated id for
the heading and the aria-labelledby attribute includes an instance-unique suffix
(e.g., a per-instance counter, timestamp or short UUID) and use that same
composed id for both the aria-labelledby and the h3 id (symbols to change:
safeId, "anchor-card-title-${...}", and the aria-labelledby attribute in the
card render in card-grid.js) so duplicated anchors cannot produce duplicate DOM
ids.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: b8c5b3b1-d8f5-42d2-bbaf-6bbcafc4f83e

📥 Commits

Reviewing files that changed from the base of the PR and between ddcf64e and d275104.

📒 Files selected for processing (6)
  • website/src/components/card-grid.js
  • website/src/components/header.js
  • website/src/components/onboarding-modal.js
  • website/src/styles/main.css
  • website/src/translations/de.json
  • website/src/translations/en.json

Comment thread website/src/components/header.js
Comment thread website/src/translations/de.json Outdated
1. Mobile lang-toggle still had hardcoded aria-label="Toggle language".
   Now uses i18n.t('header.langToggleAria') + data-i18n-aria, matching
   the desktop fix from the parent commit.

2. de.json aria-label was English ("EN — switch to English"). Now
   properly localized: "EN — zu Englisch wechseln".

3. Anchor card heading ids are now namespaced with the category id.
   An anchor can belong to multiple categories and is rendered once per
   category section, so the previous "anchor-card-title-<id>" pattern
   produced duplicate DOM ids for those anchors. New pattern:
   "anchor-card-title-<categoryId>-<anchorId>".
@rdmueller rdmueller merged commit 4b39519 into LLM-Coding:main Apr 14, 2026
7 checks passed
raifdmueller added a commit to raifdmueller/Semantic-Anchors that referenced this pull request Apr 14, 2026
Two follow-ups to LLM-Coding#433 against the persistent CLS 0.975 issue.

## 1. Logo: max-h-24 → h-24 (explicit height)

Lighthouse continued to attribute the layout shift to the header logo
even after LLM-Coding#431 added width="218" height="96" attributes. Root cause:
Tailwind's preflight resets all images to `height: auto`, which
overrides the HTML height attribute. `max-h-24` only caps the height,
it doesn't enforce one — so before the image loads, the rendered
height is `auto` (= 0). When the PNG actually loads, the header grows
by 96px and pushes everything below.

`h-24` (height: 6rem) explicitly sets the height in CSS, overriding
preflight's `auto`. Combined with `w-[218px]` (and the matching
mobile sizes), the browser reserves the exact box from first paint.

## 2. Shell-header / shell-footer dimensions match the rendered ones

The static skeleton in LLM-Coding#433 used min-height: 10.5rem for the header
placeholder, but the real <header> renders at ~8.75rem (140px:
py-3 + h-24 logo + slogan + border). When hydrateShell() replaced
the placeholder, the header shrank by ~28px and #page-content moved
up by the same amount — a small but real shift on top of every other
load contribution.

Tighten the placeholders to match: header 8.75rem, footer 6rem (already
correct), and update #page-content min-height accordingly.

These are smaller contributions than the cards-loading shift, but they
clean up the easily-fixable signal first so the next Lighthouse run
isolates the remaining CLS to the actual hard problem.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants