UI-focused analysis complementing IMPROVEMENT_PLAN.md: 4 real bugs, first-render null-guard inconsistency, Home page overload, report / list-page visual-language split, missing error boundaries, missing mutation feedback, missing search on grid pages, and a handful of minor rough edges. Phased into 7 phases (U-1 .. U-12 plus nice-to-haves) sequenced by value / risk. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
194 lines
11 KiB
Markdown
194 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)
|
|
|
|
- **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 `<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-6** Wrap each page (or the layout's `@Body`) in `<ErrorBoundary>` 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 `<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.
|