Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/agent-memory/e2e-test-engineer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
- [bug-1829-shard3-flakes.md](bug-1829-shard3-flakes.md) — root cause + fix for the shard-3 diary flakes blocking main promotions; also proves `maxFailures` already tolerates one retry-passing test (Playwright source citation).
- [story-1876-deposit-refunds.md](story-1876-deposit-refunds.md) — deposit `entryType` refunds; DataTable hidden-column read pattern (`getColumnCellText`), table+card dual-DOM visible-filter pitfall.
- [story-1877-contact-fields-attachment-typing.md](story-1877-contact-fields-attachment-typing.md) — budget-source contact fields, household settings singleton (needs `mode:'serial'`, not just try/finally), document-link attachment typing; route-mock last-registered-runs-first ordering hazard; sandbox lint/tsc caveats.
- [story-1879-report-wizard.md](story-1879-report-wizard.md) — Bank Report Wizard POM/spec; Blocker bug #1886 (budgetSources envelope crash blocks all progress past step 1) + compile errors/missing i18n keys found via `tsc`; source-report E2E seeding pattern (WI budget → invoice-budget-line link).
- [story-1879-report-wizard.md](story-1879-report-wizard.md) — Bank Report Wizard POM/spec; Blocker bug #1886 (budgetSources envelope crash blocks all progress past step 1) + compile errors/missing i18n keys found via `tsc`; source-report E2E seeding pattern (WI budget → invoice-budget-line link); Story #1899 added a 5th step ("Settings" — report language + moved toggles), preview iframe only exists on step 5 now.
- [story-1891-wizard-followup.md](story-1891-wizard-followup.md) — expandable invoice rows, CSP `blob:` frame-src hardened preview check (SUPERSEDED TWICE — `page.frames()` proof, then in-page blob fetch → now header+console-only, see general-e2e-patterns.md), deposit budget-source tagging; 2 filed bugs (#1892 full-exclusion display, #1893 missing deposit-default heuristic); sandbox CAN now build+boot the full container stack (see `sandbox-live-verification.md`) but browser binary download is still network-policy-blocked.
- [sandbox-live-verification.md](sandbox-live-verification.md) — **dhi.io build access is sandbox-dependent, re-check each session**: this session successfully built `cornerstone:e2e` and booted the full container stack (app+OIDC+proxy all healthy), a first — but Playwright's own browser binary download (`playwright.download.prss.microsoft.com`/`cdn.playwright.dev`) is blocked by network policy, and Ubuntu's `chromium-browser` apt package is a non-functional snap stub (no snapd) — no way found yet to get an actual live browser run in this sandbox class.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: story-1879-report-wizard
description: Story #1879 Bank Report Wizard (/budget/reports) — POM/spec authored against a frontend implementation with a Blocker runtime crash (filed #1886); orientations.spec.ts and invoices.spec.ts:395 known-flake fixes confirmed self-healed on beta before this story.
description: Story #1879 Bank Report Wizard (/budget/reports) — POM/spec authored against a frontend implementation with a Blocker runtime crash (filed #1886); orientations.spec.ts and invoices.spec.ts:395 known-flake fixes confirmed self-healed on beta before this story. Story #1899 added a 5th step (Settings — report language + moved toggles).
metadata:
type: project
---
Expand Down Expand Up @@ -168,3 +168,48 @@ with "received 6" on PR #1883 (desktop/tablet/mobile) — PR #1883 added a 6th "
(Household, Areas, ...)") landed in the very next commit (`fc7f6ad2`, PR #1885) and is present at
my branch's HEAD. No new regressions from this triage — see `known-flakes-and-regressions.md` for
the canonical entries; this file only records the fresh confirmation.

## Story #1899 — 5th step inserted ("Settings"), old step 4 renumbered to step 5

`ReportWizardPage.tsx` gained a NEW step 4 ("Settings" — `Step4Settings.tsx`): a report-language
radio group (`input[name="reportLanguage"][value="en"|"de"]`, plain/visible, literal
"English"/"Deutsch" labels NOT wrapped in `t()`) PLUS the `#attachDocuments`/`#includeCoverLetter`
toggles relocated verbatim from the old step 4 (same DOM ids). The old step 4 (preview iframe +
`Step4Options.tsx`) was renamed `Step5Actions.tsx` and pushed to step 5, content byte-identical
minus the toggle props/JSX. **Key consequence for every E2E scenario that used to do
`goNextFromStep3(); waitForPreviewReady();`**: the preview `<iframe>`/`ReportPdfPreview` component
is now ONLY mounted at step 5 — there is no way to observe `previewLoadingOverlay`/
`previewIframe`/`getPreviewSrc()` while on the new step 4. Every such call site became
`goNextFromStep3(); step4NextButton.click(); waitForPreviewReady();` (12 call sites across
`reportWizard.spec.ts` + `reportWizardExpansion.spec.ts`). One consequence: Scenario 1's old
"toggle attach/cover-letter, verify regeneration via `waitForPreviewRegenerated` after EACH
individual toggle" pattern no longer works (nothing to observe on step 4) — simplified to
"toggle all options on step 4 (asserting only checkbox check-state, not regeneration), advance
once, single `waitForPreviewReady()` on step 5". The still-valid multi-step regeneration proof
(`getPreviewSrc()` before, `waitForPreviewRegenerated(previousSrc)` after) now requires reaching
step 5 fully (via step 4) each time, and going back now needs TWO `goBack()` calls (step 5 → step
4 → step 3) instead of one — `goBack()` itself needed NO change since it's a generic
`[class*="buttonRow"] [class*="btnSecondary"]` re-query (works for whichever step is currently
mounted; only one `buttonRow` div is ever in the DOM at a time across all 5 steps, confirmed via
grep — this is also why `step2BackButton`/`step3BackButton`/`step4BackButton` are literally the
same object reference, and `.first()`/`.last()` are never load-bearing disambiguators, just
defensive tie-breakers).

Added `reportLanguageRadio(lang)`/`selectReportLanguage(lang)` (POM) and a new Scenario 12
(`reportWizard.spec.ts`) proving a `reportLanguage` change on step 4 regenerates the preview
(blob src change, via `waitForPreviewRegenerated`) while the wizard's OWN chrome (heading,
stepper labels, Back/Next button text) stays English — `reportLanguage` only feeds
`generateReportPdf`'s fixed-locale `i18n.getFixedT(reportLanguage, 'budget')`/`createFormatters()`
pair, completely independent of the app's `resolvedLocale`/normal `t`. Deliberately did NOT
assert on PDF byte content (that's the Jest `realRender.test.ts` unit test's job per the QA
spec) — only the blob-src-changed proof + suggestedFilename pattern (still untranslated:
`${useCase}-${slug}-${date}.pdf`, confirmed unaffected).

Verified via `npx eslint`, `npx prettier --check`, `npx tsc --noEmit -p e2e/tsconfig.json`
(zero errors referencing ReportWizard files; the ~123 pre-existing errors in ~22 unrelated files,
mostly `capture-docs-screenshots.spec.ts`'s `TestDetails` type mismatch, are the same
sandbox/stale-@playwright/test-version noise as prior stories — confirm this is still true each
session rather than assuming). No live Playwright run — browser binary download is blocked by
network policy in this sandbox (`cdn.playwright.dev`/`playwright.download.prss.microsoft.com`
both 403 "blocked by network policy: domain ... — no matching allow rule"), consistent with
`sandbox-live-verification.md`'s finding. CI's full E2E matrix is the real verification gate.
2 changes: 1 addition & 1 deletion .claude/agent-memory/ux-designer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
- `role="status"` already implies `aria-live="polite"` — never add both attributes.
- `BadgeVariantMap` entries need both `label` and `className` — a missing `className` makes the variant's style dead on arrival.
- New component reuse audit: check `Badge`, `SearchPicker`, `Modal`, `Skeleton`, `EmptyState`, `FormError` before proposing anything new; see [component-patterns.md](component-patterns.md) for established section-card/badge/picker conventions.
- Cannot `gh pr review --request-changes` **or** `--approve` on your own PRs (GraphQL rejects both, not just request-changes) — post a `gh pr comment` instead, with an explicit "Verdict: APPROVED/CHANGES_REQUIRED" line in the body. If a specific invocation is blocked by the sandbox's permission system, treat that as a real denial: report it to the orchestrator and let the human-configured policy decide — do not engineer alternative invocations to route around a block.
- Cannot `gh pr review --request-changes` **or** `--approve` on your own PRs (GraphQL rejects both, not just request-changes) — post a `gh pr comment` instead, with an explicit "Verdict: APPROVED/CHANGES_REQUIRED" line in the body. This block is keyed to the underlying `gh` auth identity, not the PR's displayed `author.login` — a PR showing a human GitHub username (e.g. the repo owner) as author still hit "Can not approve your own pull request" (PR #1903), so don't skip the self-review check just because `gh pr view --json author` shows a person's name. If a specific invocation is blocked by the sandbox's permission system, treat that as a real denial: report it to the orchestrator and let the human-configured policy decide — do not engineer alternative invocations to route around a block.
- No segmented-control component exists in the app — for a "choose one of N" form field that isn't a `<select>`, reuse the plain `<input type="radio">` + `role="group"` pattern from `AutoItemizePage.module.css` `.modeSelector` (the only radio group in the codebase) rather than inventing a segmented/button-group look. See Issue #1876 in [feature-spec-history.md](feature-spec-history.md).
- For a "this is a negative/refund/adjustment row" signal, don't rely on red text color alone — pair it with a `Badge` carrying a real text label (reuse `--color-status-blocked-bg`/`-text`, the existing red badge pair — there's no separate `--color-danger-badge-*` family) plus `formatCurrency(-amount)`'s literal minus sign. Don't recolor a _computed remainder_ (e.g. "remaining amount") red just because refunds reduce it — that falsely implies an error state; only the actual refund row gets the negative treatment.
- `Intl.NumberFormat` enables thousands-grouping by default; `toFixed()`/manual string-building never did. When a PR swaps a `toFixed()` call for a locale formatter, script-verify the _old vs new_ output isn't just per-locale-correct but also identical for large values (≥1000) — grouping separators are an easy-to-miss 3rd English-output regression beyond whatever the PR explicitly discloses. See PR #1845 in [pr-review-findings.md](pr-review-findings.md).
Expand Down
10 changes: 10 additions & 0 deletions .claude/agent-memory/ux-designer/feature-spec-history.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,13 @@ First spec written for `client/src/lib/reportPdf/overviewPdf.ts` — a `pdfmake`
- Spec posted on issue #1804; implemented in PR #1834 (approved — see [pr-review-findings.md](pr-review-findings.md))
- Section card above `.toolbar`, `<dl>`/`<dt>`/`<dd>` status rows, new Badge `.success` variant, `Skeleton` + `sharedStyles.bannerError` for loading/error states
- Mobile: `.schedulerStatusRow` → `flex-direction: column` at `<768px`

## Issue #1899 — Bank Report Wizard: Settings step (5th step, report language)

Part of the Bank Report Wizard Refinement Round 2 mini-epic (follows #1898). Spec posted on the issue; not yet implemented.

- **Report language picker**: reused the `AutoItemizePage.module.css` `.modeSelector` radio-group pattern again (2nd confirmed use after #1876's deposit entry-type picker — now a firmly established precedent, not a one-off). Explicitly rejected a `<select>` even though `ProfilePage.tsx` already has a language `<select>` for the app-wide locale (3 options incl. "System") — for exactly 2 always-visible options inside a wizard step, a radio group is more scannable than a dropdown open/close interaction.
- **Language labels are literal untranslated strings** `"English"`/`"Deutsch"` (not `t()`-wrapped) — precedent is `ProfilePage.tsx:389-390`'s existing app-locale `<select>`, which does the same for the same reason (autonyms, not translated demonyms). Called this out explicitly in the spec so a PR reviewer doesn't flag it as a missed-i18n-key bug.
- **Focus ring decision**: deliberately did NOT add a custom `box-shadow: var(--shadow-focus)` ring to the new radio group — kept the native browser `:focus-visible` outline to stay consistent with the sibling `.optionCheckbox` controls in the very same step, which also rely on the native outline (no custom ring exists there today). Don't treat "every new control must get a custom focus ring" as an absolute rule — matching the immediate visual siblings can be the more consistent choice.
- **Found a real responsive risk while auditing `WizardStepper` for a 5th step**: the desktop `<nav>` stepper (`WizardStepper.module.css` `.stepper`) has no `overflow-x` handling and the mobile dot-view only activates at `max-width: 767px`. Going from 4→5 full-text-label steps pushes the natural row width past a typical ~700px tablet content area well before that 767px cutoff — an `768px`-vs-`767px` off-by-one boundary that was invisible at 4 steps but becomes real at 5. Fix specified: a `768px–1023px` tablet-range media query shrinking `.stepList` gap, `.connector` width, and `.label` font-size (not just relying on the existing mobile/desktop binary). **Any future story that adds another step to an existing `WizardStepper` instance should re-check this same boundary** — it's a per-step-count risk, not a one-time fix.
- Confirmed (via grep) `--breakpoint-*` tokens do not exist anywhere in `tokens.css` — see [token-reference.md](token-reference.md) for the permanent note; don't reference them in future specs.
1 change: 1 addition & 0 deletions .claude/agent-memory/ux-designer/token-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ metadata:
- `--color-success-badge-bg`/`-text`: light = green-100/green-900 (#d1fae5/#065f46); dark = rgba(16,185,129,0.15)/emerald-200 (#a7f3d0) — both high contrast, same pairing as "Completed"/"Arrived" status badges
- `--color-warning-bg` EXISTS (`#fff7ed`, dark: `rgba(251,146,60,0.1)`) — use for warning banners
- `--spacing-xs` / `--spacing-sm` are NOT valid tokens — use `--spacing-1` through `--spacing-16`
- `--breakpoint-sm/md/lg/xl` tokens do **NOT exist** in `tokens.css` at all (grepped, confirmed empty), despite being referenced as if real in the agent's own system-prompt instructions. The codebase's actual, consistent convention is raw pixel media queries per component (e.g. `WizardStepper.module.css` uses `max-width: 767px` for its mobile cutoff, `ReportWizardPage.module.css` uses `max-width: 860px` for its grid collapse). Don't invent `var(--breakpoint-*)` references in specs or flag their absence as a PR violation — follow the raw-pixel convention instead. See Issue #1899.
- `--color-danger-text` = white (text ON danger bg) — NEVER use as border or text on `--color-danger-bg`; use `--color-danger-border` for border, `--color-danger-text-on-light` for red text on light bg
- Budget bar, Gantt, and milestone tokens already exist in tokens.css — check before specifying new domain-specific colors

Expand Down
16 changes: 16 additions & 0 deletions client/src/components/WizardStepper/WizardStepper.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,22 @@
}
}

/* ---- Tablet Breakpoint ---- */

@media (min-width: 768px) and (max-width: 1023px) {
.stepList {
gap: var(--spacing-2);
}

.connector {
width: 20px;
}

.label {
font-size: var(--font-size-xs);
}
}

/* ---- Mobile Breakpoint ---- */

@media (max-width: 767px) {
Expand Down
5 changes: 5 additions & 0 deletions client/src/i18n/de/budget.json
Original file line number Diff line number Diff line change
Expand Up @@ -1107,10 +1107,15 @@
"useCase": "Berichtsart",
"source": "Budgetquelle",
"invoices": "Rechnungen auswählen",
"settings": "Einstellungen",
"options": "Vorschau & Export"
},
"stepperAriaLabel": "Schritte des Berichtsassistenten",
"mobileStepLabel": "Schritt {{current}} von {{total}}",
"settingsStep": {
"languageHeading": "Berichtssprache",
"languageHelper": "Betrifft nur den exportierten Bericht – die Sprache der App bleibt unverändert."
},
"useCaseLabel": "Welchen Bericht benötigen Sie?",
"useCase": {
"budget-overview": "Budgetübersicht",
Expand Down
5 changes: 5 additions & 0 deletions client/src/i18n/en/budget.json
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,7 @@
"useCase": "Report Type",
"source": "Budget Source",
"invoices": "Select Invoices",
"settings": "Settings",
"options": "Preview & Export"
},
"stepperAriaLabel": "Report wizard steps",
Expand Down Expand Up @@ -1137,6 +1138,10 @@
"emptyInvoices": "No invoices found",
"emptyInvoicesDescription": "This budget source has no invoices for this report type",
"selectAtLeastOne": "Select at least one invoice to proceed",
"settingsStep": {
"languageHeading": "Report language",
"languageHelper": "Only affects the exported report — your app language stays the same."
},
"attachDocuments": "Attach invoice PDFs",
"attachDocumentsHelper": "Appends each selected invoice's source PDF as an appendix",
"includeCoverLetter": "Include cover letter",
Expand Down
Loading
Loading