feat(dashboard): responsive mobile sections & accessibility polish#718
feat(dashboard): responsive mobile sections & accessibility polish#718steilerDev merged 3 commits intobetafrom
Conversation
…chart fallbacks
- Mobile (<768px): group cards into collapsible sections using <details>/<summary>
- Primary cards (budget summary, alerts, invoices, quick actions) always visible
- Timeline and Budget Details sections collapsed by default with summary text
- Add role="region", aria-label, aria-live="polite" to dashboard grid
- Add role="img" and dynamic aria-label to MiniGanttCard SVG
- Add sr-only utilization percentage text to SourceUtilizationCard
- Section summary shows loading state ("…") and data-driven counts
- Chevron rotation animation with prefers-reduced-motion guard
Fixes #478
Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
…nd chart fallbacks - DashboardPage: verify ARIA region landmark, aria-live, mobile <details> elements, timeline/budget summary text in various states - MiniGanttCard: verify SVG role="img" and aria-label with item counts - SourceUtilizationCard: verify sr-only utilization percentage text Fixes #478 Co-Authored-By: Claude qa-integration-tester (Haiku 4.5) <noreply@anthropic.com>
…dPage tests DashboardPage renders cards in both a desktop grid and a mobile sections layout simultaneously. In jsdom, CSS media queries don't apply, so both layouts are in the DOM and every card appears twice. Switch all singular getByRole/getByText queries for card elements to getAllByRole/getAllByText[0], and update the loading skeleton count from 7 to 14 (7 cards × 2 layouts). Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.15.0-beta.10 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
steilerDev
left a comment
There was a problem hiding this comment.
[security-engineer] PR #718 security review — responsive mobile sections, ARIA landmarks, and accessibility polish.
Scope
Frontend-only accessibility and responsive layout changes across:
DashboardPage.tsx— mobile collapsible sections, ARIA region landmarksMiniGanttCard.tsx— SVGrole="img"+aria-labelSourceUtilizationCard.tsx— sr-only utilization percentage span- CSS modules — mobile layout, sr-only utility class
Security Checklist
- No SQL/command/XSS injection vectors in new code
- Authentication/authorization enforced on all new endpoints — N/A (frontend-only)
- No sensitive data (secrets, tokens, PII) exposed in logs, errors, or client responses
- User input validated and sanitized at API boundaries — N/A (no new inputs)
- New dependencies have no known CVEs — no new dependencies introduced
- No hardcoded credentials or secrets
- CORS configuration remains restrictive — unchanged
- Error responses do not leak internal details — N/A
Findings
No security issues identified.
Dynamic content review:
getTimelineSummary()andgetBudgetDetailsSummary()— produce strings derived fromArray.length(integer counts). No user-controlled string content reaches the DOM from these helpers.MiniGanttCardaria-label— constructed fromfilteredWorkItems.lengthandvisibleMilestones.length(integers). No user-supplied data in the label.SourceUtilizationCardsr-only span — computes(usedAmount / totalAmount * 100).toFixed(0)from numeric API fields. Division-by-zero guarded by thetotalAmount > 0conditional.- The
›chevron is a JSX text node literal — no HTML injection. emptyAction.hrefvalues ('/budget/sources','/budget/subsidies') are hardcoded string literals inCARD_DEFINITIONS, not user-supplied. No open redirect vector introduced.- All ARIA attributes (
role,aria-label,aria-live,aria-atomic) use hardcoded string literals. - Zero
dangerouslySetInnerHTML,innerHTML, oreval()usage in changed files.
The duplicate-element test pattern (getAllByRole(...)[0]) introduced to accommodate both desktop grid and mobile sections rendering the same cards is a test correctness concern, not a security concern.
Verdict: No blocking security issues. PR is clear to merge from a security perspective.
steilerDev
left a comment
There was a problem hiding this comment.
[ux-designer]
PR #718 — Design Review: feat(dashboard): responsive mobile sections & accessibility polish
Reviewed files: DashboardPage.tsx, DashboardPage.module.css, MiniGanttCard.tsx, MiniGanttCard.module.css, SourceUtilizationCard.tsx, SourceUtilizationCard.module.css (test files excluded from design review scope).
Summary
Overall this is a well-executed accessibility and responsive polish PR. Token adherence is strong throughout all three CSS modules — every color, spacing, radius, shadow, and transition value references a design token. The prefers-reduced-motion guard, the 44px minimum touch targets, and the correct --shadow-focus focus ring pattern are all in place. The MiniGanttCard SVG accessibility improvement (role="img" + dynamic aria-label) and the srOnly utilization percentage on SourceUtilizationCard are exactly right.
Two medium findings and two informational notes below.
Findings
Medium — Duplicate ARIA landmark label creates ambiguity for screen readers
File: DashboardPage.tsx lines 416–424 (desktop grid) and 427 (mobile sections div)
Both the desktop grid and the mobile sections container carry identical ARIA landmarks:
<!-- desktop -->
<div role="region" aria-label="Dashboard overview" aria-live="polite" aria-atomic="false" ...>
<!-- mobile -->
<div class="mobileSections" role="region" aria-label="Dashboard overview">At any viewport width, both elements exist in the DOM (one is display:none, the other is visible). Screen readers that enumerate landmarks by label will announce "Dashboard overview" twice in the landmark list, which is confusing. The ARIA spec does not exclude hidden (display:none) elements from the landmark tree in all assistive technology implementations.
Fix: Give the mobile landmark a distinct label, or add aria-hidden="true" to whichever section is currently hidden via CSS. A straightforward approach is to differentiate the label:
// mobile sections container
<div className={styles.mobileSections} role="region" aria-label="Dashboard overview (mobile)">Alternatively, conditionally render only one landmark and let JS toggle between them based on viewport — though the CSS-only show/hide approach is simpler if the label is made distinct.
Medium — details[open] combinator in CSS Module uses a global element selector
File: DashboardPage.module.css line 182
details[open] > .sectionSummary > .sectionChevron {
transform: rotate(90deg);
}The details[open] portion of this selector is a global element+attribute selector. While the hashed class names on .sectionSummary and .sectionChevron prevent actual style leakage, relying on the global details element selector inside a CSS Module is a non-standard pattern. If a <details> element with the .sectionSummary class were used in a different context without the open attribute, this rule would not fire correctly, and tooling linters (e.g., stylelint-no-global-selectors) may flag it.
Preferred fix: Use a companion CSS class toggled in React state or a data-open attribute to drive the chevron rotation — keeping the CSS Module fully self-contained:
// In TSX, track open state per section, or use a CSS custom approach:
<details className={`${styles.sectionDetails} ${isOpen ? styles.sectionOpen : ''}`}>.sectionOpen > .sectionSummary > .sectionChevron {
transform: rotate(90deg);
}However, if the codebase's linting config does not flag this pattern, this is a low-priority concern — the details[open] approach is semantically correct and idiomatic HTML. Downgrading to Low if the lint config does not enforce CSS Module purity on element selectors.
Informational — aria-live absent on the mobile landmark
File: DashboardPage.tsx line 427
The desktop grid has aria-live="polite" aria-atomic="false" to announce data loading changes. The mobile sections container does not. When the page is loaded at mobile viewport widths, the mobile container is the visible landmark, but data-load announcements will not fire from it (the hidden desktop grid's live region still fires, but assistive technology behavior for announcements from display:none elements is browser-dependent and unreliable).
Recommendation: Apply the same aria-live="polite" aria-atomic="false" to the mobile sections container for consistent announcement behavior across breakpoints.
Informational — min-width: 200px on .customizeDropdown is a hardcoded value
File: DashboardPage.module.css line 68
.customizeDropdown {
min-width: 200px;
...
}No spacing token maps to 200px (the largest token is --spacing-16 at 64px). This is an acceptable deviation for a component-specific layout constraint with no token equivalent. Document the rationale in a comment if challenged during future audits.
What Was Verified Correct
- All color values in all three CSS modules use
var(--...)semantic tokens — no hardcoded hex or rgb values - All spacing values use token references (
--spacing-*) - All border-radius values use
--radius-mdor--radius-fullas appropriate - All transitions use
--transition-normalor--transition-button-border— correct token usage prefers-reduced-motionguard is present and correctly scoped to.sectionChevron(line 206–210)--shadow-focusused on all interactive elements (.customizeButton,.sectionSummary,.reEnableButton,.containerin MiniGanttCard) — nooutline: 2px solid var(--color-primary)anti-pattern- Breakpoint boundaries:
min-width: 768pxandmax-width: 1023pxfor tablet (correct upper bound — avoids 1024px overlap bug noted in prior reviews) - Mobile breakpoint:
max-width: 767px(correct) - Touch targets: all interactive elements have
min-height: 44px - Dark mode: all colors resolve through semantic tokens that have Layer 3 dark overrides — no hardcoded light-mode-only values
- SVG colors in
MiniGanttCardare correctly read viagetComputedStyle+readCssVar()to handle dark mode switching —MutationObserverondata-themeensures re-render on theme change aria-hidden="true"on the chevron›character — correct (decorative punctuation)role="img"+ dynamicaria-labelon MiniGanttCard SVG — correct and well-formed label contentsrOnlyhelper in bothDashboardPage.module.cssandSourceUtilizationCard.module.css— correct implementation;shared.module.cssdoes not yet expose a shared.srOnlyclass so local definitions are the established patternlist-style: noneon.sectionSummaryremoves default disclosure triangle in non-WebKit;::-webkit-details-marker { display: none }handles WebKit — both are presentaria-haspopup="menu"+aria-expandedon the customize button — correct pairingrole="menu"on dropdown +role="menuitem"on buttons — correct ARIA menu pattern
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect] Architecture review of PR #718 — Responsive mobile sections, ARIA landmarks, and accessibility polish for the Dashboard.
Verdict: APPROVE (posted as comment due to GitHub self-review limitation)
Verified
Architecture compliance — CSS Modules pattern followed correctly. All new styles use design tokens (var(--color-*), var(--spacing-*), var(--font-size-*), etc.) with zero raw hex values. New CSS classes are scoped to their respective .module.css files as expected.
Dual-render approach — The desktop grid and mobile sections are both rendered in the DOM, with CSS display: none toggling visibility at the mobile breakpoint. This is architecturally sound for this scale: both trees share the same parent state (no duplicate API calls), and the extra DOM nodes are negligible for a dashboard with 8 cards. The alternative (media query hooks in JS) would add complexity without meaningful benefit.
ARIA patterns — Correct usage:
role="region"+aria-labelon both grid containers provides landmark navigationaria-live="polite"+aria-atomic="false"on the desktop grid correctly announces individual card changes without re-reading the entire region- SVG
role="img"+ descriptivearia-labelon MiniGanttCard follows WAI best practices role="button"+tabIndex={0}+onKeyDown(Enter/Space) on MiniGanttCard correctly implements the button pattern for a clickable div<details>/<summary>for mobile collapsible sections is semantically correct and provides built-in keyboard accessibilityaria-hidden="true"on the decorative chevron is correctprefers-reduced-motiondisables the chevron transition appropriately
Test quality — Tests cover all new features: SVG accessibility attributes, sr-only content, ARIA landmarks, mobile section summaries (with data and empty states). The switch from getByRole to getAllByRole(...)[0] throughout existing tests is a necessary consequence of the dual-render approach and is correctly applied.
Code quality — The renderCard helper eliminates duplication between the two render paths. The DashboardSection type and section field on CARD_DEFINITIONS is a clean way to partition cards. The getTimelineSummary and getBudgetDetailsSummary helper functions are well-extracted.
Findings
LOW: Duplicated .srOnly CSS class — The .srOnly utility class is now defined in 4 separate .module.css files (SourceUtilizationCard, DashboardPage, LinkedDocumentsSection, CostBreakdownTable). This is a pre-existing pattern, not introduced by this PR, but worth noting: a shared utility class in client/src/styles/utilities.css (composed via CSS Modules composes:) would reduce duplication. Not blocking — this is a future cleanup item.
INFORMATIONAL: Mobile section has no aria-live — The desktop grid has aria-live="polite" but the mobile .mobileSections region does not. This is acceptable because the mobile layout uses <details> elements where content is user-initiated (toggle open/close), so live-region announcements are unnecessary. Just confirming this was intentional.
Approved — clean, well-structured accessibility work that follows established patterns.
steilerDev
left a comment
There was a problem hiding this comment.
[product-owner] PR Review for Story #478: Responsive, Dark Mode & Accessibility Polish
AC-by-AC Verification
| AC | Description | Verdict |
|---|---|---|
| #1 | Desktop 3-column grid (>=1024px) | PASS — pre-existing CSS grid, unchanged |
| #2 | Tablet 2-column grid (768-1023px) | PASS — pre-existing CSS grid, unchanged |
| #3 | Mobile primary cards visible (<768px) | PASS — budget-summary, budget-alerts, invoice-pipeline, quick-actions in always-visible mobileSection |
| #4 | Secondary cards collapsed behind section headers | PASS — Timeline and Budget Details use <details> elements, collapsed by default |
| #5 | Collapsed headers show summary text | PASS — getTimelineSummary() / getBudgetDetailsSummary() produce data-driven one-line summaries |
| #6 | Dark mode correctness | PASS — all CSS uses design system tokens (--color-border, --color-bg-secondary, etc.) |
| #7 | ARIA landmark roles on sections | PASS — both desktop grid and mobile sections have role="region" with aria-label="Dashboard overview" |
| #8 | ARIA live regions for loading/data-refresh | PARTIAL FAIL — desktop grid has aria-live="polite" but the mobile sections <div> does NOT. Screen reader users on mobile viewports will not receive state change announcements |
| #9 | Tab order follows logical reading order | PASS — DOM order is left-to-right, top-to-bottom; no tabIndex manipulation |
| #10 | Visible focus indicators (WCAG 2.1 AA) | PASS — .sectionSummary:focus-visible uses box-shadow: var(--shadow-focus); MiniGanttCard container already has :focus-visible |
| #11 | Chart text fallbacks for screen readers | PASS — MiniGanttCard SVG has role="img" + descriptive aria-label; SourceUtilizationCard has sr-only percentage text; WorkItemProgressCard (donut) already had role="img" + aria-label |
Blocking Issue
AC #8 — Mobile sections missing aria-live="polite"
The mobile sections container at line ~731 of DashboardPage.tsx:
<div className={styles.mobileSections} role="region" aria-label="Dashboard overview">needs aria-live="polite" to match the desktop grid's accessibility behavior. Without it, screen reader users on mobile will not be notified when data loads or refreshes.
Fix: Add aria-live="polite" aria-atomic="false" to the mobile sections <div>, matching the desktop grid's attributes.
Non-Blocking Observations
-
Duplicate
.srOnlyclass — defined in bothDashboardPage.module.cssandSourceUtilizationCard.module.css. Consider extracting to the shared CSS utilities (client/src/styles/shared.module.css) in a refinement pass. -
prefers-reduced-motionguard — the chevron rotation transition has aprefers-reduced-motion: reduceoverride (good). However,aria-liveannouncements are unaffected by this media query, which is correct behavior. -
Test duplication from dual-layout DOM — the third commit correctly fixes all queries to use
getAllBy...[0]and doubles the skeleton count (7 to 14). This is a consequence of rendering both layouts simultaneously in jsdom. If the dual-layout approach leads to further test maintenance burden, consider lazy-rendering the mobile layout only when amatchMediahook signals mobile viewport.
Test Authorship
Verified: production code committed with frontend-developer (Haiku 4.5) trailer; test code committed with qa-integration-tester (Haiku 4.5) and qa-integration-tester (Sonnet 4.5) trailers. PASS.
CI Status
All quality gates green (Static Analysis, Tests 1-3/3, Docker, E2E Smoke). PASS.
Verdict
Request Changes — AC #8 is partially unmet. The fix is a single attribute addition. Once applied, all 11 ACs will pass and the PR can be approved.
Adds aria-live="polite" and aria-atomic="false" to the mobile sections container to match the desktop grid's ARIA attributes, ensuring screen reader users on mobile viewports receive state change announcements. Fixes review finding from product-owner on PR #718. Co-Authored-By: Claude Haiku (frontend-developer) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds aria-live="polite" and aria-atomic="false" to the mobile sections container to match the desktop grid's ARIA attributes, ensuring screen reader users on mobile viewports receive state change announcements. Fixes review finding from product-owner on PR #718. Co-Authored-By: Claude Haiku (frontend-developer) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds aria-live="polite" and aria-atomic="false" to the mobile sections container to match the desktop grid's ARIA attributes, ensuring screen reader users on mobile viewports receive state change announcements. Fixes review finding from product-owner on PR #718. Co-authored-by: Claude product-architect (Opus 4.6) <noreply@anthropic.com>
Summary
<details>/<summary>sectionsrole="region",aria-label,aria-live="polite") to dashboard gridrole="img"and dynamicaria-labelto MiniGanttCard SVGFixes #478
Test plan
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Co-Authored-By: Claude frontend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Haiku 4.5) noreply@anthropic.com