feat: activity-first home page (F11c) #37
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!37
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/f11c-home-page"
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
Replaces the static card-grid home page with an activity-first layout matching the approved HTML prototype. The new home page answers three questions at a glance: what changed recently, what is in progress, and how are boards tracking.
Changes
src/lib/api.ts— Addedid,created_at,updated_attoBoard,Note,Projectinterfaces. Addedboard_idtoBoardItem. ExtendedlistNotes()withlimit,offset,statusparams. Added newlistBoardActivity()endpoint function.src/app.css— Added--color-warning,--color-warning-bg,--color-warning-textCSS custom properties for in-progress badge styling.src/routes/+page.server.ts— Rewrote to fetchrecentNotes(limit 10),projects(sorted by updated_at),boards, andinProgressItems(from/boards/activity). Builds aboardMaplookup for activity item context.src/routes/+page.svelte— Full rewrite with four sections: greeting + recently updated notes, in-progress board items, projects sorted by activity, and board progress bars with done/in-progress/backlog segments. All colors usevar(--color-*)custom properties. Mobile-responsive at 600px breakpoint.Test Plan
item_countsfrom each boardis_public === falsenote_typeReview Checklist
svelte-checkpasses with 0 errorseslintpasses cleanRelated
plan-pal-e-docs— Phase F11c (activity-first home page)Self-Review Findings
Fixed (commit
1ed5852).badge-private:color: #fffreplaced withvar(--color-card-bg). The issue spec requires zero hardcoded hex values in component styles.Verified
var(--color-*)custom properties -- confirmed via grep, zero hardcoded hex remainingsvelte-checkpasses with 0 errors (2 pre-existing warnings in other files)eslintpasses clean--color-warning,--color-warning-bg,--color-warning-text) added toapp.cssfor the in-progress badge -- consistent with existing pattern for success/dangerid,created_at,updated_aton Board/Note/Project,board_idon BoardItem. No breaking changes to existing consumerslistNotes()params are backward compatible -- newlimit,offset,statusare optionallistBoardActivity()correctly placed in board endpoints sectionboardProgress()function correctly categorizes columns: done vs active work (in_progress/qa/needs_approval) vs backlog (backlog/todo/next_up)relativeTime()handles edge cases: just now, minutes, hours (with plural), yesterday, days, fallback to locale dateboardMapserialization:Object.fromEntries()used because SvelteKit serializes data as JSON (Maps are not serializable)Nits (non-blocking, defer to future issue)
relativeTime()is SSR-computed then static -- timestamps rendered at page load time won't update without a page refresh. Could use a Svelte$effectinterval for live updates, but this matches the prototype behavior and keeps complexity low.greetinguses$derived(getGreeting())-- this will re-evaluate on any state change but not on clock tick. Same SSR trade-off as timestamps. Acceptable for a home page that loads fresh on each visit.VERDICT: APPROVE
PR #37 Review
DOMAIN REVIEW
Tech stack: SvelteKit / TypeScript / CSS (frontend). 4 files changed, +405/-150 lines.
API integration -- Sound.
listBoardActivity()maps to the confirmed backend endpointGET /boards/activity(withcolumnandlimitquery params).listNotes()extensions (limit,offset,status) all correspond to existing backend query parameters inpal_e_docs/routes/notes.py.listProjects()already returnsid,updated_at,created_at,is_public-- the interface additions inapi.tsalign with the actual API response shape.CSS quality -- Every color in the component
<style>block usesvar(--color-*)references. Zero hardcoded hex values. The three new CSS custom properties (--color-warning,--color-warning-bg,--color-warning-text) are added toapp.cssunder the existing:rootblock, following the established pattern. Mobile breakpoint at 600px is consistent with the project convention.Component architecture -- The
boardProgress()function correctly groups columns into three buckets (done / in-progress / backlog) that map to the 7-column board model. TherelativeTime()function handles edge cases gracefully (sub-minute, hours, days, fallback totoLocaleDateString()). Empty states are rendered for all four sections.+page.server.ts-- Clean parallel fetch withPromise.all. TheboardMapis correctly converted fromMapto plain object viaObject.fromEntries()for SvelteKit serialization (Maps are not serializable over the wire). Error handling catches and re-throws as 502 with a descriptive message.Removed
listTagsimport -- The old home page fetched tags; the new one does not. ThelistTagsimport is cleanly removed from+page.server.ts. The/tagsroute still imports it independently -- no breakage.BLOCKERS
1. Existing E2E tests will break -- no test updates included.
/home/ldraney/pal-e-app/e2e/home.spec.tscontains 6 tests that assert the old page structure:getByRole('heading', { name: 'pal-e', level: 1 })-- the<h1>pal-e</h1>no longer exists (replaced by<p class="greeting">)getByRole('heading', { name: 'Projects' })-- still exists as an<h2>but the selector may work depending on the level filtergetByRole('heading', { name: 'Boards' })-- the old "Boards" heading is gone, replaced by "Board Progress"At minimum, the
renders the pal-e headingtest andboards section is renderedtest will fail. The PR must either update or replace the existinghome.spec.tstests to match the new page structure.2. New functionality has zero test coverage.
Four new sections are introduced with no corresponding tests:
This is a BLOCKER per review criteria: "New functionality with zero test coverage." At minimum, the E2E tests should assert that the four section headings render ("Recently Updated", "In Progress", "Projects", "Board Progress") and that the greeting text is visible.
NITS
$derived(getGreeting())will never re-derive.getGreeting()readsnew Date().getHours()which has no reactive dependencies. The greeting computes once at page load and never updates. If a user keeps the tab open across the morning/afternoon boundary, it stays stale. Consider either accepting this as intentional (SSR-rendered, rarely matters) or adding a comment noting the behavior.Board progress bars lack accessibility. The colored bar segments (
<div class="done" style="width: ...">) convey meaning only visually. Screen readers get no information about board progress. Consider addingaria-labelortitleattributes to the.board-barcontainer (e.g.,aria-label="{board.name}: {progress.done} done, {progress.inProgress} in progress, {progress.backlog} backlog"), orrole="progressbar"witharia-valuenow/aria-valuemax.relativeTimecould be extracted to a shared utility. The function is well-written but lives inline in the component. If other pages later need relative timestamps (e.g., project detail, note detail), it will need to be duplicated. Consider extracting to$lib/utils.ts. Not urgent since it is only used in one place currently..section-header:first-childselector may not match. The first.section-headerin the DOM is preceded by the greeting<p>elements, so:first-childwon't apply (the<p class="greeting">is the actual first child). Themargin-top: 0rule is effectively dead CSS. This has no visual impact since the greeting provides natural spacing above the first section header.Progress bar segments use
{#if progress.donePct}which treats0as falsy. This is actually correct behavior (don't render a zero-width segment), but worth noting that a board with, say, 1 done out of 1000 items (0.1%) will still render -- only exact 0 is excluded.SOP COMPLIANCE
feat/f11c-home-page(references plan phase F11c, not issue #36). Convention expects issue number in branch name.plan-pal-e-docswith phase F11c noted.Closes #36includedPROCESS OBSERVATIONS
home.spec.ts, this PR will fail the pipeline. If CI is not running E2E tests currently, the breakage is silent and accumulates as tech debt.listTagsremoval is clean. No dead code left behind. Other routes that uselistTagsare unaffected.VERDICT: NOT APPROVED
Two blockers must be resolved:
e2e/home.spec.tsto match the new page structure (at minimum fix the two tests that will definitely fail).PR #37 Re-Review
Re-review after dev agent pushed fixes for 2 blockers and 3 nits from the initial review.
PREVIOUS BLOCKER RESOLUTION
Blocker 1 (E2E tests breaking): RESOLVED.
e2e/home.spec.tsrewritten with 8 tests matching the new page structure. Old assertions (pal-eheading, bareProjects/Boardssections) replaced with tests for greeting, recently updated notes, in progress items, project list, and board progress bars.Blocker 2 (No test coverage for new sections): RESOLVED. All four new sections have dedicated E2E tests:
renders the greeting-- greeting element visiblerecently updated section is rendered with note list-- heading +.note-listin progress section is rendered-- heading checkprojects section is rendered with project list-- heading +.project-listboard progress section is rendered with progress bars-- heading +.board-barPREVIOUS NIT RESOLUTION
Nit 1 ($derived greeting): Addressed. Changed to
let greeting = getGreeting()-- a plain function call. This means the greeting will not reactively update if a user leaves the tab open across a time-of-day boundary (e.g., 11:59 AM to 12:01 PM), but this is a negligible edge case and acceptable.Nit 2 (Dead :first-child CSS): RESOLVED. No
:first-childselectors in the component.Nit 3 (Missing ARIA on progress bars): RESOLVED. Each
.board-barnow hasrole="progressbar",aria-labelwith board name and counts,aria-valuenow,aria-valuemin, andaria-valuemax.DOMAIN REVIEW (SvelteKit / TypeScript / CSS)
Tech stack: SvelteKit 5 (
$props,$derivedrunes), TypeScript, CSS custom properties, Playwright E2E.Accessibility:
{:else}blocks with descriptive text).Performance:
Promise.allfor 4 parallel API calls -- good.listNotes({ limit: 10 })andlistBoardActivity({ column: 'in_progress', limit: 5 })prevent over-fetching.boardMapserialized asObject.fromEntries(boardMap)for SSR transfer -- correct pattern (Maps are not serializable).Responsive design:
src/app.css.flex-direction: column).white-space: nowrapon timestamps withflex-shrink: 0prevents timestamp wrapping on desktop.CSS quality:
var(--color-*)custom properties.:root(--color-warning,--color-warning-bg,--color-warning-text) -- properly placed in the theme token system.0.6rempadding,0.5rem/0.75remmargins).:last-childused correctly to remove bottom borders on list items.Component architecture:
relativeTime()is a pure utility function -- could be extracted to$lib/utils.tsfor reuse, but acceptable inline for now.boardProgress()correctly handles missingitem_counts, zero-total boards, and all 7 column types.listTagsimport and addedlistBoardActivity.listTagsstill used in/tagsand/notes/[slug]/editroutes -- no dead import inapi.ts.Input validation:
listBoardActivityandlistNotesuseURLSearchParams-- safe from injection.encodeURIComponentused on route params in existing code -- consistent.BLOCKERS
None. Both previous blockers are resolved.
NITS
relativeTimecould handle negative diffs. Ifupdated_atis in the future (clock skew between server and client),diffMsgoes negative and the function returns "just now" (sincediffMin < 1). This is safe but could be confusing. Very minor -- not blocking.boardProgressreturn type is implicit. The function returns either a 4-property object (total === 0case) or a 7-property object. TypeScript will infer a union type, but an explicit return type interface would make the contract clearer. Non-blocking.In Progress section test only checks heading, not content. The
recently updatedandprojectstests both assert heading + list container, butin progress section is renderedonly asserts the heading. A.progress-itemcheck would be symmetric. Non-blocking since the section may legitimately be empty.SOP COMPLIANCE
feat/f11c-home-pagereferences F11c from issue #36plan-pal-e-docs-- Phase F11cCloses #36present in PR bodyPROCESS OBSERVATIONS
Clean re-review cycle. Both blockers from the initial review (broken E2E tests, missing test coverage for new sections) were addressed in the same branch push. All 3 nits (ARIA, dead CSS, $derived) were also addressed. The PR body's Test Plan is thorough with 9 manual verification items. The 5 changed files are tightly scoped to the feature with no scope creep. The
listTagsremoval from the home page is a clean refactor that does not affect other routes.VERDICT: APPROVED