AccountingNext/UI_IMPROVEMENT_PLAN.md
Claudio Schaad 766295a119 PR U-8: format the Transactions delete-confirmation amount
Route transaction.Value through ToFormattedString() so the confirmation
reads "Betrag 1'234.57" instead of "Betrag 1234.567", and fix the
"Transaction" → "Transaktion" typo. One-file change — the other four
CRUD pages don't put values into confirmations.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-07-04 10:06:44 +02:00

198 lines
11 KiB
Markdown

# UI improvement plan
Analysis produced 2026-07-03. Focused on Blazor UI concerns; the `IMPROVEMENT_PLAN.md` covered service/repository layers.
## 1. Real bugs
### 1.1 `TransactionDialog.SaveAsync` will NRE if the date picker is cleared
`TransactionDialog.razor.cs:37-40`:
```csharp
if (editContext.Validate())
{
Content.ValueDate = SelectedValue!.Value; // SelectedValue is DateTime?
Content.BookingDate = SelectedValue!.Value;
...
}
```
`SelectedValue!.Value` throws `InvalidOperationException` if the user opened the dialog, cleared the date, and hit Speichern.
### 1.2 `TransactionSplitDialog` only validates the first split row
`TransactionSplitDialog.razor.cs:50` builds an `EditContext` around the *first* transaction only. When the user adds a split row, subsequent rows have no `EditContext` — `editContext.Validate()` in `SaveAsync` returns true for the first row and completely ignores the rest. Empty text / wrong account on split rows never fails validation.
### 1.3 `AccountSelector` doesn't react to `Accounts` arriving late
`AccountSelector.razor.cs:17-20`:
```csharp
protected override void OnInitialized()
{
SelectedAccount = Accounts.FirstOrDefault(account => account.Id == AccountId);
}
```
`OnInitialized` fires once. If the parent's `Accounts` list is empty at that point (the async path introduced in PR P) and hydrates later, `SelectedAccount` stays `null` and the typeahead shows no preselection. Should be `OnParametersSet`.
### 1.4 The whole "year/mandator via `forceLoad`" architecture
`MyHeader.YearChanged` calls `NavigateTo(..., forceLoad: true)`. `Home.SplitBankTransactionAsync` and `Home.ReloadPage()` do the same after any successful split. `SettingsService` is `Singleton` (documented in `Extensions.cs`) *because* the state has to survive that reload.
Symptoms:
- **Full HTTP round-trip** on every year/mandator switch, every split save. Tears down the SignalR circuit, blows away the scoped `RepositoryCache`, re-runs every repo constructor.
- **No URL-bookmarkable state** — you can't send yourself a link to "2023 / Mandator X".
- **Multi-user hazard** (already documented — single-user in practice, but the design is fragile if that ever changes).
- **Client-side state loss** — filters, scroll positions, half-typed inputs.
## 2. First-render null trap (partially patched, still fragile)
PR P's follow-up added `@if (accounts is null) { <p>Lädt…</p> return; }` sprinkled across ~11 pages/dialogs. Works but:
- Every page rolls its own guard. Sometimes it's `@if (balance is null)`, sometimes `@if (accounts is null || transactions is null)`, sometimes `@if (config is not null && layout is not null && data is not null) { <PlotlyChart .../> }`.
- No shared skeleton visual — just `<p>Lädt…</p>`. Feels unfinished.
- Collection fields are initialised to `[]` in some pages and left nullable in others. Two idioms in the same codebase.
A shared `<LoadingWhen Data="@x" />` wrapper with a consistent skeleton visual would centralize this.
## 3. `Home.razor` is doing too much
100 lines of markup + 190-line codebehind for four unrelated concerns on the landing page:
1. **Statement upload** — `FluentInputFile`, progress bar, cancel button.
2. **XML/zip file processing** — `OnCompletedAsync`, unzip, iterate.
3. **Auto-matched bank transactions table** — hand-crafted `<table>`, `<input list="texts">` for autocomplete, per-row book / split buttons.
4. **Booking / split flow** — `BookBankTransactionAsync`, `SplitBankTransactionAsync`.
Split into `<StatementImport />` and `<PendingBookings />`; Home stacks them.
## 4. Reports vs. list pages use different visual languages
- **List pages** (Accounts, BookingRules, BookingTexts, Transactions, BankTransactions): `<FluentDataGrid>` with `<PropertyColumn>` / `<TemplateColumn>`, pagination, sortable headers.
- **Reports** (BalanceReport, BalanceSheetReport, DetailReport, ProfitLossReport): hand-rolled `<table class="report">` with `@if (account.Balance == 0) { continue; }` inside `<tr>` and hand-crafted section headers.
Two different visual languages. Report format is domain-appropriate (they're printed accounting statements, not grids), but the hand-rolled markup means:
- Adding a column means editing 4 report `.razor` files.
- The `@if ... { continue; }` pattern inside `<tr>` reads oddly.
- No component reuse across reports.
Extract `<ReportSection Title="…">`, `<ReportAccountRow Account="@x" ShowBalance />`, `<ReportTotalRow Label="…" Value="…" />`.
## 5. No error boundaries, no expected-error UX
If any component throws, users get `blazor-error-ui` at the bottom of `MainLayout.razor` — a fixed div with *"An unhandled error has occurred. Reload."* No route back except manual reload. `Error.razor` is the default template that tells the user to enable Development mode.
For expected failures (save failed because file locked, unknown id on delete):
- Wrap each page/section in `<ErrorBoundary>` with a friendly retry.
- Route repository/service exceptions through `IMessageService` / `IToastService` (both already registered) instead of letting them bubble.
## 6. Feedback after mutations is invisible
`Accounts.EditAsync` closes the dialog and re-fetches the list. No confirmation, no "Saved" toast. If the list is long and the user's edit isn't visible in the current page, they have no signal the save worked.
Same in `BookingRules`, `BookingTexts`, `Classes`, `Transactions`.
Add `toastService.ShowSuccess("Konto gespeichert")` etc.
## 7. Confirmation dialogs show unformatted values
`Transactions.DeleteAsync`:
```csharp
$"Transaction '{transaction.Text}' mit Betrag {transaction.Value} wirklich löschen?"
```
`transaction.Value` is a raw decimal — no currency, no thousands separator. Reads *"...mit Betrag 1234.567 wirklich löschen?"*. Should use `.ToFormattedString()` and append the currency.
Same in `Accounts.DeleteAsync`, `BookingRules.DeleteAsync`, `BookingTexts.DeleteAsync`, `Classes.DeleteAsync`.
## 8. No search / filter on list pages
None of the FluentDataGrid pages have search or filter controls — only pagination.
- BookingRules can grow to hundreds of entries.
- BookingTexts likewise.
- BankTransactions runs into thousands over a year.
`<FluentSearch>` bound to a `filter` field, filter the `IQueryable` before passing to the grid.
## 9. Dialogs are fixed-height
`DialogParameters { Height = "500px" }` on every dialog. Content shorter than 500px wastes space; content longer scrolls inside. Should be `Height = "auto"`.
Also every dialog uses the same header icon (`Icons.Regular.Size24.WindowApps` — a generic "window" glyph). No visual distinction between dialog types.
## 10. `AccountSelector` uses `Autofocus="true"`
Fine for the split-dialog flow. In the Home booking table it means every keystroke-navigated row grabs focus from wherever the user was. Should be an opt-in parameter, off by default.
## 11. Header year/mandator has no confirm on change
`MyHeader.YearChanged` immediately does a `forceLoad`. If the user accidentally clicks 2018 in the dropdown, whatever they were doing (unsaved dialog, half-typed booking) is gone. Fixed once (1.4) is fixed.
## 12. Minor rough edges
- `Home.razor.cs` field name `IsCanceled` describes an intent, not a state — `cancelRequested` would be clearer.
- `Home.razor:35` shows a smiley emoji when no pending bookings.
- `Home.razor:67` uses `<input list="texts">` + `<datalist>` for text autocomplete — native HTML, inconsistent with `FluentAutocomplete` used in `AccountSelector`.
- Print CSS in `app.css` only hides `.hidePrint`. No `@page`, no `page-break-inside: avoid` on report rows. Long reports fragment across pages randomly.
- No accessible label on the delete/edit icon buttons (screen readers say "Button, button").
- The header ` $"{title} {mandator} {year}"` in `Report.cs` reads like *"Bilanz Claudio Schaad 2026"* — no separator, no branding.
---
## Suggested phased plan
Sequenced by value / risk. Each is one PR unless noted.
### Phase 1 — Correctness bugs (low risk, high value)
- **U-1** Fix `AccountSelector` to use `OnParametersSet` instead of `OnInitialized` so the preselection tracks late-arriving `Accounts`.
- **U-2** Guard `TransactionDialog.SaveAsync` against `SelectedValue == null`; disable Save when the date is empty.
- **U-3** Fix `TransactionSplitDialog` validation to cover every row (per-row `EditContext`, or one `EditContext` over a wrapper).
### Phase 2 — The forceLoad/singleton trap (biggest single improvement)
Split into two PRs on delivery because the header path touches ~15 pages.
- **U-4a** Fix `Home.ReloadPage()` — the single non-settings `forceLoad` in the app. `SplitBankTransactionAsync` and `OnCompletedAsync` (statement import) now call a shared `LoadDataAsync()` that re-runs the auto-matcher, re-fetches accounts and booking texts in place, and keeps the SignalR circuit + scoped `RepositoryCache` alive. No cross-cutting scaffolding needed.
- **U-4b** Persist year and mandator (cookie + URL query params), make `SettingsService` `Scoped`, add an event or `LocationChanged` subscription so every page refreshes on year/mandator change, and drop `MyHeader`'s `forceLoad`. This is the big one — it touches every routable page.
### Phase 3 — Loading and error UX consistency
- **U-5** Shared `<LoadingWhen Data="@x">` component with a consistent skeleton visual. Adopt across every page currently using ad-hoc `@if (x is null) { <p>Lädt…</p> return; }`.
- **U-6a** Wrap `MainLayout`'s `@Body` in `<ErrorBoundary>` with a friendly retry UI. Reset on any `NavigationManager.LocationChanged` so the error state doesn't stick after the user clicks "Zur Startseite".
- **U-6b** Route expected `SaveTransactionAsync` / `DeleteAccountAsync` / etc. exceptions to `IMessageService` — try/catch in each of the ~15 event handlers with a shared error-formatting helper.
### Phase 4 — Feedback + polish for mutations
- **U-7** `IToastService.ShowSuccess(...)` after every successful save/delete across Accounts, BookingRules, BookingTexts, Classes, Transactions.
- **U-8** Format the raw decimal in Transactions' delete confirmation via `ToFormattedString()`, and fix the "Transaction" → "Transaktion" typo. Turned out to be a one-file change — the other four CRUD pages don't put values into confirmations.
- **U-9** Add `<FluentSearch>` filter to Accounts, BookingRules, BookingTexts, BankTransactions.
### Phase 5 — Home page split
- **U-10** Extract `<StatementImport />` and `<PendingBookings />` components. Home stacks them.
- **U-11** Replace the Home `<input list="texts">` + `<datalist>` with `FluentAutocomplete` so the autocomplete UI is uniform with the split dialog.
### Phase 6 — Report componentization
- **U-12** Extract `<ReportSection>`, `<ReportAccountRow>`, `<ReportSectionHeader>`, `<ReportTotalRow>`, `<ReportFooter>`. Apply to the four report pages. Shared print CSS + `page-break-inside: avoid` on rows.
### Phase 7 — Nice-to-haves (optional)
- Accessibility: `aria-label` on icon-only buttons, focus management after dialog close, skip-to-content link.
- Dark mode toggle (Fluent supports theme override).
- Keyboard: `Ctrl+S` in dialogs, `Del` on selected row in grids.
- Header format: nicer separator + optional subtitle.
- `AccountSelector`'s `Autofocus` becomes opt-in.
## What was explicitly *not* flagged
- The `de-CH` culture hardcode. Correct choice for the target user; localizing is a lot of work for zero user value.
- The `InteractiveServer` render mode. Correct for this app.
- The Fluent UI dependency.