Repository navigation
feat: landing consolidation phase 2 — consistency, assets, CI, and graceful degradation - #184
Conversation
…aceful degradation Phase 1 — Consistency Hardening: - Standardize 6 feature cards per template in shared/landings/features.ts - Fix version drift (Vite 8→Vite, React 19→React, Astro 7→Astro, etc.) - Switch all docs links to root-absolute /docs/ prefix - Create docs/ dirs for remix and astro starters (fix 404s) - Fix 'Start editing' CTA to non-navigable <span> in all templates - Replace all hardcoded CSS colors with --cna-* tokens - Add --cna-amber-soft light-mode override where missing - Add .cna-landing__card to stagger animation selectors - Add prefers-reduced-motion + light mode to webext popup - Reconcile DESIGN vs GUIDE docs (keyframe names, shadows, fill mode) - Remove dead files (next.svg, vercel.svg, Welcome.astro, stray page.tsx) Phase 2 — Asset Robustness: - Unique gradient IDs across all SVGs (no ID collisions) - Add role=img to favicon.svg, aria-hidden to wordmark text - Create logo-wordmark-dark.svg variant - Create scripts/sync-assets.js for idempotent asset distribution Phase 3 — Automated Verification: - Create scripts/landing-audit.js (static landing page validator) - Wire validate-templates.js into CI (validate-registry job) - Expand smoke-test to all 7 UI templates - Add npm cache, concurrency cancel, timeout-minutes to all CI jobs - Run test-combinations on PRs (not just push/main + cron) - Create Playwright visual regression harness + CI workflow - Update docs to reference CI automation Phase 4 — Graceful Degradation: - Fix react-tailwindcss PostCSS plugin (tailwindcss → @tailwindcss/postcss) - Add extension-isolation CI job testing each UI extension individually
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughIntroduces a shared landing-page feature/docs data module, applies consistent ChangesLanding Design System and CI Automation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant LandingVisualWorkflow
participant ScaffoldedTemplate
participant PlaywrightTests
PullRequest->>LandingVisualWorkflow: trigger on path-filtered PR or manual dispatch
LandingVisualWorkflow->>ScaffoldedTemplate: scaffold template and start dev server
LandingVisualWorkflow->>ScaffoldedTemplate: wait for server readiness on configured port
LandingVisualWorkflow->>PlaywrightTests: run tests with TEMPLATE and BASE_URL
PlaywrightTests-->>LandingVisualWorkflow: generate screenshots
LandingVisualWorkflow-->>PullRequest: upload screenshot artifacts
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
The actions/setup-node cache option with cache: npm requires a package-lock.json or yarn.lock at repo root. This repo scaffolds projects dynamically so there is no root lockfile. Remove the option from all three workflows (smoke-test, test-combinations, landing-visual). Also change landing-visual from npm ci to npm install since the test harness directory has no committed lockfile.
- Replace fragile npx wait-on with bash port-polling loop - Add step id for conditional execution of downstream steps - Handle scaffold failure gracefully (astro/remix pre-existing issues) - Always upload screenshots as artifacts for inspection
…oHaveScreenshot - Export PORT as a shell variable before using it in curl loop - Replace toHaveScreenshot with page.screenshot (baselines not yet committed) - Upload screenshots as artifacts for manual inspection
- Fix spec to only test the current matrix template instead of all 5 - Pass TEMPLATE env var to Playwright tests - Remove --host flag for Next.js (unsupported) - Make visual tests non-blocking with continue-on-error - Add condition for nextjs-starter to avoid --host flag
- Fix critical: logo-wordmark.svg references non-existent gradient url(#cnaNest) → url(#cnaNestWord) - Fix: landing-visual.yml health check now fails on timeout - Fix: landing-visual.yml uses matrix.port (DRY, no repeated expressions) - Fix: landing-visual.yml removes continue-on-error from test step - Fix: smoke-test.yml simplifies SKIP_ENV_VALIDATION (dead ternary) - Fix: landing-audit.js regex anchors to const docs only - Fix: landing-audit.js executable bit set - Fix: Next.js page.module.css adds flex-wrap to .button
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
templates/astro-starter/docs/PROJECT_STRUCTURE.md.template (1)
3-198: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftRewrite this guide for Astro, not Next.js.
Lines 3-198 describe Next.js App Router, route groups, and server components, but this file lives under
astro-starter/docs. That mismatch will mislead users of the Astro template. Please replace the examples with Astro-specific structure, or move the document to the Next.js starter if that was the intent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/astro-starter/docs/PROJECT_STRUCTURE.md.template` around lines 3 - 198, Rewrite the PROJECT_STRUCTURE guide to match Astro instead of Next.js: replace the App Router, route groups, server components, and Next.js-specific examples with Astro’s directory and routing conventions, using the existing template symbols like srcDir and projectImportPath only where they still apply. Update the architecture overview, directory tree, principles, and example sections so they describe Astro starter structure and imports, and ensure any references to features/shared remain accurate for this template. If the intent was actually Next.js, move this document to the Next.js starter instead of keeping it in the Astro docs.templates/remix-starter/docs/PROJECT_STRUCTURE.md.template (1)
3-198: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRewrite this page for Remix, not Next.js.
The whole document talks about Next.js App Router, route groups, and server components, which do not apply to Remix and will mislead users of this starter. Replace it with Remix route, loader/action, and layout conventions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/remix-starter/docs/PROJECT_STRUCTURE.md.template` around lines 3 - 198, The PROJECT_STRUCTURE document is written for Next.js App Router and should be rewritten to describe Remix conventions instead. Update the overview and directory examples in the template to use Remix route files, nested layouts, loaders/actions, and shared app structure, and remove Next.js-specific terms like route groups, server components, and App Router from the guidance. Keep the feature-based architecture framing, but align the examples and import guidance with Remix patterns and the symbols used in this template such as srcDir, features/auth, and shared.
🧹 Nitpick comments (6)
.github/workflows/test-combinations.yml (1)
7-16: 🚀 Performance & Scalability | 🔵 TrivialNew
pull_requesttrigger duplicates smoke-test.yml coverage per PR.Running
generate-combinations/test-combinations(full scaffold + install + lint + type-check + build per template) on every PR tomain, in addition tosmoke-test.yml's own per-template scaffold/build, meaningfully increases CI minutes per PR. Consider restricting this to paths that touchtemplates.json/extensions, or keeping it scheduled-only and relying onsmoke-test.ymlfor per-PR coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test-combinations.yml around lines 7 - 16, The new pull_request trigger in the test-combinations workflow duplicates the per-PR coverage already provided by smoke-test.yml, so remove or narrow that trigger in the workflow definition. Update the workflow configuration to run generate-combinations/test-combinations only on schedule or only when relevant template files change, and keep smoke-test.yml as the per-PR coverage path.scripts/landing-visual/playwright.config.ts (1)
17-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
firefoxproject is defined but never installed/run in CI.CI only installs and runs the
chromiumproject. Either wire firefox into the workflow or drop the unused project for now.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/landing-visual/playwright.config.ts` around lines 17 - 26, The Playwright config defines a firefox project that is never used by CI, so either remove the unused firefox entry from the projects array in the Playwright config or update the CI workflow to install and run it alongside chromium. Use the existing project definitions in the Playwright config as the place to make the change, and keep the project list aligned with what the pipeline actually executes..github/workflows/smoke-test.yml (1)
96-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent
SKIP_ENV_VALIDATIONscope between jobs.
smoke-testsetsSKIP_ENV_VALIDATION: 'true'for the whole "Run basic checks" step (Lines 96-97), butextension-isolation's equivalent step (Lines 142-151) only sets it inline beforenpm run build. Lint/type-check inextension-isolationwon't see it, unlikesmoke-test.Also applies to: 142-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/smoke-test.yml around lines 96 - 97, The `SKIP_ENV_VALIDATION` setting is applied inconsistently between the `smoke-test` and `extension-isolation` workflow steps. Update the `Run basic checks` step in `extension-isolation` to match `smoke-test` by setting `SKIP_ENV_VALIDATION: 'true'` at the step level, so `npm run lint`, `npm run type-check`, and `npm run build` all run with the same environment; use the existing `Run basic checks` step blocks in the workflow as the place to align this behavior.templates/remix-starter/app/styles/landing.css (1)
87-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHover lift now applies to a non-interactive CTA.
.cna-landing__button:hover(translateY lift) still fires on.cna-landing__button--primary, which per the companion Newtab.tsx.template change is now rendered as a plain<span>(non-navigable code snippet), not a link/button. Hovering an inert element with a "lift" transform signals interactivity that doesn't exist, which is a minor but real affordance mismatch.💡 Suggested scoping fix
-.cna-landing__button:hover { - transform: translateY(-2px); -} +a.cna-landing__button:hover { + transform: translateY(-2px); +}Token swaps (
--cna-glow,--cna-amber-soft), the newgap, and.cna-landing__codeclass otherwise look correct and match the shared design tokens confirmed in Newtab.css.Also applies to: 105-116, 140-174, 215-225, 275-281
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/remix-starter/app/styles/landing.css` around lines 87 - 94, The hover lift is still applied to `.cna-landing__button--primary` even though it now renders as a non-interactive `<span>`, which creates a false affordance. Update the hover styling in the landing CSS so the translateY lift only applies to truly interactive button/link variants, and explicitly exclude or neutralize `.cna-landing__button--primary` in the `.cna-landing__button:hover` rule. Use the existing `.cna-landing__button` and `.cna-landing__button--primary` selectors to scope the change cleanly.templates/webextension-react-vite-starter/[src]/popup/index.css (2)
23-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLight-mode
--popup-accentbypasses the shared token.Unlike the dark-mode default (
var(--cna-amber-soft,#fbbf24)), the light-mode override hardcodes#d97706directly instead of usingvar(--cna-amber-soft,#d97706), inconsistent with the token-based approach adopted elsewhere in this PR.♻️ Suggested fix
- --popup-accent: `#d97706`; + --popup-accent: var(--cna-amber-soft, `#d97706`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/webextension-react-vite-starter/`[src]/popup/index.css around lines 23 - 32, The light-mode `--popup-accent` override in the popup stylesheet is hardcoding a color instead of using the shared token pattern. Update the `@media (prefers-color-scheme: light)` block in the popup `body` styles to use `var(--cna-amber-soft, `#d97706`)` for `--popup-accent`, matching the token-based approach used elsewhere and keeping consistency with the dark-mode default.
7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused token indirection in the popup styles
popup/index.css:7-9and18-20only reference--cna-amber-soft,--cna-amber,--cna-teal, and--cna-glowthrough fallbacks. The popup document only loads its own CSS, and these tokens are defined elsewhere, so the fallback values are always what render here. Either define the tokens in the popup scope or inline the hardcoded values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/webextension-react-vite-starter/`[src]/popup/index.css around lines 7 - 9, The popup styles are relying on fallback-only design tokens, so the indirection in the popup CSS is effectively unused. In popup/index.css, update the popup token definitions and any dependent button/accent styles to either declare the needed values directly in the popup scope or replace the var(...) fallbacks with the hardcoded colors. Use the existing popup custom properties such as --popup-accent and --popup-button-gradient-start/end to keep the styling centralized while removing references to external tokens like --cna-amber-soft, --cna-amber, --cna-teal, and --cna-glow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/landing-visual.yml:
- Around line 61-67: The Scaffold test project step in the landing visual
workflow is masking failures with continue-on-error, which can let the job
finish green without running any visual tests. Remove the silent success
behavior from the scaffold step, or keep tolerance only via retries, and add an
explicit failure check after the scaffold step that fails the job when
steps.scaffold.outcome is not success so the downstream dev server and
Playwright steps cannot be skipped unnoticed.
In `@scripts/landing-audit.js`:
- Around line 136-139: The light-mode CSS audit in the landing-audit script is
using a regex in the lightModeBlock check that stops at the first closing brace,
so nested rules inside the `@media` block are not fully captured. Replace that
regex-based extraction with a balanced-brace helper such as extractBalancedBlock
(or equivalent logic in the same script) and use it when scanning the CSS so the
--cna-amber-soft override is detected anywhere within the full media block.
In `@scripts/landing-visual/landing.spec.ts`:
- Around line 10-29: The visual tests in landing.spec.ts are only writing
screenshots with page.screenshot(), so they never assert against a baseline and
cannot catch regressions. Update the dark mode, light mode, and reduced motion
tests to use Playwright snapshot assertions in the same test cases, and connect
the snapshot update behavior to Playwright’s built-in update flow instead of
relying on UPDATE_SNAPSHOTS=true. Keep the changes localized to the existing
test functions and any test:update wiring that currently drives snapshot
generation.
In `@scripts/sync-assets.js`:
- Around line 46-49: The missing-source path in sync-assets.js is only logging a
SKIP message and never increments the errors counter, so the final summary and
exit code stay successful. Update the fs.existsSync(sourceFile) branch in the
asset-sync loop to treat a missing shared asset as an error by incrementing
errors alongside the existing console.error, so the existing summary/exit logic
reflects the failure. Reference the sync-assets.js asset processing block that
checks sourceFile and the final errors-based exit path.
In `@templates/astro-starter/docs/PROJECT_CONFIGURATION.md.template`:
- Line 55: The example path in PROJECT_CONFIGURATION.md.template is inconsistent
because it changes from MyComponent to MyComponents, which can mislead readers.
Update the sample under the projectImportPath explanation so the referenced
component name is consistent between the source path and the import path
example, using the surrounding projectImportPath and srcDir examples as the
anchor.
In `@templates/astro-starter/docs/README.md`:
- Around line 3-24: The README in the Astro starter docs still references
Next.js throughout, so update the copy and links to be Astro-specific. In the
README content, replace the project description, core documentation links,
external resources, and help section entries with the corresponding Astro
documentation and support references. Use the README’s existing section
structure and update the wording so it matches the astro-starter template rather
than Next.js.
In `@templates/remix-starter/docs/PROJECT_CONFIGURATION.md.template`:
- Line 55: The import-path example in PROJECT_CONFIGURATION.md.template has a
typo: the referenced component name in the path should match the earlier
example. Update the example near the projectImportPath explanation so the path
uses MyComponent consistently, and keep the surrounding text unchanged.
In `@templates/remix-starter/docs/README.md`:
- Around line 1-3: The README in the docs folder still refers to the Next.js
Starter template, which is incorrect for the Remix starter. Update the text in
the README content to use Remix Starter terminology instead of Next.js Starter,
keeping the rest of the documentation wording aligned with the remix-starter
docs context.
---
Outside diff comments:
In `@templates/astro-starter/docs/PROJECT_STRUCTURE.md.template`:
- Around line 3-198: Rewrite the PROJECT_STRUCTURE guide to match Astro instead
of Next.js: replace the App Router, route groups, server components, and
Next.js-specific examples with Astro’s directory and routing conventions, using
the existing template symbols like srcDir and projectImportPath only where they
still apply. Update the architecture overview, directory tree, principles, and
example sections so they describe Astro starter structure and imports, and
ensure any references to features/shared remain accurate for this template. If
the intent was actually Next.js, move this document to the Next.js starter
instead of keeping it in the Astro docs.
In `@templates/remix-starter/docs/PROJECT_STRUCTURE.md.template`:
- Around line 3-198: The PROJECT_STRUCTURE document is written for Next.js App
Router and should be rewritten to describe Remix conventions instead. Update the
overview and directory examples in the template to use Remix route files, nested
layouts, loaders/actions, and shared app structure, and remove Next.js-specific
terms like route groups, server components, and App Router from the guidance.
Keep the feature-based architecture framing, but align the examples and import
guidance with Remix patterns and the symbols used in this template such as
srcDir, features/auth, and shared.
---
Nitpick comments:
In @.github/workflows/smoke-test.yml:
- Around line 96-97: The `SKIP_ENV_VALIDATION` setting is applied inconsistently
between the `smoke-test` and `extension-isolation` workflow steps. Update the
`Run basic checks` step in `extension-isolation` to match `smoke-test` by
setting `SKIP_ENV_VALIDATION: 'true'` at the step level, so `npm run lint`, `npm
run type-check`, and `npm run build` all run with the same environment; use the
existing `Run basic checks` step blocks in the workflow as the place to align
this behavior.
In @.github/workflows/test-combinations.yml:
- Around line 7-16: The new pull_request trigger in the test-combinations
workflow duplicates the per-PR coverage already provided by smoke-test.yml, so
remove or narrow that trigger in the workflow definition. Update the workflow
configuration to run generate-combinations/test-combinations only on schedule or
only when relevant template files change, and keep smoke-test.yml as the per-PR
coverage path.
In `@scripts/landing-visual/playwright.config.ts`:
- Around line 17-26: The Playwright config defines a firefox project that is
never used by CI, so either remove the unused firefox entry from the projects
array in the Playwright config or update the CI workflow to install and run it
alongside chromium. Use the existing project definitions in the Playwright
config as the place to make the change, and keep the project list aligned with
what the pipeline actually executes.
In `@templates/remix-starter/app/styles/landing.css`:
- Around line 87-94: The hover lift is still applied to
`.cna-landing__button--primary` even though it now renders as a non-interactive
`<span>`, which creates a false affordance. Update the hover styling in the
landing CSS so the translateY lift only applies to truly interactive button/link
variants, and explicitly exclude or neutralize `.cna-landing__button--primary`
in the `.cna-landing__button:hover` rule. Use the existing
`.cna-landing__button` and `.cna-landing__button--primary` selectors to scope
the change cleanly.
In `@templates/webextension-react-vite-starter/`[src]/popup/index.css:
- Around line 23-32: The light-mode `--popup-accent` override in the popup
stylesheet is hardcoding a color instead of using the shared token pattern.
Update the `@media (prefers-color-scheme: light)` block in the popup `body`
styles to use `var(--cna-amber-soft, `#d97706`)` for `--popup-accent`, matching
the token-based approach used elsewhere and keeping consistency with the
dark-mode default.
- Around line 7-9: The popup styles are relying on fallback-only design tokens,
so the indirection in the popup CSS is effectively unused. In popup/index.css,
update the popup token definitions and any dependent button/accent styles to
either declare the needed values directly in the popup scope or replace the
var(...) fallbacks with the hardcoded colors. Use the existing popup custom
properties such as --popup-accent and --popup-button-gradient-start/end to keep
the styling centralized while removing references to external tokens like
--cna-amber-soft, --cna-amber, --cna-teal, and --cna-glow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40e9d62a-06c6-4fca-be2f-e826ee00e0b2
⛔ Files ignored due to path filters (31)
shared/assets/favicon.svgis excluded by!**/*.svgshared/assets/logo-mark-dark.svgis excluded by!**/*.svgshared/assets/logo-mark.svgis excluded by!**/*.svgshared/assets/logo-wordmark-dark.svgis excluded by!**/*.svgshared/assets/logo-wordmark.svgis excluded by!**/*.svgtemplates/astro-starter/public/favicon.svgis excluded by!**/*.svgtemplates/astro-starter/public/logo.svgis excluded by!**/*.svgtemplates/hono-starter/public/favicon.svgis excluded by!**/*.svgtemplates/hono-starter/public/logo.svgis excluded by!**/*.svgtemplates/nestjs-starter/public/favicon.svgis excluded by!**/*.svgtemplates/nestjs-starter/public/logo.svgis excluded by!**/*.svgtemplates/nextjs-saas-ai-starter/public/favicon.svgis excluded by!**/*.svgtemplates/nextjs-saas-ai-starter/public/logo.svgis excluded by!**/*.svgtemplates/nextjs-saas-ai-starter/public/next.svgis excluded by!**/*.svgtemplates/nextjs-saas-ai-starter/public/vercel.svgis excluded by!**/*.svgtemplates/nextjs-starter/[src]/app/favicon.svgis excluded by!**/*.svgtemplates/nextjs-starter/public/favicon.svgis excluded by!**/*.svgtemplates/nextjs-starter/public/logo.svgis excluded by!**/*.svgtemplates/nextjs-starter/public/next.svgis excluded by!**/*.svgtemplates/nextjs-starter/public/vercel.svgis excluded by!**/*.svgtemplates/react-vite-starter/public/favicon.svgis excluded by!**/*.svgtemplates/react-vite-starter/public/logo.svgis excluded by!**/*.svgtemplates/remix-starter/public/favicon.svgis excluded by!**/*.svgtemplates/remix-starter/public/logo.svgis excluded by!**/*.svgtemplates/turborepo-starter/public/favicon.svgis excluded by!**/*.svgtemplates/turborepo-starter/public/logo.svgis excluded by!**/*.svgtemplates/wdio-starter/public/favicon.svgis excluded by!**/*.svgtemplates/wdio-starter/public/logo.svgis excluded by!**/*.svgtemplates/webextension-react-vite-starter/[src]/assets/img/logo.svgis excluded by!**/*.svgtemplates/webextension-react-vite-starter/public/favicon.svgis excluded by!**/*.svgtemplates/webextension-react-vite-starter/public/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (39)
.github/workflows/landing-visual.yml.github/workflows/smoke-test.yml.github/workflows/test-combinations.ymldocs/DEFAULT_LANDING_DESIGN.mddocs/DEFAULT_LANDING_GUIDE.mddocs/MAINTENANCE_RUNBOOK.mdextensions/react-tailwindcss/package.jsonextensions/react-tailwindcss/postcss.config.js.templatescripts/landing-audit.jsscripts/landing-visual/landing.spec.tsscripts/landing-visual/package.jsonscripts/landing-visual/playwright.config.tsscripts/sync-assets.jsshared/landings/features.tstemplates/astro-starter/docs/COMPONENTS_AND_STYLING.mdtemplates/astro-starter/docs/PERFORMANCE.mdtemplates/astro-starter/docs/PROJECT_CONFIGURATION.md.templatetemplates/astro-starter/docs/PROJECT_STRUCTURE.md.templatetemplates/astro-starter/docs/README.mdtemplates/astro-starter/docs/STATE_MANAGEMENT.mdtemplates/astro-starter/src/components/Welcome.astrotemplates/astro-starter/src/pages/index.astrotemplates/nextjs-starter/[src]/app/page.module.csstemplates/nextjs-starter/[src]/app/page.tsxtemplates/nextjs-starter/[src]/app/page.tsx.templatetemplates/react-vite-starter/[src]/pages/Landing.csstemplates/react-vite-starter/[src]/pages/Landing.tsx.templatetemplates/remix-starter/app/routes/_index.tsx.templatetemplates/remix-starter/app/styles/landing.csstemplates/remix-starter/docs/COMPONENTS_AND_STYLING.mdtemplates/remix-starter/docs/PERFORMANCE.mdtemplates/remix-starter/docs/PROJECT_CONFIGURATION.md.templatetemplates/remix-starter/docs/PROJECT_STRUCTURE.md.templatetemplates/remix-starter/docs/README.mdtemplates/remix-starter/docs/STATE_MANAGEMENT.mdtemplates/webextension-react-vite-starter/[src]/newtab/Newtab.csstemplates/webextension-react-vite-starter/[src]/newtab/Newtab.tsx.templatetemplates/webextension-react-vite-starter/[src]/popup/Popup.csstemplates/webextension-react-vite-starter/[src]/popup/index.css
💤 Files with no reviewable changes (2)
- templates/nextjs-starter/[src]/app/page.tsx
- templates/astro-starter/src/components/Welcome.astro
| - name: Scaffold test project | ||
| id: scaffold | ||
| continue-on-error: true | ||
| run: | | ||
| CI=true npx create-awesome-node-app visual-test-project \ | ||
| -t "file://${PWD}?subdir=templates/${{ matrix.template }}" | ||
| echo "Project scaffolded" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scaffold failures are silently swallowed, job reports success with no tests run.
continue-on-error: true on the scaffold step means its failure doesn't fail the job. Since all later steps (dev server, Playwright run, screenshot upload) are gated on steps.scaffold.outcome == 'success' and none of them run/fail in that case, the job conclusion is a false "success" — visual regression silently never executes.
🔧 Proposed fix
- name: Scaffold test project
id: scaffold
- continue-on-error: true
run: |
CI=true npx create-awesome-node-app visual-test-project \
-t "file://${PWD}?subdir=templates/${{ matrix.template }}"
echo "Project scaffolded"If scaffold flakiness needs tolerance, retry instead of masking failure, and add an explicit final step that fails the job when steps.scaffold.outcome != 'success'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Scaffold test project | |
| id: scaffold | |
| continue-on-error: true | |
| run: | | |
| CI=true npx create-awesome-node-app visual-test-project \ | |
| -t "file://${PWD}?subdir=templates/${{ matrix.template }}" | |
| echo "Project scaffolded" | |
| - name: Scaffold test project | |
| id: scaffold | |
| run: | | |
| CI=true npx create-awesome-node-app visual-test-project \ | |
| -t "file://${PWD}?subdir=templates/${{ matrix.template }}" | |
| echo "Project scaffolded" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/landing-visual.yml around lines 61 - 67, The Scaffold test
project step in the landing visual workflow is masking failures with
continue-on-error, which can let the job finish green without running any visual
tests. Remove the silent success behavior from the scaffold step, or keep
tolerance only via retries, and add an explicit failure check after the scaffold
step that fails the job when steps.scaffold.outcome is not success so the
downstream dev server and Playwright steps cannot be skipped unnoticed.
| const lightModeBlock = css.match(/@media\s*\(prefers-color-scheme:\s*light\)\s*\{([^}]*)\}/); | ||
| if (lightModeBlock && !lightModeBlock[1].includes('--cna-amber-soft')) { | ||
| error(config.name, 'Light mode block does not override --cna-amber-soft'); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
[^}]* breaks on nested CSS rules inside @media blocks.
The light-mode media block regex stops at the first }, which closes the first nested selector rule rather than the whole @media block. Any --cna-amber-soft override declared after the first nested rule is never captured, so this check can produce false positives/negatives and silently defeat the audit's purpose.
🐛 Suggested approach
- const lightModeBlock = css.match(/@media\s*\(prefers-color-scheme:\s*light\)\s*\{([^}]*)\}/);
- if (lightModeBlock && !lightModeBlock[1].includes('--cna-amber-soft')) {
+ const mediaStart = css.search(/@media\s*\(prefers-color-scheme:\s*light\)\s*\{/);
+ const lightModeBody = mediaStart !== -1 ? extractBalancedBlock(css, mediaStart) : null;
+ if (lightModeBody && !lightModeBody.includes('--cna-amber-soft')) {Implement extractBalancedBlock with a brace counter to find the matching closing } instead of relying on a non-nested regex.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/landing-audit.js` around lines 136 - 139, The light-mode CSS audit in
the landing-audit script is using a regex in the lightModeBlock check that stops
at the first closing brace, so nested rules inside the `@media` block are not
fully captured. Replace that regex-based extraction with a balanced-brace helper
such as extractBalancedBlock (or equivalent logic in the same script) and use it
when scanning the CSS so the --cna-amber-soft override is detected anywhere
within the full media block.
| test('dark mode', async ({ page }) => { | ||
| await page.emulateMedia({ colorScheme: 'dark', reducedMotion: 'no-preference' }); | ||
| await page.goto('/'); | ||
| await page.waitForLoadState('networkidle'); | ||
| await page.screenshot({ path: filename('dark'), fullPage: true }); | ||
| }); | ||
|
|
||
| test('light mode', async ({ page }) => { | ||
| await page.emulateMedia({ colorScheme: 'light', reducedMotion: 'no-preference' }); | ||
| await page.goto('/'); | ||
| await page.waitForLoadState('networkidle'); | ||
| await page.screenshot({ path: filename('light'), fullPage: true }); | ||
| }); | ||
|
|
||
| test('reduced motion', async ({ page }) => { | ||
| await page.emulateMedia({ colorScheme: 'dark', reducedMotion: 'reduce' }); | ||
| await page.goto('/'); | ||
| await page.waitForLoadState('networkidle'); | ||
| await page.screenshot({ path: filename('motion'), fullPage: true }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files scripts/landing-visual | sed 's#^`#-` #'
printf '\n== landing.spec.ts ==\n'
wc -l scripts/landing-visual/landing.spec.ts
cat -n scripts/landing-visual/landing.spec.ts
printf '\n== package.json ==\n'
wc -l scripts/landing-visual/package.json
cat -n scripts/landing-visual/package.json
printf '\n== playwright config candidates ==\n'
fd -a 'playwright.config.*' scripts/landing-visual . || true
printf '\n== search for screenshot assertions / update flag ==\n'
rg -n "toHaveScreenshot|screenshot\\(|UPDATE_SNAPSHOTS|update-snapshots|snapshot" scripts/landing-visual . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: Create-Node-App/cna-templates
Length of output: 4565
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== playwright.config.ts ==\n'
wc -l scripts/landing-visual/playwright.config.ts
cat -n scripts/landing-visual/playwright.config.ts
printf '\n== docs references around UPDATE_SNAPSHOTS ==\n'
sed -n '118,160p' docs/DEFAULT_LANDING_GUIDE.md | cat -n
printf '\n== search for UPDATE_SNAPSHOTS handling ==\n'
rg -n "UPDATE_SNAPSHOTS|toHaveScreenshot|expect\\(page\\)\\.toHaveScreenshot|snapshot" scripts/landing-visual docs -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: Create-Node-App/cna-templates
Length of output: 3577
Use screenshot assertions instead of raw file writes
scripts/landing-visual/landing.spec.ts:10-29 only calls page.screenshot(), so these tests always pass and can’t fail on a visual regression. Switch them to Playwright snapshot assertions and wire test:update to Playwright’s snapshot-update flow; UPDATE_SNAPSHOTS=true is unused as-is.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/landing-visual/landing.spec.ts` around lines 10 - 29, The visual
tests in landing.spec.ts are only writing screenshots with page.screenshot(), so
they never assert against a baseline and cannot catch regressions. Update the
dark mode, light mode, and reduced motion tests to use Playwright snapshot
assertions in the same test cases, and connect the snapshot update behavior to
Playwright’s built-in update flow instead of relying on UPDATE_SNAPSHOTS=true.
Keep the changes localized to the existing test functions and any test:update
wiring that currently drives snapshot generation.
| if (!fs.existsSync(sourceFile)) { | ||
| console.error(` SKIP ${asset} → ${template}/${targetPath} (source not found)`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing source files are logged as errors but not counted, so the script silently exits 0.
console.error is used for the SKIP message, but errors isn't incremented, so a missing shared asset never affects the final summary or exit code (Lines 67-68). In CI or automated contexts this hides a real failure mode.
🩹 Proposed fix
if (!fs.existsSync(sourceFile)) {
console.error(` SKIP ${asset} → ${template}/${targetPath} (source not found)`);
+ errors++;
continue;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!fs.existsSync(sourceFile)) { | |
| console.error(` SKIP ${asset} → ${template}/${targetPath} (source not found)`); | |
| continue; | |
| } | |
| if (!fs.existsSync(sourceFile)) { | |
| console.error(` SKIP ${asset} → ${template}/${targetPath} (source not found)`); | |
| errors++; | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sync-assets.js` around lines 46 - 49, The missing-source path in
sync-assets.js is only logging a SKIP message and never increments the errors
counter, so the final summary and exit code stay successful. Update the
fs.existsSync(sourceFile) branch in the asset-sync loop to treat a missing
shared asset as an error by incrementing errors alongside the existing
console.error, so the existing summary/exit logic reflects the failure.
Reference the sync-assets.js asset processing block that checks sourceFile and
the final errors-based exit path.
|
|
||
| [Paths Configuration](../tsconfig.json) | ||
|
|
||
| It is also possible to define multiple paths for various folders(such as `<%= projectImportPath%>components`, `<%= projectImportPath%>hooks`, etc.), but using `<%= projectImportPath%>*` works very well because it is short enough so there is no need to configure multiple paths and it differs from other dependency modules so there is no confusion in what comes from `node_modules` and what is our source folder. That means that anything in the `src` folder can be accessed via `@`, e.g some file that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponents`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the example path typo.
Line 55 switches from MyComponent to MyComponents, so the sample path is inconsistent and readers may copy a broken example.
♻️ Proposed fix
- that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponents`.
+ that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponent`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| It is also possible to define multiple paths for various folders(such as `<%= projectImportPath%>components`, `<%= projectImportPath%>hooks`, etc.), but using `<%= projectImportPath%>*` works very well because it is short enough so there is no need to configure multiple paths and it differs from other dependency modules so there is no confusion in what comes from `node_modules` and what is our source folder. That means that anything in the `src` folder can be accessed via `@`, e.g some file that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponents`. | |
| It is also possible to define multiple paths for various folders(such as `<%= projectImportPath%>components`, `<%= projectImportPath%>hooks`, etc.), but using `<%= projectImportPath%>*` works very well because it is short enough so there is no need to configure multiple paths and it differs from other dependency modules so there is no confusion in what comes from `node_modules` and what is our source folder. That means that anything in the `src` folder can be accessed via `@`, e.g some file that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponent`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/astro-starter/docs/PROJECT_CONFIGURATION.md.template` at line 55,
The example path in PROJECT_CONFIGURATION.md.template is inconsistent because it
changes from MyComponent to MyComponents, which can mislead readers. Update the
sample under the projectImportPath explanation so the referenced component name
is consistent between the source path and the import path example, using the
surrounding projectImportPath and srcDir examples as the anchor.
| This folder contains detailed documentation for the Next.js Starter template. Each document focuses on a specific aspect of the project: | ||
|
|
||
| ## Core Documentation | ||
|
|
||
| - [Project Configuration](./PROJECT_CONFIGURATION.md) - Setup and configuration of development tools | ||
| - [Components and Styling](./COMPONENTS_AND_STYLING.md) - Best practices for components and styling solutions | ||
| - [Performance](./PERFORMANCE.md) - Performance optimization guidelines | ||
| - [State Management](./STATE_MANAGEMENT.md) - State management patterns and recommendations | ||
|
|
||
| ## Additional Resources | ||
|
|
||
| - [Next.js Documentation](https://nextjs.org/docs) | ||
| - [React Documentation](https://reactjs.org/docs/getting-started.html) | ||
| - [TypeScript Documentation](https://www.typescriptlang.org/docs/) | ||
|
|
||
| ## Getting Help | ||
|
|
||
| If you need additional help or have questions: | ||
|
|
||
| 1. Check the [Next.js GitHub Discussions](https://github.com/vercel/next.js/discussions) | ||
| 2. Join the [Next.js Discord](https://discord.gg/nextjs) | ||
| 3. Open an issue in this repository |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the Next.js references with Astro-specific docs.
This folder is under astro-starter, but the copy and links are still Next.js-specific. That will mislead users of the Astro starter docs.
Suggested fix
- This folder contains detailed documentation for the Next.js Starter template.
+ This folder contains detailed documentation for the Astro Starter template.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/astro-starter/docs/README.md` around lines 3 - 24, The README in
the Astro starter docs still references Next.js throughout, so update the copy
and links to be Astro-specific. In the README content, replace the project
description, core documentation links, external resources, and help section
entries with the corresponding Astro documentation and support references. Use
the README’s existing section structure and update the wording so it matches the
astro-starter template rather than Next.js.
|
|
||
| [Paths Configuration](../tsconfig.json) | ||
|
|
||
| It is also possible to define multiple paths for various folders(such as `<%= projectImportPath%>components`, `<%= projectImportPath%>hooks`, etc.), but using `<%= projectImportPath%>*` works very well because it is short enough so there is no need to configure multiple paths and it differs from other dependency modules so there is no confusion in what comes from `node_modules` and what is our source folder. That means that anything in the `src` folder can be accessed via `@`, e.g some file that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponents`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the import-path example typo.
MyComponents should be MyComponent; as written, the example path does not match the example above and will confuse readers.
Suggested fix
- ... can be accessed using `<%= projectImportPath%>components/MyComponents`.
+ ... can be accessed using `<%= projectImportPath%>components/MyComponent`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| It is also possible to define multiple paths for various folders(such as `<%= projectImportPath%>components`, `<%= projectImportPath%>hooks`, etc.), but using `<%= projectImportPath%>*` works very well because it is short enough so there is no need to configure multiple paths and it differs from other dependency modules so there is no confusion in what comes from `node_modules` and what is our source folder. That means that anything in the `src` folder can be accessed via `@`, e.g some file that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponents`. | |
| It is also possible to define multiple paths for various folders(such as `<%= projectImportPath%>components`, `<%= projectImportPath%>hooks`, etc.), but using `<%= projectImportPath%>*` works very well because it is short enough so there is no need to configure multiple paths and it differs from other dependency modules so there is no confusion in what comes from `node_modules` and what is our source folder. That means that anything in the `src` folder can be accessed via `@`, e.g some file that lives in `<%= srcDir%>/components/MyComponent` can be accessed using `<%= projectImportPath%>components/MyComponent`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/remix-starter/docs/PROJECT_CONFIGURATION.md.template` at line 55,
The import-path example in PROJECT_CONFIGURATION.md.template has a typo: the
referenced component name in the path should match the earlier example. Update
the example near the projectImportPath explanation so the path uses MyComponent
consistently, and keep the surrounding text unchanged.
| # Extra Documentation | ||
|
|
||
| This folder contains detailed documentation for the Next.js Starter template. Each document focuses on a specific aspect of the project: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the template reference to Remix.
Line 3 still says “Next.js Starter template,” but this folder is under remix-starter/docs. That label is incorrect and should be updated.
♻️ Proposed fix
- This folder contains detailed documentation for the Next.js Starter template.
+ This folder contains detailed documentation for the Remix Starter template.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Extra Documentation | |
| This folder contains detailed documentation for the Next.js Starter template. Each document focuses on a specific aspect of the project: | |
| # Extra Documentation | |
| This folder contains detailed documentation for the Remix Starter template. Each document focuses on a specific aspect of the project: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/remix-starter/docs/README.md` around lines 1 - 3, The README in the
docs folder still refers to the Next.js Starter template, which is incorrect for
the Remix starter. Update the text in the README content to use Remix Starter
terminology instead of Next.js Starter, keeping the rest of the documentation
wording aligned with the remix-starter docs context.
- docs: rewrite PROJECT_STRUCTURE for Astro/Remix (remove Next.js copy) - docs: fix README links for Astro/Remix (s/Next.js/Astro|Remix/) - docs: fix PROJECT_CONFIGURATION import path typos - ci(landing-visual): remove continue-on-error from scaffold step - ci(smoke-test): add SKIP_ENV_VALIDATION to extension-isolation step - ci(test-combinations): narrow pull_request trigger to template/ext changes - scripts(landing-audit): use balanced-brace extraction for light mode check - scripts(landing-visual): use toHaveScreenshot assertions instead of raw screenshot - scripts(landing-visual): remove unused firefox project, add snapshotDir - scripts(sync-assets): increment errors on missing source file - css(remix): scope hover lift to interactive elements only - css(webext-popup): declare --cna-* tokens in popup scope
…store continue-on-error for scaffold
…check and landing path
Summary
Landing consolidation second phase — 4 phases of improvements to harden landing page consistency, automate verification, and prove graceful degradation with UI extensions.
Phase 1 — Consistency Hardening
shared/landings/features.tswith canonical 6-card data per template. Standardized titles (Vite 8 → Vite, React 19 → React, etc.). Added 6th card to Next.js../docs/...to/docs/.... Createddocs/dirs for remix and astro so links resolve (fixes 404s).<a href>"Start editing" to<span>with<code>snippets. Fixed webext CTA path from Popup.tsx to Newtab.tsx.rgba(245,158,11,...),#f59e0b,#d97706,#0d9488withvar(--cna-*)in remix, astro, webext. Gradient uses tokens everywhere..cna-landing__cardto stagger selector where missing. Added reduced-motion + light mode to webext popup. Next.js keyframe renamed tocna-fade-in-up.next.svg,vercel.svg,Welcome.astro, straypage.tsx.Phase 2 — Asset Robustness
role=imgto favicon,aria-hiddento wordmark text.shared/assets/logo-wordmark-dark.svgwith light text fill.scripts/sync-assets.js— idempotent asset distribution fromshared/assets/to all templates.Phase 3 — Automated Verification
scripts/landing-audit.js: Static checker for card count, docs paths, CTA structure, hardcoded colors, reduced-motion, light mode, animation selectors.validate-registryjob (wiresvalidate-templates.js),landing-auditjob. Smoke-test expanded to all 7 UI templates. NPM cache, concurrency cancel, timeouts added to all workflows.scripts/landing-visual/) +landing-visual.ymlworkflow for dark/light/motion screenshots.Phase 4 — Graceful Degradation
react-tailwindcssPostCSS config to use@tailwindcss/postcss(was on v3 plugin syntax with v4 package).Verification
node scripts/validate-templates.js— passes (pre-existing warnings for sls/backend types only)node scripts/landing-audit.js— all 5 UI landings pass (6 cards, token-based CSS, correct CTA, root-absolute docs)Summary by CodeRabbit
New Features
Bug Fixes
Chores