Skip to content

fix(website/a11y): contrast tokens, focus-visible, tap targets, theme toggle, search combobox - #707

Merged
ajitpratap0 merged 4 commits into
mainfrom
fix/a11y-contrast-focus
Oct 14, 2026
Merged

ajitpratap0 merged 4 commits into
mainfrom
fix/a11y-contrast-focus

Conversation

@ajitpratap0

Copy link
Copy Markdown
Owner

Summary

  • Muted text token raised to >=4.5:1 in both themes (dark #8B8BA0, light #5B6472); informational text-zinc-500/600 moved to text-text-muted.
  • Global :focus-visible ring; icon buttons and search trigger min-h-11 min-w-11; footer links py-2.
  • ThemeToggle: stable label "Dark mode" + aria-pressed + data-theme-toggle; light-theme CSS rewritten to use data attributes (search trigger, repo link, toggle) instead of aria-label selectors.
  • SearchModal: combobox pattern with aria-activedescendant, portal + inert background, focus restored to trigger on close. Navbar logo alt="".
  • Removed dead SocialProof.tsx (StatsBar.tsx and FadeInCSS.tsx were already gone on main). CHANGELOG.md untouched.

Test Plan

  • npm ci && npm run typecheck && npm run lint && npm run build pass (lint: warnings only, pre-existing).
  • Lighthouse CI runs on this PR; manual keyboard check of nav, toggle and search.

Breaking Changes

None.

Closes #655
Part of #634

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

… toggle, search combobox

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@vercel

vercel Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
gosqlx-website Ready Ready Preview Oct 14, 2026 6:43pm UTC

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 20s —— View job


PR review: a11y contrast, focus-visible, tap targets, theme toggle, search combobox

Overall this is a solid, focused a11y pass. I read the diff for globals.css, Navbar, SearchModal and ThemeToggle closely. I skimmed the mechanical text-zinc-500/600 → text-text-muted swaps. I did not run the build or Lighthouse.

Issues worth fixing

  1. SearchModal: portal runs on the first client render, which risks a hydration mismatch (SearchModal.tsx, if (typeof document === 'undefined') return null;).

    • On the server this returns null. On the client's first render it returns a portal.
    • For a 'use client' component that SSRs, that can trigger a hydration mismatch warning.
    • Use a mounted state set in useEffect, or render the portal only when open.
  2. SearchModal: focus is restored to the wrong element when the modal opens.

    • The triggerRef effect reads document.activeElement when open flips. The existing effect that focuses the input may already have run, depending on effect order. In that case you'd save the input as the "trigger" and restore focus to a node that is gone.
    • Capture the trigger before the input focus happens. Put the capture effect first, or capture it in the open handler.
    • The Cmd+K path has the same exposure.
  3. Inert handling can leave the page inert.

    • !el.hasAttribute('inert') skips elements that are already inert, which is right. But if two modals or effects overlap, the cleanup of one can remove inert that another set.
    • This is an edge case. A counter or a single owner would make it safer.
  4. Combobox ARIA details (SearchModal.tsx).

    • aria-expanded="true" is always set, even with no results. Use results.length > 0.
    • The option id search-option-${slug} assumes slugs are valid and unique. If a slug contains spaces or /, the id is invalid for aria-activedescendant. Sanitize it or use the index.
    • The "No results" <li> sits inside a role="listbox" and has no role="option". Move it outside the listbox or give it role="presentation".
  5. ThemeToggle: the aria-label never changes, so the CSS state can drift.

    • A stable label plus aria-pressed is the right pattern. But the label "Dark mode" is shown in the SSR/placeholder branch with aria-pressed="true" hard-coded.
    • A light-theme user will briefly get an incorrect pressed state on hydration. This is minor, but the placeholder could read the current html.light class.

Minor

  • outline: 2px solid var(--color-link) on :focus-visible is global. Inputs that use outline-none (e.g. the search input) lose their ring because that utility sits in a layer, while this rule is unlayered. You said "unlayered so it wins", but check that the search input still gets a visible focus indicator. It may or may not be intended for the combobox.
  • Light-theme [data-search-trigger] colour #6B7280 on a near-white background is about 4.8:1, which passes. The kbd colour #9CA3AF is about 2.5:1 and fails, and it is the same value you just replaced for the muted token. Consider #5B6472 there.
  • Dark #8B8BA0 on #0A0A0F-like backgrounds should pass. Confirm against --color-bg-elevated and hover surfaces too.
  • data-search-trigger="" and data-link-repo="" are fine, though data-search-trigger alone is enough.
  • Deleting SocialProof.tsx is fine if nothing imports it. Run grep -r SocialProof website/src to confirm, though the typecheck in your test plan should catch it.

Test coverage

There are no automated a11y tests. Consider a Playwright/axe check on the home and docs pages, plus a keyboard test for open search → arrow → Enter → focus restore. Lighthouse CI will catch contrast but not the combobox behaviour.

Nothing here is blocking. Items 1 and 2 are the ones I'd fix before merge.
· branch fix/a11y-contrast-focus

…ch targets

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@ajitpratap0

Copy link
Copy Markdown
Owner Author

Addressed review feedback:

  • Light-mode muted text: @theme inline bakes the value into utilities, so added html.light .text-text-muted, html.light .placeholder\:text-text-muted::placeholder { color: #5B6472 } in globals.css (verified present in built CSS; #5B6472 on white is about 5.9:1, passes AA). Not run through a live axe session; typecheck, lint and build pass.
  • Touch targets: footer links now inline-flex min-h-11 items-center, mobile nav links flex min-h-11 items-center, mobile Get Started button min-h-11.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 29s —— View job


Review of PR #707 (a11y: contrast, focus-visible, tap targets, theme toggle, search combobox)

I read the diff for globals.css, Navbar, SearchModal and ThemeToggle. I did not run the build or Lighthouse. This is a solid, focused a11y pass. Nothing blocks the merge. I also checked the earlier automated review's two "fix before merge" items and found that neither one applies.

Earlier review items that don't apply

  • Hydration mismatch from the portal. SearchModal is loaded with next/dynamic and only rendered once searchLoaded is true (Navbar.tsx:14 and Navbar.tsx:211). That only happens after a client interaction, so the modal is never server-rendered. The typeof document === 'undefined' guard is harmless.
  • Focus restore capturing the input. The input is focused inside requestAnimationFrame (SearchModal.tsx ~L52). The triggerRef capture runs synchronously in the effect before that frame, so it records the real trigger. The Cmd+K path is the same.
  • Search input losing its focus ring. The global :focus-visible rule is unlayered, so it beats the layered outline-none utility. The ring does show on the input. That matches your intent, though a ring on a text input inside a bordered dialog may look heavy.

Worth fixing (small)

  1. aria-expanded="true" is hardcoded on the combobox (SearchModal.tsx). Use results.length > 0, or keep true and always keep the listbox mounted.
  2. Option ids built from slugs. search-option-${entry.slug} is not a valid id if a slug contains / or spaces, and doc slugs may be nested. aria-activedescendant needs a valid, unique id. Using the index (search-option-${i}) is safest.
  3. The "No results" <li> sits directly inside role="listbox" and has no role="option". That is an ARIA structure violation, which axe flags. Add role="presentation" or move it outside the listbox. Consider role="status" or aria-live="polite" so the empty state is announced.
  4. Light-theme [data-search-trigger] kbd colour is still #9CA3AF (globals.css). That is about 2.5:1 on a near-white background, the same value you just replaced for the muted token. Use #5B6472 here.

Minor

  • Inert handling: inert is set on every document.body child except the portal, and removed in cleanup. That is fine for a single modal. If a second inert owner ever appears, the cleanup could clear its attribute. A counter would be more robust, and none is needed today.
  • ThemeToggle placeholder: the SSR/placeholder button hardcodes aria-pressed="true". A light-theme user gets a wrong pressed state until hydration. Reading the html.light class on mount would fix it. It is brief and low impact.
  • Redundant data attributes: data-search-trigger="" and data-link-repo="" work as written, but data-search-trigger is the only one the search styling needs.
  • Contrast check: please confirm dark #8B8BA0 against --color-bg-elevated and the hover surfaces, not just the base background.
  • Cleanup: SocialProof is no longer referenced anywhere in website/src, so removing it is safe.

Tests
There are no automated a11y or keyboard tests. A Playwright and axe check on the home and docs pages would help. Another would cover opening search, pressing an arrow key, pressing Enter, and checking that focus returns to the trigger. Lighthouse CI will catch contrast but not the combobox behaviour.
· branch fix/a11y-contrast-focus

@ajitpratap0

Copy link
Copy Markdown
Owner Author

Review fixes pushed:

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 21s —— View job


Review of PR #707 (a11y pass)

I read SearchModal.tsx, ThemeToggle.tsx and the light-theme rules in globals.css. I did not run the build, axe or Lighthouse. Nothing blocks the merge, but three small ARIA fixes in SearchModal.tsx are worth making first. I also checked the two earlier automated reviews and found some of their points stale.

Fix before merge (all in SearchModal.tsx)

  1. aria-expanded is hardcoded to "true" (L175). Use aria-expanded={results.length > 0}, or leave it, since the listbox is always mounted while the modal is open. The first option is cheap and more accurate.
  2. Option ids come from slugs (L120 and L209, search-option-${entry.slug}). Doc slugs can be nested and contain /. aria-activedescendant needs a valid, unique id. Use the index instead: search-option-${i}.
  3. The "No results" <li> (L201) sits directly inside role="listbox" with no role="option". axe flags this as an ARIA structure violation. Move it outside the <ul>, or give it role="presentation". Adding aria-live="polite" or role="status" would also announce the empty state.

Earlier review points that don't hold

  • Hydration mismatch from the portal. The earlier review said SearchModal is loaded via next/dynamic and only rendered after interaction. I did not verify that in Navbar.tsx. If so, the typeof document guard is harmless.
  • Focus restore capturing the input. The trigger is captured synchronously in the effect (L60). The input is focused in requestAnimationFrame (L52), so it runs later and the trigger is recorded correctly.
  • kbd contrast in the light theme. html.light [data-search-trigger] kbd is already #5B6472 (globals.css L254-258), so the earlier "still #9CA3AF" note is out of date.
  • Search input losing its focus ring. The global :focus-visible rule is unlayered and beats the layered outline-none. The ring does show, which seems intended.

Minor, non-blocking

  • ThemeToggle placeholder (L63). It hardcodes aria-pressed="true" before mount, so a light-theme user gets a wrong pressed state until hydration. Read document.documentElement.classList.contains('light') for the initial state, or omit aria-pressed on the placeholder.
  • Result item text (L215). text-zinc-400 on unselected options is about 5.5:1 on dark, which passes. It is the one informational colour not moved to text-text-muted. Consider switching it for consistency.
  • Inert cleanup (L63-73). Fine for one modal. If another component ever sets inert, a shared counter would be safer.
  • Contrast. Please confirm #8B8BA0 against --color-bg-elevated and the hover surfaces, not just the page background.

Tests
There are no automated a11y or keyboard tests. A Playwright and axe check on the home and docs pages would help. Another would cover opening search, pressing an arrow key, pressing Enter, and checking that focus returns to the trigger.
· branch fix/a11y-contrast-focus

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 26s —— View job


Review of PR #707 (a11y pass)

I read SearchModal.tsx and ThemeToggle.tsx in full. I did not read the CSS or the mechanical text-zinc-* → text-text-muted swaps line by line, and I did not run the build, axe or Lighthouse. The overall approach is sound: the stable toggle label with aria-pressed, the inert background and the focus restore are all correct patterns. The prior automated reviews repeated several points, and I checked those against the current code.

Fix before merge (all in SearchModal.tsx)

  1. Invalid option ids. search-option-${entry.slug} is used at L120 and L209. Doc slugs can contain /, which is not a safe id for aria-activedescendant. Use the index instead: search-option-${i}.
  2. aria-expanded="true" is hardcoded (L175). Use aria-expanded={results.length > 0}.
  3. The "No results" <li> is inside role="listbox" (L201) with no role="option". axe flags this as an ARIA structure violation. Render it outside the <ul> as <div role="status">, or at least give it role="presentation".
  4. NaN selection when results are empty. In handleKeyDown, (i + 1) % results.length and (i - 1 + results.length) % results.length both evaluate to NaN when results.length === 0, which happens when you type a no-match query and press an arrow key. selectedIndex becomes NaN until the next query change. This predates the PR, but it is a one-line guard (if (!results.length) break;) and it is on the same keyboard path you are fixing.

Prior review points that don't hold

  • Focus capture. The trigger is captured synchronously in the effect at L60. The input is focused later in requestAnimationFrame (L52), so the captured element is the real trigger.
  • Inert and focus order. The cleanup removes inert before calling trigger.focus() (L70-72). That ordering is correct, because focusing an inert element would fail.
  • Search input focus ring. The unlayered global :focus-visible rule beats the layered outline-none, so the ring shows. That seems intended.
  • html.light kbd colour. This was reported as still #9CA3AF, but your later push changed it.

Minor, non-blocking

  • ThemeToggle placeholder (L63). It hardcodes aria-pressed="true" before mount, so a light-theme user briefly gets the wrong state. Either omit aria-pressed on the placeholder or read document.documentElement.classList.contains('light') in the mounted branch. The placeholder also uses text-zinc-300 while the mounted button uses text-zinc-400, which is a small visual jump on hydration.
  • text-zinc-400 on result rows (L215). This is the one informational colour not moved to text-text-muted. Consider switching it for consistency.
  • Portal and SSR guard (L122). I did not verify the earlier claim that the modal is only rendered after interaction. Check that Navbar.tsx and DocsSearchTrigger.tsx mount it only on the client. If either renders it during SSR, you would need a mounted state.
  • Inert cleanup. Removing inert from elements this effect set is fine for one modal. A shared counter would only matter if another component starts using inert.
  • Contrast. Confirm #8B8BA0 against --color-bg-elevated and the hover surfaces, not just the page background.

Tests
There are no automated a11y or keyboard tests. A Playwright and axe check on the home and docs pages would help. A second test could open search, press an arrow key, press Enter, and check that focus returns to the trigger. Lighthouse CI will catch contrast but not the combobox behaviour.

Nothing here blocks the merge, but I'd fix items 1-3 first.
· branch fix/a11y-contrast-focus

@ajitpratap0
ajitpratap0 merged commit 7904ffc into main Oct 14, 2026
30 checks passed
@ajitpratap0
ajitpratap0 deleted the fix/a11y-contrast-focus branch October 14, 2026 18:52

This branch was successfully deployed

1 active deployment
Preview — 2887d554 Deployed Oct 14, 2026 by vercel[bot]
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.

fix(website/a11y): contrast tokens, focus-visible, tap targets, theme toggle, search combobox [1.15]

1 participant