fix(dashboard): address UAT round 10 feedback (#729, #730, #731)

[steilerDev]

Summary

This PR addresses three critical UAT fixes for the Cornerstone dashboard:

• Fix fix(dashboard): make invoice pipeline items clickable with detail page navigation #731: Invoice Pipeline rows are now fully clickable (navigation to invoice detail)
• Fix fix(dashboard): split Timeline Status into separate Milestone, Progress, and Critical Path cards #730: Timeline Status cards split from single composite card into 3 separate cards (upcoming milestones, work item progress, critical path) with navigation links
• Fix fix(dashboard): redesign Mini Gantt to show current week at full card height #729: Mini Gantt redesigned to show the current week (Monday-Sunday) with improved UI and cleaner grid

Changes

Fix #731 — Invoice Pipeline Click-Through

• Each invoice row <li> contains a <Link> wrapping all content spans
• Navigates to /budget/invoices/${invoice.id} on click
• Added .itemLink CSS class with proper focus states and hover behavior
• Files: InvoicePipelineCard.tsx, InvoicePipelineCard.module.css

Fix #730 — Split Timeline Status Cards

• Replaced single 'timeline-status' card ID with three separate IDs:
  • 'upcoming-milestones'
  • 'work-item-progress'
  • 'critical-path'
• Removed internal wrapper divs and section headers from sub-card components (parent DashboardCard provides the title)
• Added Links to milestone detail and critical path item detail
• Deleted unused TimelineStatusCards.tsx wrapper and AtRiskItemsCard.tsx
• Files: preference.ts, UpcomingMilestonesCard.tsx, WorkItemProgressCard.tsx, CriticalPathCard.tsx, DashboardPage.tsx

Fix #729 — Mini Gantt Current Week Redesign

• Changed layout constants: ROW_HEIGHT (24→36), BAR_HEIGHT (16→24), BAR_OFFSET_Y (4→6), HEADER_HEIGHT (24→32)
• Switched from 30-day window to Monday-anchored current week (7 days)
• Header labels now show day-of-week (Mon, Tue, etc.) instead of day-of-month
• Grid simplified: 31 mixed-weight lines → 8 uniform day-boundary lines
• Today marker computed relative to week start, only renders if within bounds
• Added work item title text labels on bars (truncated if bar width < 40px)
• Removed dependency arrow rendering (too cluttered for mini view)
• Updated empty state message and SVG aria-label to reference "this week"
• Files: MiniGanttCard.tsx

Test Plan

• Invoice pipeline card: click on invoice row navigates to detail
• Timeline status: verify 3 separate cards render (upcoming milestones, work item progress, critical path)
• Milestone title and critical path deadline are clickable links
• Mini Gantt: displays current Monday-Sunday week with proper labels
• Mini Gantt: work items within the week are visible with titles on bars
• Mini Gantt: today marker displays correctly (or hidden if outside week bounds)
• Dark mode: all components render correctly
• Responsive: tablet and mobile layouts work properly

🤖 Generated with Claude Code

[steilerDev]

[product-architect]

Architecture Review — PR #732 (APPROVE)

Note: Cannot use --approve on own PR due to GitHub restriction. This review constitutes an architectural approval.

Summary

This PR implements three UAT fixes for the EPIC-09 Dashboard: invoice pipeline clickability (#731), timeline status card split (#730), and mini Gantt week-based redesign (#729). I reviewed all 16 changed files for architecture compliance, consistency, and code quality.

Findings

All clear. No blocking issues found.

Verified

1. DashboardCardId type consistency: The shared type in preference.ts correctly replaces 'timeline-status' with the three new IDs ('upcoming-milestones', 'work-item-progress', 'critical-path'). The CARD_DEFINITIONS array in DashboardPage.tsx matches exactly.

2. Deleted file cleanup: TimelineStatusCards.tsx and AtRiskItemsCard.tsx (+ test) are deleted. No remaining imports of these files exist anywhere in the codebase. DashboardPage.tsx now imports the three sub-components directly from the TimelineStatusCards/ directory.

3. Test coverage: All three changes have corresponding test updates:

   • InvoicePipelineCard.test.tsx: New test verifying link hrefs
   • UpcomingMilestonesCard.test.tsx: New test (Test 11) for milestone title links
   • CriticalPathCard.test.tsx: New test (Test 11) for deadline link to /work-items/:id
   • MiniGanttCard.test.tsx: Rewritten with daysFromMonday helper for week-based window, dependency arrow removal test (Test 10)
   • DashboardPage.test.tsx: Updated card titles array and count (8 to 10 cards, 18 loading skeletons)

4. Naming conventions: All new CSS classes use camelCase (itemLink), data-testid attributes use kebab-case, component files use PascalCase. Consistent with project conventions.

5. API contract: No backend changes. The shared DashboardCardId type is the only shared type change, and it is correctly reflected in both DashboardPage.tsx and DashboardPage.test.tsx.

6. Code quality:

   • No any types introduced
   • Inline styles in CriticalPathCard and WorkItemProgressCard use CSS custom properties (design token compliant)
   • Link components use proper to prop with React Router
   • SVG accessibility preserved: role="img" and descriptive aria-label on mini Gantt

Informational

1. E2E page object stale type: The E2E DashboardPage.ts page object (lines 13-33) still references 'timeline-status' in its DashboardCardId type and CARD_TITLES array. This is not part of this PR's scope since E2E test updates are handled separately by the e2e-test-engineer, but it will need updating before E2E tests pass against the new card structure.

2. Unused visibleDependencies in MiniGanttCard: Lines 128-132 compute visibleDependencies but the variable is never used since dependency arrows were removed. This is dead code that the linter/CI will catch.

[steilerDev]

[security-engineer]

Reviewed PR #732 (UAT round 10 — dashboard fixes: invoice click-through, timeline status split, mini Gantt redesign).

Security Review — PR #732

Scope: Frontend-only changes — InvoicePipelineCard, UpcomingMilestonesCard, WorkItemProgressCard, CriticalPathCard, MiniGanttCard, DashboardPage, preference.ts (DashboardCardId enum). No backend changes.

Checklist

• No SQL/command/XSS injection vectors in new code
• Authentication/authorization enforced on all new endpoints (no new endpoints)
• No sensitive data exposed in logs, errors, or client responses
• User input validated and sanitized at API boundaries
• New dependencies: none introduced
• No hardcoded credentials or secrets
• CORS configuration unchanged
• Error responses do not leak internal details

Link Construction Analysis

All three new <Link> usages construct paths using API-sourced entity IDs embedded in hardcoded route templates:

Component	Link target	ID type
InvoicePipelineCard	/budget/invoices/${invoice.id}	UUID string
UpcomingMilestonesCard	/schedule/milestones/${milestone.id}	integer (number)
CriticalPathCard	/work-items/${nextItem.id}	UUID string

All three are safe. React Router's <Link to=...> serializes the path through its internal URL construction — it does not interpret the to prop as an arbitrary URL, it constructs an internal href relative to the router basename. UUIDs and integers in path segments cannot carry javascript: protocol injection or open redirect payloads. No user-typed input reaches any of these IDs; they originate exclusively from authenticated API responses (GET /api/timeline, GET /api/invoices).

MiniGanttCard — SVG Text Rendering

item.title and dayLabel are rendered as JSX text nodes inside SVG <text>/<tspan> elements. React escapes all JSX text content at render time — no dangerouslySetInnerHTML, innerHTML, or eval() anywhere in the component. The character-truncation arithmetic (item.title.slice(0, ...)) operates on string indices and cannot produce executable content. The navigate('/schedule') call uses a hardcoded literal — no user-controlled navigation.

MutationObserver on data-theme

The MutationObserver in MiniGanttCard watches document.documentElement for data-theme attribute changes and calls getComputedStyle(...).getPropertyValue(name) to re-read CSS custom properties. The CSS variable names are all string literals defined in the component — no user input reaches readCssVar(). This is the same pattern used by the full GanttChart and is safe.

DashboardCardId Enum Change

timeline-status was removed from DashboardCardId; three new IDs (upcoming-milestones, work-item-progress, critical-path) were added. Users who had previously hidden the timeline-status card will have a stale entry in their dashboard.hiddenCards preference that no longer matches any card definition. The JSON parse in DashboardPage uses Set.has() against CARD_DEFINITIONS IDs — unknown IDs are silently ignored, so the stale entry has no effect beyond failing to hide a card. This is a data integrity concern, not a security risk, and is acceptable behaviour in this single-tenant model.

No security findings. This PR is clear to merge.

[steilerDev]

[product-owner] Requirements coverage review for PR #732 — UAT round 10 fixes for EPIC-09 Dashboard.

Issue #731: Invoice Pipeline Click-Through (5/5 ACs PASS)

• AC #1 (each invoice clickable): <Link> wraps all content spans in each invoice row
• AC #2 (navigates to detail page): Link target is /budget/invoices/${invoice.id}
• AC #3 (hover + focus-visible styles): .itemLink:hover and .itemLink:focus-visible with box-shadow: var(--shadow-focus) in CSS module
• AC #4 (keyboard Tab + Enter): <Link> renders as <a>, natively focusable and activatable
• AC #5 (mobile tap): Standard anchor element, works on all viewports

Issue #730: Split Timeline Status Cards (8/8 ACs PASS)

• AC #1 (2x2 grid removed): TimelineStatusCards.tsx wrapper deleted, 'timeline-status' card ID removed from CARD_DEFINITIONS and DashboardCardId type
• AC #2 (Upcoming Milestones standalone): Own card ID 'upcoming-milestones', UpcomingMilestonesCard rendered directly by DashboardPage, internal section wrapper and <h3> removed (parent DashboardCard provides title)
• AC #3 (Work Item Progress standalone): Same pattern with 'work-item-progress' card ID
• AC #4 (Critical Path standalone): Same pattern with 'critical-path' card ID
• AC #5 (At Risk Items removed): AtRiskItemsCard.tsx and its test file deleted, no remaining references
• AC #6 (milestone links to detail): <Link to={/schedule/milestones/${milestone.id}}> in UpcomingMilestonesCard.tsx, test verifies href
• AC #7 (critical path deadline links to work item): <Link to={/work-items/${nextItem.id}}> in CriticalPathCard.tsx, test verifies href
• AC #8 (responsive): Each card is a standalone DashboardCard in the existing responsive grid layout

Issue #729: Mini Gantt Current Week Redesign (6/6 ACs PASS)

• AC #1 (full card height): Layout constants increased — ROW_HEIGHT 24->36, BAR_HEIGHT 16->24, HEADER_HEIGHT 24->32
• AC #2 (current week Mon-Sun): weekStart computed as Monday of current week, CHART_DAYS = 7, header shows day-of-week labels (Mon, Tue, etc.)
• AC #3 (only overlapping items): Filter logic start <= weekEnd && end >= weekStart
• AC #4 (simplified Gantt): Same bar styling/color conventions, title labels on bars (width >= 40px), dependency arrows removed
• AC #5 (items clipped at edges): SVG viewBox naturally clips bars beyond boundaries, clampedX/clampedWidth ensure proper visual bounds
• AC #6 (responsive): SVG uses viewBox with width="100%", same responsive pattern

Non-Blocking Observations

1. DashboardCardId type change: 'timeline-status' replaced by 3 new IDs. Any persisted user preferences referencing the old ID will silently be ignored (card shows by default). Acceptable for a UAT fix within the same epic.
2. CI: Docker check failing at time of review — not a requirements coverage concern.

Verdict: APPROVE — All 19 acceptance criteria across 3 issues are satisfied.

[steilerDev]

[ux-designer]

Reviewed PR #732 against the design system (tokens.css, shared.module.css, Style Guide). Three UAT fix areas examined: InvoicePipelineCard link wrapping, TimelineStatusCards split, and MiniGanttCard redesign.

---

Summary

Overall this PR is well-executed. The token usage is strong across all three changes, focus management is correct, and the structural cleanup of TimelineStatusCards is clean. I have a small number of findings below, none of which are blocking — the most important is the hardcoded fill="white" in the SVG text layer, and a missing token for --color-warning-bg.

---

1. InvoicePipelineCard — .itemLink CSS class

Passes all checks.

• display: flex, gap: var(--spacing-2), color: inherit, text-decoration: none — all correct
• Focus visible: outline: none; box-shadow: var(--shadow-focus); border-radius: var(--radius-sm) — correct pattern
• No hardcoded values introduced
• Hover state (no underline) consistent with the pattern for row-level links throughout the app
• Overdue badge correctly sits outside the <Link> element so it is not part of the link label — correct

Low / Informational: The .itemLink:hover rule sets text-decoration: none but does not add any other affordance. Consider a subtle background-color: var(--color-bg-hover) on the parent <li> via the existing .item:hover rule so the user gets visual feedback that the row is interactive. Currently only the cursor changes. Not a blocker given that the vendor name and amounts inherit color: inherit (no visual link indicator), but worth flagging.

---

2. TimelineStatusCards — Split into 3 DashboardCards

Passes all checks.

The CSS module (TimelineStatusCards.module.css) is clean:

• All spacing uses tokens (var(--spacing-2), var(--spacing-3), var(--spacing-4))
• Typography: var(--font-size-sm), var(--font-size-xs), var(--font-size-2xl), var(--font-size-lg) — all correct
• Badges: border-radius: var(--radius-full), padding uses spacing tokens — correct
• .link focus ring: box-shadow: var(--shadow-focus) — correct
• .badgeGreen, .badgeRed use semantic tokens correctly

Medium — Missing token: --color-warning-bg

TimelineStatusCards.module.css line:

.badgeYellow {
  background-color: var(--color-warning-bg);
  color: var(--color-text-secondary);
}

--color-warning-bg does not exist in tokens.css. The token --color-warning maps to var(--color-orange-400) (light) / var(--color-orange-300) (dark), but there is no -bg variant. This will silently compute to an empty value, rendering the badge with no background color at all — a visual regression for the "Warning" health label in CriticalPathCard.

The correct approach is to use the amber in-transit token family (which does have bg and text variants) or a direct approach:

.badgeYellow {
  background-color: var(--color-hi-status-scheduled-bg);  /* --color-amber-100 in light */
  color: var(--color-hi-status-scheduled-text);           /* --color-amber-800 in light */
}

This reuses an existing amber token that is already dark-mode-aware, and is semantically appropriate for a "warning" visual treatment.

Informational — Inline style props in CriticalPathCard and UpcomingMilestonesCard

Both components use inline style props for layout and typography (e.g. style={{ display: 'flex', flexDirection: 'column', gap: 'var(--spacing-3)' }}, style={{ color: 'var(--color-text-muted)', fontSize: 'var(--font-size-xs)' }}). These correctly reference CSS custom properties rather than hardcoded values, so they are technically compliant. However the project convention is CSS Modules for component styling — moving these to the shared .module.css file would improve maintainability. This is not a blocker for a UAT fix PR.

Milestone title links (UpcomingMilestonesCard) and critical path deadline links (CriticalPathCard) both use the .link CSS class from the module — color: var(--color-primary); text-decoration: none with hover underline and var(--shadow-focus) on focus-visible. Correct and consistent with InvoicePipelineCard's "View all invoices" link style.

---

3. MiniGanttCard — Current week view redesign

Passes most checks. The core logic changes (Mon–Sun window, CHART_DAYS = 7, taller bars, day-of-week labels, today marker range guard) are all well-implemented.

Medium — Hardcoded fill="white" on bar title text (SVG)

MiniGanttCard.tsx line ~288:

<text
  x={clampedX + 4}
  y={barY + BAR_HEIGHT / 2 + 4}
  fontSize="11"
  fill="white"
  fontWeight="500"
>

SVG attributes cannot use var(), but the chart already reads all other colors through readCssVar() and the resolveColors() function. This hardcoded white will be invisible on not-started (gray) bars in light mode and potentially low-contrast on some bar colors. The correct fix is to extend resolveColors() with a barTextColor entry:

// In resolveColors():
barTextColor: readCssVar('--color-text-inverse'),  // white in light, gray-900 in dark

Then use colors.barTextColor in the fill attribute. --color-text-inverse is var(--color-white) in light mode and var(--color-gray-900) in dark mode — which may still be an issue on dark bars in dark mode. An alternative is to always use #ffffff since all bar colors (gantt bar tokens) are dark enough in both modes, but that should be an explicit design decision. At minimum, replace fill="white" with fill={colors.barTextColor} so the value is resolved through the token system and dark-mode-aware.

Low — fontSize="11" hardcoded on header labels and bar text

The day-of-week header labels use fontSize="11" (approximately 11px), and bar title text also uses fontSize="11". These are SVG presentation attributes, not CSS properties, so they cannot use var() directly. However 11 is below the --font-size-xs (12px) minimum. Reading the pixel equivalent via readCssVar and stripping the unit is the established pattern for other charts in this codebase. As a practical matter, 11px in an SVG at a dashboard preview scale is acceptable — flag informational only.

Passes — Today marker range guard

The if (x < 0 || x > CHART_WIDTH) return null; guard is correct — when today falls outside the current week window (e.g. viewing past data), the marker is suppressed rather than rendering off-chart. Good defensive implementation.

Passes — Dependency arrow removal

The visibleDependencies variable is still computed but never rendered. This is a benign dead-code situation that will be caught by lint. Informational only.

Passes — Dark mode

All bar and grid colors are read through resolveColors() / readCssVar(), which resolves at runtime from computed CSS variables. The ResizeObserver / theme-watching pattern means dark mode transitions update the SVG colors correctly. The only exception is fill="white" noted above.

Passes — Accessibility

aria-label correctly updated to "Mini Gantt: N work items scheduled this week [, M milestones]". The containing <div> retains aria-label="View full schedule" which is appropriate for the clickable wrapper.

Passes — Responsive

CHART_WIDTH = 600 with width: 100% and viewBox scaling means the SVG scales to any container width correctly.

---

Verdict

The --color-warning-bg missing token (medium) is the only issue that produces a visible regression (transparent badge background). The hardcoded fill="white" (medium) is a token-adherence issue that could affect contrast on lighter bar colors. Both should be fixed before merge.

Because I cannot request-changes on my own team's PR, this review is posted as a comment. If I could act on the severity, this would be --request-changes for the two medium findings.

[github-actions]

🎉 This PR is included in version 1.15.0-beta.14 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀