Skip to content

site: regenerate from static-tool-page-template#24

Open
plx wants to merge 5 commits into
mainfrom
plx/create-pr
Open

site: regenerate from static-tool-page-template#24
plx wants to merge 5 commits into
mainfrom
plx/create-pr

Conversation

@plx
Copy link
Copy Markdown
Owner

@plx plx commented May 19, 2026

Summary

  • Rebuilds the Astro/Starlight site on the static-tool-page-template baseline: project metadata is centralized in src/site.config.mjs, the per-module *.components.css files are replaced by a generated theme.css plus a rewritten landing.css, and docs pages move from .md to .mdx.
  • Adds site tooling: Playwright + axe tests, cspell, Prettier, tsconfig.json, justfile, env.d.ts, favicon/tool-mark assets, and a stock site README.md.
  • Splits the single pages.yml workflow into a site-check.yml PR validator (format, spell, typecheck, build, Playwright) and a site-publish.yml deploy workflow, both scoped to site/** paths.
  • Refreshes AGENTIC_NAVIGATION_GUIDE.md to match the new site/ tree.

Test plan

  • site-check workflow passes on the PR
  • site-publish deploys cleanly on merge to main

🤖 Generated with Claude Code

Rebuilds the Astro/Starlight site on the static-tool-page-template baseline:
centralizes project metadata in src/site.config.mjs, replaces the per-module
component CSS files with a generated theme.css plus rewritten landing.css,
converts docs pages from .md to .mdx, and adds Playwright/axe tests, cspell,
Prettier, and tsconfig. Splits the single pages.yml workflow into a site-check
PR validator and a site-publish deploy workflow scoped to site/** paths, and
updates AGENTIC_NAVIGATION_GUIDE.md to match the new tree.

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

claude Bot commented May 19, 2026

Code Review

Overview

This PR rebuilds the site/ tree from a static-tool-page-template baseline. The structural wins are real: project metadata is centralized in site.config.mjs, the old multi-file design-system CSS is replaced by a flatter theme.css + landing.css, docs pages gain .mdx extensions, and a new CI pair (site-check.yml / site-publish.yml) replaces the monolithic pages.yml. The addition of Playwright+axe tests, cspell, Prettier, and a justfile rounds out the tooling story. Overall a solid regeneration. A few things worth addressing before merging:


Issues

Security — action pins regressed to floating tags

The deleted pages.yml used immutable SHA pins for every third-party action:

uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5  # v4.3.1

Both new workflows use floating version tags:

uses: actions/checkout@v5
uses: actions/setup-node@v5

Floating tags can be moved by an account compromise or a force-push. Consider pinning to SHAs as the old workflow did — or at minimum add a comment explaining the deliberate policy change (e.g. Dependabot will keep them updated).


Performance — npm cache dropped from CI

The old pages.yml had:

- uses: actions/setup-node@...
  with:
    node-version: 22
    cache: npm
    cache-dependency-path: site/package-lock.json

Neither site-check.yml nor site-publish.yml include cache: npm. With the new devDependencies (Playwright, axe, cspell, Prettier, TypeScript) the install step will be noticeably heavier on every run. Adding the cache key back is a one-liner.


Maintainability — basePath duplicated across three files

/agentic-navigation-guide is defined independently in three places:

File Line
site/src/site.config.mjs "basePath": "/agentic-navigation-guide"
site/playwright.config.ts const basePath: string = "/agentic-navigation-guide"
site/tests/site.spec.ts const basePath: string = "/agentic-navigation-guide"

If the deploy path changes, all three need to be updated in sync. playwright.config.ts and site.spec.ts could import from site.config.mjs:

import { siteConfig } from "../src/site.config.mjs";
const basePath = siteConfig.site.basePath;

The same applies to projectTitle and projectDescription in site.spec.ts — they are copied verbatim from siteConfig.project.*.


Minor observations

  • docs.sidebardocs.pages data duplication in site.config.mjs: these two arrays overlap on title/slug for the same four pages. A future addition would require touching both. Not a blocker, but worth a note in a comment that they must stay in sync.

  • package.json semver floor lowered: @astrojs/starlight drops from ^0.39.2^0.39.0 and astro from ^6.3.1^6.0.0. The lock file pins actual resolved versions so this won't cause a runtime change today, but it's a wider allowable window. Likely intentional for template compatibility.

  • Missing astro script alias: the old package.json had "astro": "astro" as a shorthand; the new one doesn't. Minor, but npx astro ... still works.


What's well done

  • Accessibility improvements: skip-to-content link, aria-controls="mobile-nav", aria-expanded toggling, and Escape-key close for mobile nav are all implemented correctly.
  • The landing.ts copy-button handler correctly clears the reset timer before starting a new one, avoiding race conditions.
  • tsconfig.json with astro/tsconfigs/strict + checkJs: true gives good coverage of the .mjs config file.
  • The Playwright test suite is thorough: viewport overflow, link validity (including hash-target existence), and axe audits on key pages.
  • The site-check.yml / site-publish.yml split is a clean separation of concerns.

🤖 Generated with Claude Code

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 995abe8d40

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread site/package.json Outdated
"typescript": "^6.0.0"
},
"engines": {
"node": ">=22"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Raise the advertised Node floor to 22.12

For local or engine-strict installs on Node 22.0–22.11, this package now advertises support even though the committed lock resolves astro@6.3.1, whose own engines entry requires node >=22.12.0; those environments can fail npm ci/build despite satisfying this top-level constraint. Please align this to >=22.12.0 so contributors and tooling reject unsupported Node versions up front.

Useful? React with 👍 / 👎.

Apply the local half of plx/static-tool-page-template#3 ahead of that
PR landing:

- SHA-pin every third-party action in site-check.yml and
  site-publish.yml; restores the security posture the old pages.yml
  workflow had before regeneration.
- Add `cache: npm` + `cache-dependency-path: site/package-lock.json`
  to setup-node so the new devDependencies (Playwright, axe, cspell,
  Prettier, TypeScript) reuse cached installs.
- Have site/playwright.config.ts and site/tests/site.spec.ts import
  basePath, projectTitle, projectDescription, and docsPages from
  src/site.config.mjs instead of duplicating them. Eliminates three
  places where the deploy path could drift.
- Add an "astro": "astro" script alias to site/package.json.

After plx/static-tool-page-template#3 lands and we re-render, this
patch becomes a no-op.

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

claude Bot commented May 20, 2026

Code Review

Overview

This PR regenerates the Astro/Starlight site from a static-tool-page-template baseline. The main changes are:

  • Project metadata centralized in src/site.config.mjs (consumed by Astro config, Playwright config, and the test suite)
  • CSS refactored: per-module *.components.css files → a leaner theme.css + rewritten landing.css
  • Docs pages renamed from .md to .mdx
  • New tooling: Playwright + axe-core, cspell, Prettier, tsconfig.json, justfile
  • Workflows split: pages.ymlsite-check.yml (PR validator) + site-publish.yml (deploy-only)
  • AGENTIC_NAVIGATION_GUIDE.md updated to reflect the new site/ tree

The overall direction is good — separating CI concerns, adding accessibility testing, and centralizing site config are all meaningful improvements.


Issues

Dead branch in both CI workflows

In site-check.yml and site-publish.yml, the install step is:

run: |
  if [ -f package-lock.json ]; then
    npm ci
  else
    npm install
  fi

package-lock.json is committed to the repo, so the else branch is unreachable. Just use npm ci directly. The conditional adds noise without benefit.

Missing prettier-plugin-astro

prettier is listed as a dev dependency, and npm run format:check runs prettier --check .. Without prettier-plugin-astro, Prettier has no built-in parser for .astro files and will silently skip them (or warn, depending on version). This means .astro formatting is not actually enforced in CI. Either add prettier-plugin-astro as a dev dependency with a matching .prettierrc config, or explicitly scope the format check to non-Astro files.

Dependency version ranges downgraded

"@astrojs/starlight": "^0.39.0"  // was ^0.39.2
"astro": "^6.0.0"                // was ^6.3.1

The lock file pins the actual versions, so this won't matter day-to-day, but the declared ranges now allow older patch releases that the project apparently wasn't using before. If the downgrade is intentional to match the template baseline, a comment explaining the decision would help.


Minor Notes

site.config.mjs "Generated" comment is slightly misleading

// Generated by static-tool-page-template. Edit this file and docs content before changing
// stock landing page components.

The file is both generated and the canonical source of truth going forward. The comment direction ("edit this file … before changing stock components") is correct, but "Generated" implies it can be regenerated and overwritten. Consider rewording to make clear this file owns project configuration.

typescript: ^6.0.0 — TypeScript 6 is the cutting edge. If any transitive tooling has stricter TS4/5 compatibility requirements this range is fine (npm resolves semver), but it's worth confirming the @astrojs/check peer dep is happy with TS6 in practice.

Playwright audit scope (pagesToAudit) only runs axe on the landing page and first docs page. That's a reasonable starting point, but noting it's not exhaustive.


What's Good

  • Splitting pages.yml into separate check and publish workflows is the right separation of concerns. The check workflow correctly scopes to pull_request triggers and the publish workflow is deploy-only.
  • siteConfig as the single source of truth shared across astro.config.mjs, playwright.config.ts, and site.spec.ts is clean — no config drift possible between the test runner and the actual site.
  • The Playwright test suite is thorough: viewport overflow checks, accessible mobile nav state (ARIA attributes + keyboard Escape), link validation (status + hash anchor existence), and axe accessibility audits across three device classes.
  • SITE_TEST_PORT escape hatch in the Playwright config is a nice DX touch.
  • The accessibility additions (skip-link, sr-only, proper aria-controls/aria-expanded on the mobile toggle) are well-implemented.
  • The navigation guide (AGENTIC_NAVIGATION_GUIDE.md) is correctly updated in the same PR — consistent with the project's own self-validation principle.
  • Action SHAs are pinned with version comments throughout — good supply-chain hygiene.

Astro 6.3.1 (the locked version) requires node >=22.12.0, so advertising
>=22 here let engine-strict installs on Node 22.0–22.11 pass the
top-level check then fail later during npm ci/build.

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

claude Bot commented May 23, 2026

Code Review: site — regenerate from static-tool-page-template

Overall this is a clean, well-structured rebuild. The split of a single workflow into validation vs. deploy, the centralized site.config.mjs, and the addition of Playwright/axe-core tests are all solid improvements. A few issues worth addressing before merge:


Bugs / Potential Issues

1. box-sizing reset drops pseudo-element coverage (landing.css)**

-*,
-*::before,
-*::after {
+* {
   box-sizing: border-box;

Removing ::before and ::after from the selector is a regression: pseudo-elements used for decorative bullets, overlays, or icon content will silently revert to content-box sizing. The three-selector form is the CSS-reset standard for a reason. Easy fix.

2. overflow-x: visible on pre in Starlight context (starlight.css)

.sl-markdown-content pre {
  overflow-x: visible;
  overflow-wrap: anywhere;
  white-space: pre-wrap;
}

Per the CSS spec, setting overflow-x: visible while overflow-y is not visible (which it won't be once Starlight sets it) causes the computed value to be promoted to auto. The intent appears to be "wrap long lines instead of scrolling" — white-space: pre-wrap + overflow-wrap: anywhere already achieves that. Drop overflow-x: visible, which does nothing helpful here and can cause confusion.

3. External link label check misses aria-label (tests/site.spec.ts, line ~106)

if (!link.label) {
  failures.push(`${pagePath}: external link ${link.href} has no text label`);
}

link.label is populated via anchor.textContent?.trim(), which is empty for icon-only links that use only aria-label (e.g., the GitHub icon link in the Starlight sidebar). This would either produce false failures or silently pass depending on which external links appear on the page. Consider also checking anchor.getAttribute("aria-label") before flagging the failure.


Minor / Style

4. npm ci fallback in CI is dead code (site-check.yml, site-publish.yml)

if [ -f package-lock.json ]; then npm ci; else npm install; fi

package-lock.json is committed, so the else branch never runs in CI. Just use npm ci directly — it's clearer and faster.

5. Redundant site-check run on push to main

site-check.yml triggers on both PRs and push to main. When a PR merges, both site-check (build + validate) and site-publish (build + deploy) run, building the site twice. Since site-publish only deploys when the build succeeds, the extra site-check run on main push doesn't add safety — it just burns CI minutes. Consider removing the push trigger from site-check.yml and relying on the PR run for validation.

6. actions/checkout/setup-node version comments (site-check.yml, site-publish.yml)

Both new workflows pin to commit SHAs labeled # v5.0.1 (checkout) and # v5.0.0 (setup-node). Worth a quick verify that the SHAs actually correspond to those tags — the old pages.yml was on checkout v4.3.1 and setup-node v4.4.0, and a jump to v5 would be a noteworthy major bump.

7. pagesToAudit only covers 2 pages (tests/site.spec.ts, line ~18)

const pagesToAudit = ["/", docsPages[0]?.href].filter(Boolean);

Accessibility violations on the remaining 3 docs pages won't be caught. Even at the cost of a few extra seconds, running axe on all pagesToCheck would give much better coverage since docs pages can include different components (code blocks, tables, etc.).

8. package.json dependency range widened

-"@astrojs/starlight": "^0.39.2",
-"astro": "^6.3.1"
+"@astrojs/starlight": "^0.39.0",
+"astro": "^6.0.0"

The lock file pins the actual installed versions, so this has no runtime impact now. But the range is wider than the version the code was developed against, which could surprise someone doing a fresh install from the manifest without a lock file. Consider keeping the ranges at the versions actually used.


Positives Worth Calling Out

  • Workflow separation into site-check (validate) + site-publish (deploy) is clean and follows the recommended GitHub Pages pattern.
  • Centralized site.config.mjs — having one source of truth for title, description, nav, sidebar, and theme tokens is a big ergonomic improvement.
  • Multi-viewport Playwright projects (mobile/tablet/desktop) with correct isMobile and deviceScaleFactor settings.
  • axe-core integration is the right call for a developer tooling site.
  • landing.ts extracted from inline <script> into a typed module — much easier to maintain and test.
  • skip-link added to the landing page — was missing before, now present and keyboard-accessible.
  • SVG assets use role="img" + aria-label (tool-mark) and alt="" (img tag) correctly.
  • AGENTIC_NAVIGATION_GUIDE.md kept in sync — consistent with project conventions.

🤖 Generated with Claude Code

…g comment)

- Drop the unreachable `if [ -f package-lock.json ]` branch in both site
  workflows; the lock file is committed, so just `npm ci`.
- Add prettier-plugin-astro as a devDep and a `.prettierrc` that registers
  the plugin and the `.astro` parser override. Format the existing Astro
  landing page accordingly; without this, `format:check` was silently
  skipping `.astro` files.
- Bump declared dependency floors to `@astrojs/starlight ^0.39.2` and
  `astro ^6.3.1` to match the lockfile (the prior `^0.39.0` / `^6.0.0`
  ranges allowed older patch releases the project already moved past).
- Re-word the `site.config.mjs` leading comment so it's clear this file
  is the canonical project configuration, not a regenerable artifact.
- Track the new `.prettierrc` in the navigation guide.

Upstream template fixes filed at
plx/static-tool-page-template#5.

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

claude Bot commented May 23, 2026

Code Review — PR #24: site: regenerate from static-tool-page-template

Overview

This PR rebuilds the Astro/Starlight site from a static-tool-page-template baseline. Key structural changes:

  • Centralizes all project/site data in src/site.config.mjs (single source of truth)
  • Replaces the scattered *.components.css files with a generated theme.css + rewritten landing.css
  • Renames doc pages from .md.mdx
  • Adds a full dev-tooling layer: Playwright + axe tests, cspell, Prettier, tsconfig, justfile
  • Splits the monolithic pages.yml into site-check.yml (PR validation) and site-publish.yml (deploy)
  • Updates AGENTIC_NAVIGATION_GUIDE.md to reflect the new site/ tree

The overall direction is solid. The config centralization, workflow split, and Playwright suite are all clear improvements over the prior state.


Potential Issues

1. Escape key: focus not returned to toggle after mobile nav closes (site/src/scripts/landing.ts, lines 50–54)

document.addEventListener("keydown", (event) => {
  if (event.key === "Escape") {
    setNavOpen(false);  // closes panel but does not move focus
  }
});

When a keyboard user presses Escape to dismiss the nav panel, focus stays wherever it was inside the panel (likely on a nav link), which can disorient screen-reader users. The toggle button is the logical focus target after dismiss. Adding navToggle?.focus() inside that branch would fix this:

document.addEventListener("keydown", (event) => {
  if (event.key === "Escape") {
    setNavOpen(false);
    navToggle?.focus();
  }
});

The Playwright test for this flow (site.spec.ts line 90: await page.keyboard.press("Escape")) checks aria-expanded but doesn't assert focus placement, so this slips through.


2. CI Playwright tests run against the dev server, not the production build (site/playwright.config.ts, line 28)

webServer: {
  command: `npm run dev -- --host 127.0.0.1 --port ${testPort}`,

In site-check.yml, the workflow runs npm run build and then npm run test. The build step validates the production build succeeds, but Playwright spins up its own dev server and tests against that. Built assets in dist/ go unused. The tests therefore don't catch production-only regressions (e.g. base-path issues, inlined assets, treeshaking side effects).

Consider switching the webServer.command to build-then-preview, or running two separate steps:

webServer: {
  command: `npm run preview -- --host 127.0.0.1 --port ${testPort}`,
  // requires 'npm run build' to have run first

3. docs.sidebar and docs.pages duplicate the same four page records (site/src/site.config.mjs, lines 4068–4117)

Both arrays describe the same four pages with largely the same fields (label/title, slug, href). They can silently diverge when a new docs page is added. At minimum a code comment pointing from one to the other would help; at best, a single array (perhaps docs.pages) could be the source of truth and the sidebar items derived from it in astro.config.mjs.


4. actions/configure-pages was silently dropped from the publish workflow

The old pages.yml included actions/configure-pages before uploading the artifact. The new site-publish.yml omits it. This is most likely fine — configure-pages mainly sets the BASE_URL env variable, which the new setup handles via site.config.mjs — but worth a note confirming that GitHub Pages is already configured in the repo settings, since without configure-pages there's no automatic environment bootstrap.


Minor Notes

  • pagesToAudit is limited to two pages (site/tests/site.spec.ts, line 13): the axe accessibility check only covers the landing page and docsPages[0]. The other three docs pages are checked for viewport overflow and link health but not audited for a11y violations. This is a reasonable tradeoff for CI speed, but might miss violations on the guide-format or commands pages specifically.

  • prettier-ignore on the entire siteConfig object: The comment suppresses formatting for ~170 lines. The reason is presumably to preserve the consistent JSON-style quoting, which is fine — just unusual to see it applied at object scope rather than to individual lines.

  • Workflow action versions: checkout bumped from v4.3.1v5.0.1 and setup-node from v4.4.0v5.0.0. Both are pinned to specific SHAs with version comments, consistent with the stated practice in the docs (ci.mdx). The deploy-pages SHA (d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e) is the same as in the old workflow (still v4.0.5), which is fine.

  • tsconfig.json enables checkJs: Good call — this catches type errors in site.config.mjs (a .mjs file imported from TypeScript) that would otherwise be silent.


Summary

The structural changes are well thought out. The site.config.mjs centralization is the biggest quality-of-life win here, and the Playwright suite adds meaningful coverage that was missing entirely. The focus-management gap on Escape (#1) is the most actionable issue; the dev-vs-production test server gap (#2) is a lower-priority improvement. Everything else is minor polish.

- landing.css: restore `::before`/`::after` in the box-sizing reset so
  pseudo-elements (e.g. `.command-card::before` overlay) inherit
  border-box sizing.
- starlight.css: drop `overflow-x: visible` on `.sl-markdown-content
  pre`; `white-space: pre-wrap` + `overflow-wrap: anywhere` already
  wrap long lines, and `visible` would get promoted to `auto` once
  Starlight sets `overflow-y` to anything else.
- site.spec.ts: read `aria-label` alongside `textContent` for external
  links and only flag links missing both. Rename failure message to
  "no accessible label".
- site.spec.ts: expand `pagesToAudit` to cover every page in
  `pagesToCheck`, so axe runs against the docs pages too (27 tests vs
  18 before).
- site-check.yml: drop the redundant `push: branches: [main]` trigger;
  `site-publish.yml` already validates the build on merge.

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

claude Bot commented May 23, 2026

Code Review: site: regenerate from static-tool-page-template

Overview

This PR rebuilds the Astro/Starlight site on a new template baseline, centralises all project metadata in src/site.config.mjs, splits the monolithic pages.yml workflow into separate check and publish workflows, adds a full Playwright + axe test suite, and updates AGENTIC_NAVIGATION_GUIDE.md to match the new tree. The scope is entirely within site/ and CI configuration — no Rust source changes.


What Works Well

  • Pinned action SHAs with version comments — both new workflows use commit-pinned actions (e.g. actions/checkout@93cb6efe… # v5.0.1). This is the correct security posture for supply-chain safety.
  • Centralized site.config.mjs — pulling all hardcoded strings (repo URL, basePath, sidebar, landing copy) into a single file eliminates the config-drift that made the old astro.config.mjs fragile. The config is consumed by both astro.config.mjs and the Playwright tests, so a single edit propagates everywhere.
  • Workflow splitsite-check.yml (PR gate) and site-publish.yml (deploy on merge) are cleaner than the old single file that conditionally skipped deploy steps on PRs.
  • Accessibility-first test suitesite.spec.ts tests landmarks, heading hierarchy, viewport overflow, mobile nav aria state, link validity, and axe audits. The isSkippableHref helper and external-link label checks are particularly thoughtful.
  • trailingSlash: "always" in astro.config.mjs — prevents double-redirect issues with GitHub Pages.
  • Strict TypeScripttsconfig.json extends astro/tsconfigs/strict with checkJs: true, so the .mjs config file is also type-checked.
  • AGENTIC_NAVIGATION_GUIDE.md kept in sync — updating the navigation guide to match the new site tree is exactly the right practice for a tool that self-verifies its own guide.

Issues and Suggestions

1. Playwright browser cache not configured in CI (performance)

site-check.yml runs npx playwright install --with-deps chromium on every run, downloading ~300 MB of Chromium each time. Consider caching ~/.cache/ms-playwright:

- name: Cache Playwright browsers
  uses: actions/cache@...
  with:
    path: ~/.cache/ms-playwright
    key: playwright-${{ runner.os }}-${{ hashFiles('site/package-lock.json') }}

- name: Install Playwright browsers
  run: npx playwright install --with-deps chromium

2. pagesToAudit is unused (dead code)

In site/tests/site.spec.ts line 19:

const pagesToAudit = pagesToCheck;

This variable is assigned but not referenced anywhere in the visible test file. If the axe audit loop was intended to use a distinct page list, the separate constant makes sense — but as written it adds confusion. Either use it or remove it.

3. siteConfig.docs.pages[0]?.href is order-dependent

In site/src/pages/index.astro:

const docsHref = siteConfig.docs.pages[0]?.href || "docs/";

This silently breaks if the pages array is reordered. Since site.config.mjs already has named sidebar slugs, consider adding a named "overview" or "entry" key to the docs config, or referencing by slug lookup rather than index 0.

4. Mobile nav close does not return focus (minor a11y)

In site/src/scripts/landing.ts, when the nav is closed via Escape key or a nav link click, focus stays wherever it landed rather than returning to the toggle button. WCAG 2.1 SC 2.1.2 and common modal/disclosure patterns expect focus to return to the opener:

document.addEventListener("keydown", (event) => {
  if (event.key === "Escape") {
    setNavOpen(false);
    navToggle?.focus(); // return focus to opener
  }
});

5. "and" in cspell.json custom words list

"words": ["and", ...]

"and" is a standard English word — it will never be flagged by any dictionary. This entry is harmless but is likely a copy-paste artifact from the template. It can be removed.

6. site-check.yml has no branches filter

site-publish.yml scopes to branches: ["main"] on push, but site-check.yml has no branch filter on pull_request. This means it runs on PRs targeting any branch. For a single-branch workflow this is fine now, but worth noting if feature branches with site-path PRs are expected.

7. render alias in justfile duplicates build

render:
    npm run build

render and build do the same thing. Removing render (or adding a comment explaining why both exist) would prevent confusion for contributors.


Summary

This is a clean, well-structured site rebuild. The centralized config, proper action pinning, and accessibility-first test suite are all strong. The main actionable items are the Playwright browser caching (CI cost), the unused pagesToAudit variable, and the focus-return gap in mobile nav. Everything else is polish.

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.

1 participant