chore: create FormError and SignatureSection shared components (#862)

[steilerDev]

Summary

• Created generic FormError component with banner and field-level error variants
• Extracted SignatureSection from DiaryEntryForm (eliminated 2x duplication)
• Migrated BudgetLineForm, InvoiceLinkModal, and DiaryEntryForm to use FormError
• Migrated DiaryEntryForm's daily_log and site_visit signature blocks to SignatureSection
• Removed duplicated error and signature CSS from migrated components

Fixes #862

Test plan

• 23 unit tests across 2 test files (FormError: 10, SignatureSection: 13)
• Quality Gates CI pass
• Form error display and signature behavior unchanged

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[steilerDev]

[security-engineer] PR #863 security review — FormError and SignatureSection shared components.

No security findings.

Both new components render user-supplied strings exclusively as React JSX text nodes — no dangerouslySetInnerHTML, innerHTML, or eval() usage anywhere. React escapes all content at render time; XSS risk is zero for both components.

Specific checks:

• FormError.tsx: {message} rendered as a text node inside a plain <div>. The role="alert" on the banner variant is a positive accessibility addition with no security implication.
• SignatureSection.tsx: {label} rendered as a JSX text node inside <label>. All SignatureCapture props (signerName, index) pass through React's controlled rendering pipeline.
• CSS: all styling uses design tokens via CSS Modules — no user data in style props, no CSS injection vector.
• No new API endpoints, authentication, or authorization logic introduced.
• No new npm dependencies.

Checklist:

• No SQL/command/XSS injection vectors in new code
• Authentication/authorization enforced on all new endpoints (N/A — frontend only)
• No sensitive data exposed in logs, errors, or client responses
• User input validated and sanitized at API boundaries (N/A — display-only components)
• New dependencies have no known CVEs (no new dependencies)
• No hardcoded credentials or secrets
• CORS configuration remains restrictive (unchanged)
• Error responses do not leak internal details (N/A)

[steilerDev]

[product-architect]

Architecture Review

Clean extraction of two shared components with proper structure. No schema, API, or deployment changes -- purely client-side refactoring.

Verified

• Component Reuse Policy: FormError was already declared in CLAUDE.md as a required shared component -- this PR delivers the implementation. Correct placement in client/src/components/FormError/ with barrel export.
• Design tokens: All CSS values in both new components use design tokens from tokens.css. No hardcoded values.
• File naming: PascalCase for React components, barrel index.ts exports -- consistent with project conventions.
• Migration correctness: BudgetLineForm, InvoiceLinkModal, and DiaryEntryForm all correctly replaced inline error markup with <FormError>. The role="alert" accessibility attribute is preserved for banner variant and correctly omitted for field variant.
• SignatureSection extraction: Eliminates exact duplication between daily_log and site_visit signature blocks in DiaryEntryForm. CSS styles match the original nested selectors (explicitly specifying font-weight, font-size, color since CSS Modules scope prevents inheritance).
• Tests: 23 tests covering both components. Dynamic import pattern for SignatureSection correctly handles canvas dependency mocking.

Observations (informational, not blocking)

1. Import ordering in FormError.tsx: The FormErrorProps interface is declared before the import styles statement (line 1 vs line 10). Convention is imports-first. Minor style nit.

2. onSignaturesChange dual-callback in SignatureSection: The handleSignatureUpdate calls both onSignatureChange and onSignaturesChange if both are provided -- this could cause double state mutations if a future caller provides both callbacks that update the same state. Current usage only passes onSignatureChange so this is safe today. Worth a JSDoc note on the interface clarifying mutual exclusivity or intended use.

3. Remaining error banner duplication: There are ~57 files still using inline errorBanner/fieldError patterns. This PR covers the three migrated components -- the remaining migration is expected to be incremental (per the issue scope).

4. Unrelated file: .claude/agent-memory/ux-designer/story-15-4-invoice-budget-lines.md is included in the diff. Not harmful but unrelated to the chore.

No architectural concerns. Approve from architecture perspective.

[steilerDev]

[ux-designer]

Design system review for PR #863 — FormError and SignatureSection shared components.

Token Adherence — PASS

Both new CSS module files use only valid Layer 2 semantic tokens throughout. No hardcoded hex values, no raw pixel/rem literals for spacing/radius/font sizes.

FormError.module.css:

• var(--color-danger-bg) / var(--color-danger-border) / var(--color-danger-active) / var(--radius-md) / var(--font-size-sm) / var(--font-size-2xs) / spacing tokens — all valid.

SignatureSection.module.css:

• var(--spacing-6) / var(--spacing-4) / var(--spacing-3) / var(--color-border) / var(--font-weight-medium) / var(--font-size-sm) / var(--color-text-secondary) / var(--radius-md) / var(--color-bg-secondary) — all valid.

Dark Mode — PASS

All color values use CSS custom properties that are overridden in [data-theme="dark"]. Both new components will theme automatically. No component-level dark mode overrides needed.

Note: --color-danger-active in dark mode resolves to var(--color-red-500) (#dc2626), which is consistent with the pre-existing error styling that was extracted into this component — not a regression.

Visual Consistency — PASS

The FormError banner padding (var(--spacing-2-5) var(--spacing-3)) and margin-bottom (var(--spacing-4)) match the more fully-specified InvoiceLinkModal error banner. The prior BudgetLineForm error banner had slightly different padding (var(--spacing-3) on all sides) — the new unified version is correct to adopt the more complete spec.

SignatureSection.module.css .label rule exactly matches DiaryEntryForm's .label (font-size-sm, font-weight-medium, color-text-secondary) — consistent extraction.

CSS Cleanup — PASS

Duplicate .errorBanner/.fieldError blocks removed from BudgetLineForm.module.css and InvoiceLinkModal.module.css. Duplicate signature CSS removed from DiaryEntryForm.module.css. Clean deduplication with no orphaned rules.

---

Findings

Medium — Accessibility: <label> without htmlFor in SignatureSection

File: client/src/components/diary/SignatureSection/SignatureSection.tsx line 46

<label className={styles.label}>{label}</label>

A <label> element without a htmlFor attribute has no programmatic association with any form control. Screen readers may announce it unexpectedly (some will expose it as a standalone label; others may skip or mis-associate it). Since this is a section heading — not a label for a single input — the correct semantic is either:

• Replace with a <span> or <p> styled the same way (simplest fix)
• Or wrap the entire section in a <fieldset> with <legend> (semantically richer for a group of related inputs)

The <label> element is reserved for form control associations. Using it purely as a styled heading is a semantic misuse that can produce unexpected AT behavior.

Recommended fix:

// Option A — span (simplest)
<span className={styles.label}>{label}</span>

// Option B — fieldset/legend (semantically fullest)
<fieldset className={styles.signatureSection}>
  <legend className={styles.label}>{label}</legend>
  ...
</fieldset>

The CSS .label rule needs no changes — display: block already handles the block rendering that .label provides.

Informational — "Add Signature" button text includes raw + character

File: client/src/components/diary/SignatureSection/SignatureSection.tsx line 66

+ Add Signature

The + character at the start of the button text will be read aloud by most screen readers (as "plus Add Signature"). If this matches the existing pattern in the codebase this is acceptable, but consider using an explicit aria-label or a visually-hidden prefix to avoid the literal "plus":

<button ... aria-label="Add signature">
  + Add Signature
</button>

This matches the pattern used on other "+" action buttons in the app.

---

Summary

The refactoring correctly creates two reusable shared components with full design token adherence and automatic dark mode support. The <label> semantic issue in SignatureSection is the only substantive finding — it does not affect visual rendering but does create an accessibility concern for assistive technology users.

[github-actions]

🎉 This PR is included in version 1.16.0-beta.28 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀