diff --git a/UI_IMPROVEMENT_PLAN.md b/UI_IMPROVEMENT_PLAN.md new file mode 100644 index 0000000..d1f18f2 --- /dev/null +++ b/UI_IMPROVEMENT_PLAN.md @@ -0,0 +1,194 @@ +# 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) {

Lädt…

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) { }`. +- No shared skeleton visual — just `

Lädt…

`. Feels unfinished. +- Collection fields are initialised to `[]` in some pages and left nullable in others. Two idioms in the same codebase. + +A shared `` 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 ``, `` for autocomplete, per-row book / split buttons. +4. **Booking / split flow** — `BookBankTransactionAsync`, `SplitBankTransactionAsync`. + +Split into `` and ``; Home stacks them. + +## 4. Reports vs. list pages use different visual languages + +- **List pages** (Accounts, BookingRules, BookingTexts, Transactions, BankTransactions): `` with `` / ``, pagination, sortable headers. +- **Reports** (BalanceReport, BalanceSheetReport, DetailReport, ProfitLossReport): hand-rolled `
` with `@if (account.Balance == 0) { continue; }` inside `` 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 `` reads oddly. +- No component reuse across reports. + +Extract ``, ``, ``. + +## 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 `` 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. + +`` 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 `` + `` 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) + +- **U-4** Persist year and mandator to a cookie *and* keep them in the URL as query params. `SettingsService` becomes `Scoped`, reading initial state from query params (fallback: cookie, fallback: config default). Header switching becomes an in-place update — no `forceLoad`, no circuit teardown, no state loss. + +### Phase 3 — Loading and error UX consistency + +- **U-5** Shared `` component with a consistent skeleton visual. Adopt across every page currently using ad-hoc `@if (x is null) {

Lädt…

return; }`. +- **U-6** Wrap each page (or the layout's `@Body`) in `` with a friendly retry UI. Route `SaveTransactionAsync` / `DeleteAccountAsync` etc. exceptions to `IMessageService` instead of bubbling. + +### Phase 4 — Feedback + polish for mutations + +- **U-7** `IToastService.ShowSuccess(...)` after every successful save/delete across Accounts, BookingRules, BookingTexts, Classes, Transactions. +- **U-8** Format values in confirmation messages; centralize the confirmation message builder. +- **U-9** Add `` filter to Accounts, BookingRules, BookingTexts, BankTransactions. + +### Phase 5 — Home page split + +- **U-10** Extract `` and `` components. Home stacks them. +- **U-11** Replace the Home `` + `` with `FluentAutocomplete` so the autocomplete UI is uniform with the split dialog. + +### Phase 6 — Report componentization + +- **U-12** Extract ``, ``, ``, ``, ``. 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.