feat: rewrite project route to use ProjectLayout with architecture diagrams #83

Merged
forgejo_admin merged 2 commits from 73-port-project-page-v2 into main 2026-03-28 05:28:41 +00:00

Summary

  • Rewrites /projects/[slug] to delegate to ProjectLayout when a project-page note exists, enabling architecture diagram rendering (mermaid blocks) and matching the playground design
  • Projects without a page note get a fallback listing restyled with playground CSS classes instead of Tailwind utilities
  • Wires up the ProjectLayout component (from PR #80) that was built but never connected to the route

Changes

  • src/routes/projects/[slug]/+page.svelte: Rewritten to find the project-page note, fetch its blocks/TOC/slugs, and render via ProjectLayout. Fallback path uses playground CSS classes (note-header, block-heading, note-list, note-card--compact, board-progress). All Tailwind utility classes removed from template.

Test Plan

  • npm run build passes
  • npm run check passes (only pre-existing errors in boards/[slug])
  • Visit /projects/pal-e-docs -- renders via ProjectLayout with architecture diagram
  • Visit a project without a page note -- shows fallback listing with board summary + notes by type
  • No regressions on /notes/[slug] for project-page notes

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • forgejo_admin/pal-e-app#73 -- the Forgejo issue this PR implements
  • pal-e-docs -- the project this work belongs to
  • Dependency: #68 (merged as PR #75)
  • Prior work: PR #80 created ProjectLayout.svelte

Closes #73

## Summary - Rewrites `/projects/[slug]` to delegate to `ProjectLayout` when a project-page note exists, enabling architecture diagram rendering (mermaid blocks) and matching the playground design - Projects without a page note get a fallback listing restyled with playground CSS classes instead of Tailwind utilities - Wires up the `ProjectLayout` component (from PR #80) that was built but never connected to the route ## Changes - `src/routes/projects/[slug]/+page.svelte`: Rewritten to find the project-page note, fetch its blocks/TOC/slugs, and render via ProjectLayout. Fallback path uses playground CSS classes (note-header, block-heading, note-list, note-card--compact, board-progress). All Tailwind utility classes removed from template. ## Test Plan - [x] `npm run build` passes - [x] `npm run check` passes (only pre-existing errors in boards/[slug]) - [ ] Visit `/projects/pal-e-docs` -- renders via ProjectLayout with architecture diagram - [ ] Visit a project without a page note -- shows fallback listing with board summary + notes by type - [ ] No regressions on `/notes/[slug]` for project-page notes ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - `forgejo_admin/pal-e-app#73` -- the Forgejo issue this PR implements - `pal-e-docs` -- the project this work belongs to - Dependency: #68 (merged as PR #75) - Prior work: PR #80 created ProjectLayout.svelte Closes #73
feat: rewrite project route to use ProjectLayout with architecture diagrams
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
7597bc2924
When a project has a project-page note, the /projects/[slug] route now
fetches its blocks (including mermaid architecture diagrams) and renders
via ProjectLayout. Projects without a page note get a fallback listing
styled with playground CSS classes instead of Tailwind utilities.

Closes #73

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Owner

QA Review

Files reviewed: src/routes/projects/[slug]/+page.svelte (1 file, +154/-124)

Findings

Nit (minor):

  1. Unused import typeColor -- import { typeColor } from '$lib/colors' is still present but no longer referenced in the template. The old code used it for colored dots next to type section headings; the new fallback path does not. Should be removed to keep imports clean.

Positive observations:

  • Clean two-path rendering: ProjectLayout when project-page note exists, playground-styled fallback otherwise
  • Proper filteredNotes derived state excludes the project-page note from the fallback listing
  • Parallel fetching with Promise.all for both the initial data and the secondary block/toc/slug fetch
  • All Tailwind utility classes replaced with playground CSS classes (note-header, block-heading, note-list, note-card--compact, board-progress, etc.)
  • Build passes, type check passes (pre-existing errors only)
  • Scoped styles are minimal and only cover page-specific concerns; global playground classes handle the rest

VERDICT: APPROVE_WITH_NITS

Remove the unused typeColor import. Otherwise this is clean and ready to merge.

## QA Review **Files reviewed:** `src/routes/projects/[slug]/+page.svelte` (1 file, +154/-124) ### Findings **Nit (minor):** 1. **Unused import `typeColor`** -- `import { typeColor } from '$lib/colors'` is still present but no longer referenced in the template. The old code used it for colored dots next to type section headings; the new fallback path does not. Should be removed to keep imports clean. **Positive observations:** - Clean two-path rendering: ProjectLayout when project-page note exists, playground-styled fallback otherwise - Proper `filteredNotes` derived state excludes the project-page note from the fallback listing - Parallel fetching with `Promise.all` for both the initial data and the secondary block/toc/slug fetch - All Tailwind utility classes replaced with playground CSS classes (`note-header`, `block-heading`, `note-list`, `note-card--compact`, `board-progress`, etc.) - Build passes, type check passes (pre-existing errors only) - Scoped styles are minimal and only cover page-specific concerns; global playground classes handle the rest ### VERDICT: APPROVE_WITH_NITS Remove the unused `typeColor` import. Otherwise this is clean and ready to merge.
fix: remove unused typeColor import from project route
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
7353ce7c4b
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Owner

PR #83 Review

DOMAIN REVIEW

Tech stack: SvelteKit 5 (Svelte 5 runes), TypeScript, CSS custom properties, client-side SPA (adapter-static).

Component wiring -- The PR correctly wires ProjectLayout.svelte (from PR #80) into the /projects/[slug] route. When a project-page note exists in the project's notes, it delegates rendering to ProjectLayout with the proper props (note, blocks, toc, knownSlugs, isAuthenticated). The Props interface in ProjectLayout is satisfied correctly.

Tailwind cleanup -- All Tailwind utility classes (py-4, mb-6, text-2xl, font-bold, flex, items-center, gap-x-3, etc.) are removed from the template. Replaced with playground CSS classes (note-header, block-heading, note-list, note-card--compact, board-progress, breadcrumb, view-all, badge, meta-time). All referenced classes are verified to exist in src/app.css.

Unused import cleanup -- typeColor from $lib/colors is correctly removed. New imports (getCachedSlugs, relativeTime, getNoteBlocks, getNoteToc, ProjectLayout) and types (Block, TocEntry) are all used and exist in the codebase.

Data fetching -- The project-page note detection and secondary fetch (blocks, toc, slugs) is done inside the same onMount try/catch, with proper Promise.all parallelization. Error handling catches and displays failures. The pageNote filter excludes the project-page from the notes-by-type listing, preventing double-display.

Accessibility -- Anchor links on headings (#board, #type-{type}) provide deep-linking. The data-type attribute on note cards enables CSS targeting by type. Breadcrumb navigation is clear. No ARIA regressions from the old code (the old code also lacked ARIA landmarks).

CSS quality -- New component-scoped styles use var() tokens consistently. No magic numbers. The max-width: 64rem matches ProjectLayout.svelte. rem units throughout.

BLOCKERS

None.

On test coverage: This repo has no unit tests -- only E2E tests in e2e/. None of the existing E2E tests cover /projects/[slug], and none of the prior "port from playground" PRs (#76-#81) added route-specific E2E tests either. This is a view-only page with no user input, no mutations, and no auth-gated writes. The PR's test plan includes manual verification steps (npm run build, npm run check, visit the route). Given the established pattern across the playground port series and the absence of security-sensitive behavior, this does not meet the blocker threshold.

NITS

  1. Undefined CSS class block--board-summary -- The fallback path uses class="block block--board-summary" but block--board-summary is not defined in app.css. The block base class applies, but the modifier is a no-op. Either define it in app.css or remove it to avoid confusion about whether it is doing anything.

  2. Redundant API calls when pageNote exists -- When the ProjectLayout path is taken, the parent fetches listBoards and listNotes, then ProjectLayout internally fetches them again in its own onMount. This is by design of the self-contained component, but it results in duplicate network requests for boards and notes. Not blocking, but worth considering whether the parent could pass the already-fetched board down as a prop in a future iteration.

  3. authenticated derived vs. local auth -- The code creates both let authenticated = $derived(isAuthenticated()) at module scope and const auth = isAuthenticated() inside onMount. The derived is used only for the ProjectLayout prop. This is functionally fine but slightly confusing -- a comment clarifying that the derived tracks reactivity for the template while the local captures the value at mount time would help readability.

SOP COMPLIANCE

  • Branch named after issue (73-port-project-page-v2 references issue #73)
  • PR body has Summary, Changes, Test Plan, Related sections
  • Related section references issue #73 and dependency PRs
  • No secrets committed
  • No unnecessary file changes (single file, tightly scoped)
  • Commit messages are descriptive
  • Related section does not reference a plan slug (no plan slug appears to exist for this work; this is standalone board work, so acceptable)

PROCESS OBSERVATIONS

  • This is the final PR in the "port from playground" series (#75-#83). The series has been consistent in approach: replace Tailwind with playground CSS classes, wire up layout components, maintain fallback paths.
  • The three unchecked items in the Test Plan (manual visit verification) are appropriate for visual UI work that cannot be automated without E2E infrastructure for this route.
  • Change failure risk is low -- this is a self-contained route rewrite with a clean fallback path. No shared state mutations, no API changes.

VERDICT: APPROVED

## PR #83 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit 5 (Svelte 5 runes), TypeScript, CSS custom properties, client-side SPA (adapter-static). **Component wiring** -- The PR correctly wires `ProjectLayout.svelte` (from PR #80) into the `/projects/[slug]` route. When a `project-page` note exists in the project's notes, it delegates rendering to `ProjectLayout` with the proper props (`note`, `blocks`, `toc`, `knownSlugs`, `isAuthenticated`). The `Props` interface in `ProjectLayout` is satisfied correctly. **Tailwind cleanup** -- All Tailwind utility classes (`py-4`, `mb-6`, `text-2xl`, `font-bold`, `flex`, `items-center`, `gap-x-3`, etc.) are removed from the template. Replaced with playground CSS classes (`note-header`, `block-heading`, `note-list`, `note-card--compact`, `board-progress`, `breadcrumb`, `view-all`, `badge`, `meta-time`). All referenced classes are verified to exist in `src/app.css`. **Unused import cleanup** -- `typeColor` from `$lib/colors` is correctly removed. New imports (`getCachedSlugs`, `relativeTime`, `getNoteBlocks`, `getNoteToc`, `ProjectLayout`) and types (`Block`, `TocEntry`) are all used and exist in the codebase. **Data fetching** -- The `project-page` note detection and secondary fetch (blocks, toc, slugs) is done inside the same `onMount` try/catch, with proper `Promise.all` parallelization. Error handling catches and displays failures. The `pageNote` filter excludes the project-page from the notes-by-type listing, preventing double-display. **Accessibility** -- Anchor links on headings (`#board`, `#type-{type}`) provide deep-linking. The `data-type` attribute on note cards enables CSS targeting by type. Breadcrumb navigation is clear. No ARIA regressions from the old code (the old code also lacked ARIA landmarks). **CSS quality** -- New component-scoped styles use `var()` tokens consistently. No magic numbers. The `max-width: 64rem` matches `ProjectLayout.svelte`. `rem` units throughout. ### BLOCKERS None. **On test coverage**: This repo has no unit tests -- only E2E tests in `e2e/`. None of the existing E2E tests cover `/projects/[slug]`, and none of the prior "port from playground" PRs (#76-#81) added route-specific E2E tests either. This is a view-only page with no user input, no mutations, and no auth-gated writes. The PR's test plan includes manual verification steps (`npm run build`, `npm run check`, visit the route). Given the established pattern across the playground port series and the absence of security-sensitive behavior, this does not meet the blocker threshold. ### NITS 1. **Undefined CSS class `block--board-summary`** -- The fallback path uses `class="block block--board-summary"` but `block--board-summary` is not defined in `app.css`. The `block` base class applies, but the modifier is a no-op. Either define it in `app.css` or remove it to avoid confusion about whether it is doing anything. 2. **Redundant API calls when `pageNote` exists** -- When the ProjectLayout path is taken, the parent fetches `listBoards` and `listNotes`, then `ProjectLayout` internally fetches them again in its own `onMount`. This is by design of the self-contained component, but it results in duplicate network requests for boards and notes. Not blocking, but worth considering whether the parent could pass the already-fetched board down as a prop in a future iteration. 3. **`authenticated` derived vs. local `auth`** -- The code creates both `let authenticated = $derived(isAuthenticated())` at module scope and `const auth = isAuthenticated()` inside `onMount`. The derived is used only for the `ProjectLayout` prop. This is functionally fine but slightly confusing -- a comment clarifying that the derived tracks reactivity for the template while the local captures the value at mount time would help readability. ### SOP COMPLIANCE - [x] Branch named after issue (`73-port-project-page-v2` references issue #73) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related section references issue #73 and dependency PRs - [x] No secrets committed - [x] No unnecessary file changes (single file, tightly scoped) - [x] Commit messages are descriptive - [ ] Related section does not reference a plan slug (no plan slug appears to exist for this work; this is standalone board work, so acceptable) ### PROCESS OBSERVATIONS - This is the final PR in the "port from playground" series (#75-#83). The series has been consistent in approach: replace Tailwind with playground CSS classes, wire up layout components, maintain fallback paths. - The three unchecked items in the Test Plan (manual visit verification) are appropriate for visual UI work that cannot be automated without E2E infrastructure for this route. - Change failure risk is low -- this is a self-contained route rewrite with a clean fallback path. No shared state mutations, no API changes. ### VERDICT: APPROVED
forgejo_admin deleted branch 73-port-project-page-v2 2026-03-28 05:28:41 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/pal-e-docs-app!83
No description provided.