refactor(ui): harmonize page layout with shared PageLayout and SubNav components

[steilerDev]

Summary

• Create shared PageLayout component (container, header, subNav, content slots) and unified SubNav component to replace 4 duplicate section-specific SubNav implementations
• Migrate all 12+ pages (Project, Budget, Settings sections) to use the new shared components for consistent layout and structure
• Fix DashboardPage and MilestoneCreatePage import issues, restore ManagePage h1 heading, fix SubsidyProgramsPage hardcoded English strings, fix ScheduleSubNav aria-label

Fixes #1187

Test plan

• Unit tests for PageLayout and SubNav components pass (27 new tests)
• Stale SettingsSubNav test mocks cleaned up
• E2E BudgetCategoriesPage heading locator un-deprecated
• German translation for schedule.navigation.ariaLabel added
• Pre-commit hook quality gates pass
• CI Quality Gates pass

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com

[steilerDev]

[product-owner]

All 11 acceptance criteria verified against the diff. This PR meets requirements. Approving (posted as comment due to GitHub self-review restriction).

AC Verification

#	Criterion	Verdict
1	PageLayout at client/src/components/PageLayout/	PASS
2	Unified SubNav at client/src/components/SubNav/	PASS
3	All 12 affected pages use PageLayout	PASS
4	Old per-page CSS classes removed	PASS
5	Old SubNav components deleted/replaced	PASS
6	ManagePage h1 restored	PASS
7	ManagePage tabList border-bottom removed	PASS
8	Container max-width 1200px/1400px	PASS
9	Title font-size-4xl / 2xl responsive	PASS
10	Header spacing-6 consistent	PASS
11	TimelinePage not affected	PASS

Additional Observations (non-blocking, positive)

• SubsidyProgramsPage hardcoded English strings replaced with proper i18n keys -- good catch.
• ScheduleSubNav aria-label fixed from the Gantt tab label to "Schedule section navigation" -- correct, the old value was a bug.
• BudgetCategoriesPage POM heading locator un-deprecated to match restored h1.
• Settings page tests properly mock AuthContext instead of stubbing out SettingsSubNav.
• 27 new unit tests for the shared components.

CI is still running at time of review. Approval is contingent on Quality Gates passing.

[steilerDev]

[product-architect]

Architecture Review: PageLayout & SubNav Harmonization

Verdict: APPROVE (cannot use --approve on own PR; this review carries approval weight)

Verified Areas

1. Component Design -- GOOD

PageLayout is well-scoped: title, action, subNav, children, maxWidth, testId. The slot-based API is composable and follows existing shared component patterns (Modal, EmptyState). The SubNav component correctly extracts the common tab-rendering logic with a clean SubNavTab[] + ariaLabel interface. The TabLink inner component properly scopes useTranslation per-tab via the ns field, which is a nice touch for the schedule namespace case.

2. CSS Token Compliance -- GOOD

PageLayout.module.css uses design tokens throughout (--spacing-*, --font-size-*, --font-weight-*, --color-text-primary). The max-width: 1200px/1400px values and min-height: 44px (WCAG touch target) are structural layout values, not design tokens -- consistent with the existing codebase pattern. SubNav.module.css is a clean rename of the former BudgetSubNav.module.css with full token usage.

3. Accessibility -- GOOD

• Semantic <nav> element with aria-label on every SubNav instance
• role="list" / role="listitem" for tab navigation
• <h1> rendered by PageLayout for every page (restoring the ManagePage heading that was removed in #1185)
• :focus-visible styling with --shadow-focus-subtle on tabs
• 44px minimum touch target on mobile/tablet for action buttons
• ScheduleSubNav aria-label fix (was incorrectly using the first tab label "Gantt")
• New i18n keys (schedule.navigation.ariaLabel) for both EN and DE

4. E2E Test Preservation -- GOOD

BudgetCategoriesPage.ts correctly un-deprecates the heading locator (h1 "Manage" is back via PageLayout). The goto() readiness check still uses createFormHeading, which is sensible. The @deprecated JSDoc annotation is properly removed. data-testid attributes on schedule tabs are preserved through the testId field on SubNavTab.

5. Dead Code Removal -- GOOD

Four section-specific SubNav components fully deleted (BudgetSubNav, ProjectSubNav, SettingsSubNav with their CSS modules, ScheduleSubNav CSS). ~580 lines of duplicated CSS removed. 12 page-level CSS modules cleaned of .container, .header, .pageTitle boilerplate. The ScheduleSubNav component is retained but refactored to delegate to the shared SubNav.

6. SubsidyProgramsPage i18n Fix -- GOOD

Hardcoded English strings ("Budget", "Error", "Retry", "Loading subsidy programs...") replaced with proper t() calls. Good catch.

Medium Findings (noted for refinement, not blocking)

M1: Settings tab definition duplicated in 4 pages

BackupsPage, ManagePage, ProfilePage, and UserManagementPage each independently construct the identical settingsTabs array (including the useAuth() call for visible: isAdmin). The budget tabs (BUDGET_TABS) are duplicated across 5 files. Consider extracting these into shared constants (e.g., lib/navigationTabs.ts) in a follow-up. This is purely a DRY concern -- the current approach works correctly.

M2: ProfilePage CSS still has hardcoded rem/px values

The removed boilerplate (.container, .pageTitle) used hardcoded 2rem, 1.5rem, etc. These are now handled by PageLayout with tokens. However, the remaining ProfilePage CSS (.card, .errorCard, .button, etc.) still uses raw rem/px values like 0.5rem, 0.75rem, 0.375rem, 1rem instead of var(--spacing-*) and var(--radius-*) tokens. This is pre-existing technical debt, not introduced by this PR -- but worth noting for a future token compliance pass.

Summary

Clean, well-executed refactoring. Two new shared components with proper tests (27 for PageLayout, comprehensive SubNav coverage). Consistent migration across 12+ pages. Good accessibility improvements (ScheduleSubNav aria-label fix, ManagePage h1 restoration, SubsidyProgramsPage i18n fix). No architecture or contract concerns.

[steilerDev]

[ux-designer]

Design Review — PR #1188: Shared PageLayout + SubNav Components

Reviewed against tokens.css, shared.module.css, wiki/Style-Guide.md, and the component source.

---

Summary

This is a well-executed structural refactor. It correctly consolidates 4 duplicate SubNav implementations and 12+ per-page container/header patterns into two shared components with consistent token usage. The result is a significant reduction in duplication and the new components are properly spec'd for shared reuse.

---

Token Adherence

PageLayout.module.css — all values use design tokens. Spacing, typography, and color all reference var(--token-name). The min-height: 44px touch target literals are intentional and correct (there is no spacing token at 44px; --spacing-10 = 40px and --spacing-12 = 48px).

The two container max-widths (1200px / 1400px) are hardcoded pixel values — they are not in tokens.css. These values existed previously in individual page modules, so this PR neither introduces nor worsens the situation. I recommend adding --layout-max-width-narrow: 1200px and --layout-max-width-wide: 1400px as layout tokens to tokens.css in a follow-up, so the shared component can reference them properly. Tracking this as informational — the refactor is net-positive on token compliance overall.

SubNav.module.css (renamed from BudgetSubNav.module.css with a 96% similarity) — clean token usage throughout. All colors, spacing, font-sizes, radii, and transition durations reference var(--token-name).

ProfilePage.module.css — contains many pre-existing hardcoded values (e.g., 2rem, 0.875rem, 700, 0.375rem). These are not introduced by this PR (the diff only removes the .container/.header/.pageTitle selectors). I'm flagging these as informational pre-existing debt; they should be addressed in a dedicated cleanup story.

---

Dark Mode Correctness

All color references in both new CSS modules use semantic Layer 2 tokens (--color-text-primary, --color-bg-secondary, --color-primary, --color-primary-hover, --shadow-focus-subtle). All of these switch correctly under [data-theme="dark"]. No hardcoded hex or rgba values introduced. Dark mode is correct.

---

Responsive Behavior

PageLayout.module.css:

• Mobile (max-width: 767px): padding reduces from --spacing-8 to --spacing-4, header stacks vertically, action widens to full width with min-height: 44px. Correct.
• Tablet (min-width: 768px and max-width: 1024px): action also gets min-height: 44px. The use of 1024px as both the upper bound of the tablet range and the lower bound of the desktop breakpoint creates a one-pixel overlap. The convention used in this project is max-width: 1023px for the tablet upper bound (documented in review findings for PRs #380, #398, #414). This should be max-width: 1023px.

SubNav.module.css: Mobile padding reduction at max-width: 767px is correct and unchanged from the prior per-component implementations.

---

Accessibility

SubNav: <nav aria-label={ariaLabel}> → correct landmark. The tab container uses role="list" and each NavLink uses role="listitem". This is a consistent pattern carried forward from the prior components. The aria-labels ("Budget section navigation", "Project section navigation", "Schedule section navigation", "Settings section navigation") are descriptive and unique per-section.

PageLayout: Renders the page title as <h1> — establishes correct heading hierarchy for all migrated pages. Previously, several pages (BackupsPage, ProfilePage in certain states) did not render an <h1> in the loading/error early-return states. The new PageLayout renders the <h1> consistently regardless of state. This is an accessibility improvement.

ScheduleSubNav aria-label fix: The old ScheduleSubNav used the first tab's translated label ("Gantt") as the nav aria-label — a semantic error since that's a tab label, not a navigation landmark description. The fix to aria-label={t('schedule.navigation.ariaLabel')} is correct.

ARIA live regions: No new interactive widgets introduced; no live region requirements.

Focus management: SubNav.tab:focus-visible uses box-shadow: var(--shadow-focus-subtle) — correct pattern, consistent with the design system.

---

Visual Consistency

• h1 font size: PageLayout uses --font-size-4xl (32px) at desktop, scaling to --font-size-2xl (24px) on mobile. This matches the previous per-page implementations for budget/project pages. Settings pages previously used --font-size-4xl or 2rem (identical value), so no visual regression.
• h1 missing in some prior states: Previously ManagePage had no <h1> at all (SettingsSubNav didn't render one). Now ManagePage renders t('manage.pageTitle') via PageLayout — this is a visual addition / restoration consistent with #1187.
• BUDGET_TABS constant duplication: The BUDGET_TABS array is defined in 5 separate files (BudgetOverviewPage, BudgetSourcesPage, InvoicesPage, SubsidyProgramsPage, VendorsPage) and PROJECT_TABS in 5 files (DashboardPage, HouseholdItemsPage, MilestonesPage, MilestoneCreatePage, WorkItemsPage). These should be extracted to shared constants (e.g., client/src/lib/navTabs.ts) to avoid divergence risk. Flagging as informational — worth noting in a follow-up.

---

prefers-reduced-motion

SubNav.module.css has transition: color var(--transition-normal), background-color var(--transition-normal) on .tab. There is no prefers-reduced-motion guard. This carries forward from the original per-component implementations and is a minor consistency gap with newer components. Non-blocking, but worth addressing when the SubNav CSS is next touched.

---

Component Reuse Audit

UI Element	Shared Component	Verdict
Page container + header	PageLayout (new)	New shared component, correctly built for reuse
Sub-navigation tabs	SubNav (new)	Consolidates 4 duplicates; correct shared component
Conditional tab visibility	SubNavTab.visible prop	Generic — properly designed for admin-only tabs

No one-off implementations introduced. The new components are generic and reusable.

---

Findings Summary

Severity	Finding
Medium	Tablet breakpoint upper bound max-width: 1024px in PageLayout.module.css should be max-width: 1023px to avoid overlap with the min-width: 768px tablet range
Informational	1200px / 1400px max-widths in PageLayout are not design tokens; suggest adding --layout-max-width-narrow / --layout-max-width-wide to tokens.css
Informational	BUDGET_TABS and PROJECT_TABS constants duplicated across 5+ files each — extract to client/src/lib/navTabs.ts
Informational	SubNav.module.css .tab transition has no prefers-reduced-motion guard (pre-existing from prior implementations)
Informational	ProfilePage.module.css contains pre-existing hardcoded values not introduced by this PR; track as separate cleanup

The medium finding (tablet breakpoint overlap) is a non-blocking correction — the overlap window is a single pixel at exactly 1024px width and unlikely to produce a visible artifact. Fix before merge or in refinement.

[github-actions]

🎉 This PR is included in version 2.2.0-beta.31 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 2.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀