feat: rewrite project route to use ProjectLayout with architecture diagrams #83
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo_admin/pal-e-docs-app!83
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "73-port-project-page-v2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
/projects/[slug]to delegate toProjectLayoutwhen a project-page note exists, enabling architecture diagram rendering (mermaid blocks) and matching the playground designProjectLayoutcomponent (from PR #80) that was built but never connected to the routeChanges
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 buildpassesnpm run checkpasses (only pre-existing errors in boards/[slug])/projects/pal-e-docs-- renders via ProjectLayout with architecture diagram/notes/[slug]for project-page notesReview Checklist
Related Notes
forgejo_admin/pal-e-app#73-- the Forgejo issue this PR implementspal-e-docs-- the project this work belongs toCloses #73
QA Review
Files reviewed:
src/routes/projects/[slug]/+page.svelte(1 file, +154/-124)Findings
Nit (minor):
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:
filteredNotesderived state excludes the project-page note from the fallback listingPromise.allfor both the initial data and the secondary block/toc/slug fetchnote-header,block-heading,note-list,note-card--compact,board-progress, etc.)VERDICT: APPROVE_WITH_NITS
Remove the unused
typeColorimport. Otherwise this is clean and ready to merge.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 aproject-pagenote exists in the project's notes, it delegates rendering toProjectLayoutwith the proper props (note,blocks,toc,knownSlugs,isAuthenticated). ThePropsinterface inProjectLayoutis 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 insrc/app.css.Unused import cleanup --
typeColorfrom$lib/colorsis 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-pagenote detection and secondary fetch (blocks, toc, slugs) is done inside the sameonMounttry/catch, with properPromise.allparallelization. Error handling catches and displays failures. ThepageNotefilter excludes the project-page from the notes-by-type listing, preventing double-display.Accessibility -- Anchor links on headings (
#board,#type-{type}) provide deep-linking. Thedata-typeattribute 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. Themax-width: 64remmatchesProjectLayout.svelte.remunits 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
Undefined CSS class
block--board-summary-- The fallback path usesclass="block block--board-summary"butblock--board-summaryis not defined inapp.css. Theblockbase class applies, but the modifier is a no-op. Either define it inapp.cssor remove it to avoid confusion about whether it is doing anything.Redundant API calls when
pageNoteexists -- When the ProjectLayout path is taken, the parent fetcheslistBoardsandlistNotes, thenProjectLayoutinternally fetches them again in its ownonMount. 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.authenticatedderived vs. localauth-- The code creates bothlet authenticated = $derived(isAuthenticated())at module scope andconst auth = isAuthenticated()insideonMount. The derived is used only for theProjectLayoutprop. 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
73-port-project-page-v2references issue #73)PROCESS OBSERVATIONS
VERDICT: APPROVED