refactor(ui): delete dead Icon.* geometries and icon-stroke styles, add MDI name guard test

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
mika kuns
2026-10-01 16:56:45 +02:00
co-authored by Claude Sonnet 5.5
parent 03f5eae35b
commit de68515128
4 changed files with 49 additions and 207 deletions
+2 -2
View File
@@ -34,7 +34,7 @@ Views/ — mirrors the VM layout; Islands/Detail/ holds TaskHeaderBar,
Views/Controls/ — MarkdownView, ModalShell, ThemedDatePicker, DiffTextView, DiffEditorSetup,
InheritedBadge, AgentConfigEditor, UsagePill, UsageGaugeBar
Design/ — Tokens.axaml (design tokens; merged before styles)
+ IslandStyles.axaml (component styles + the filled icon geometry library)
+ IslandStyles.axaml (component styles; icons are Material Design Icons, no local geometry)
```
## Core ViewModels
@@ -130,7 +130,7 @@ snaps `CanResize="True"` windows, which is the opt-in), and it insets itself by
## Gotchas
- **Never set `Foreground`/`Stroke` on an icon inline.** A local value outranks every style, so the button's hover/checked/disabled states can't reach the icon and it looks dead on hover — this is what broke the icons that were "fixed" to the right colour inline. Colour is picked with a class; the single `ICON PAINTING` block in `IslandStyles.axaml` owns all states (`.danger`, `.star-btn.on`, `.active`, `:checked`, `:disabled`). Avalonia has no specificity — that block's **declaration order** is the mechanism, so add new variants in the right section, not at the end.
- **`PathIcon` *fills* its geometry.** Line-art/stroke icons must be authored as filled geometry or rendered as `<Path Classes="icon-stroke">` inside a `Viewbox` (e.g. `Icon.Refine`, `Icon.GitBranch`). For new icons prefer Material Design Icons (`mdi:Name` via the `IconKey` converter) — they are filled and work in a plain `PathIcon`. A pure stroke path in a `PathIcon` is **invisible**. `Path.icon-stroke` takes the same colour classes/states as `PathIcon`.
- **Icons are Material Design Icons, nothing hand-drawn.** Use `<PathIcon Data="{Binding Source='mdi:Name', Converter={StaticResource IconKey}}"/>` (`IconKeyConverter`); legacy bare list keys stored in the DB resolve through `IconKeyConverter.LegacyKinds`. A misspelled kind makes the converter return null and the icon **silently vanishes** — `MdiIconNamesTests` scans every `mdi:Name` in `src/ClaudeDo.Ui` and fails on an unknown one. Never set `Foreground` inline (see above).
- **`NumericUpDown.Value` is `decimal?` and goes null while the box is empty** — i.e. every time the user clears a value to type a new one. Bound TwoWay to a non-nullable `int`/`decimal`, that null throws `InvalidCastException`. Either bind a `decimal?` property (as `AgentConfigEditorViewModel.MaxTurns` does) or add `Converter={StaticResource KeepLastNumber}`, which drops the null via `BindingOperations.DoNothing`.
- **`ScrollViewer.Padding` is excluded from the Extent (Avalonia 12).** At max scroll the content still sticks out below the viewport by the padding height, so the last line(s) are clipped and unreachable — even after `ScrollToEnd()`. Put the inset on the *content* (`Margin` on the inner `ItemsControl`/`StackPanel`) instead. Verified headlessly: 12,8,12,4 on the viewer → last item 20px below the viewport; same inset as content margin → fully visible. **`ListBox` does *not* have this trap** — its Fluent control-theme template forwards `Padding` onto `PART_ItemsPresenter`'s margin (content side), not the raw `ScrollViewer.Padding`; verified headlessly that `ListBox.Padding="10,4"` and a `Margin="10,4"` on the `ItemsPanelTemplate` panel produce an *identical* Extent and leave the last row fully visible at max scroll. Still fine to put the inset on the panel for consistency with the other scrollables above, just don't assume it's fixing a real bug there.
- **Re-templating a `TabControl` must keep `PART_ItemsPresenter`.** `TabControl.SelectedContent` is resolved via `ContainerFromIndex`, and containers are only realized by the `ItemsPresenter`. A template that renders just `PART_SelectedContentHost` (to hide the tab strip) leaves `SelectedContent` permanently `null` → an empty body. Keep the presenter with `IsVisible="False"`; that still realizes containers (verified headlessly, Avalonia 12.0.4). This broke the whole Settings modal body in 56c3719e.