From 2fc361f753829e5532303fa020ab983cfd45dc72 Mon Sep 17 00:00:00 2001 From: dmiesopust Date: Wed, 2 Sep 2026 11:13:58 +0200 Subject: [PATCH] fix(workspace): replace Pivot with TabView to fix tab-header clipping Pivot silently clips overflowing headers into an unreachable scroll strip once its host column narrows even slightly, making a tab unclickable with no visible sign why. TabView doesn't have that failure mode. Same three tabs, same headers, no ViewModel changes. --- CLAUDE.md | 5 ++++- src/Pulsemap.App/Views/WorkspacePage.xaml | 20 ++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dcd3c0c..2217495 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,7 @@ Pulsemap is developed by Dotify. It follows Dotify's internal engineering standa - **XAML compiler quirks (WindowsAppSDK.WinUI 2.3.6, observed 2026-09-01):** A standalone `.xaml` file whose root element is `ResourceDictionary` may fail with a cryptic `WMC9999 "cannot find Polish/neutral culture resources"` error — this is the XAML compiler's own error-reporting path breaking on a non-English OS locale, not a real problem with your markup, and it can mask a genuinely different underlying error. If you hit `WMC9999`, don't trust it as the real problem: temporarily move suspect files fully outside the project directory (renaming within the tree isn't enough — the SDK's XAML globbing is recursive and folder-name-independent) to bisect down to the actual cause. Concretely already found: (1) an extra `ResourceDictionary`-rooted file caused it once a `Page`-rooted file was *also* present and broken — inlining resources into `App.xaml` (which is `Application`-rooted and always compiled first) sidesteps it; (2) `{x:Bind Some.Method('literalArg')}` — the function-call-with-string-literal-argument form of x:Bind — is not supported by this compiler version and triggers the same masked error; use a computed property on the bound type instead; (3) `{x:Bind PageRoot.ViewModel.SomeCommand}` inside a nested `DataTemplate` — binding to an outer named element's property path from within a `DataTemplate`'s own `x:DataType` scope — also isn't supported here; fall back to a plain `Tag="{x:Bind}"` (binds the template's own DataContext, which *is* supported) plus a code-behind `Click` handler that calls the command directly. - **Related but distinct — a runtime (not compile-time) XAML crash:** declaring `IsChecked="True"` on a `ToggleButton` in markup throws `Microsoft.UI.Xaml.Markup.XamlParseException: Failed to assign to property 'Microsoft.UI.Xaml.Controls.Primitives.ToggleButton.IsChecked'` the moment that page is constructed (`LoadComponent`), silently killing the app since it happens inside `async void`/fire-and-forget navigation code with nothing to catch it. Same locale-masked-error family (the exception's primary message is unreadable Polish mojibake; only a secondary fallback line names the real property), but this one fires at `InitializeComponent()` time, not build time — so it won't show up until the page is actually navigated to. Fix: don't set `IsChecked` declaratively; set it in code-behind after `InitializeComponent()` instead (e.g. `SomeToggleButton.IsChecked = true;`). If a page like this crashes the app with no visible error, temporarily wire `Application.UnhandledException` to write `e.Exception.ToString()` to a file — the default unhandled-exception behavior here is silent process termination with nothing in the UI. - **`x:Bind TwoWay` on `TextBox.Text` defaults to `UpdateSourceTrigger=LostFocus`, not `PropertyChanged`.** This bit us for real: a wizard step's "Next" button stayed disabled after typing a value and clicking straight to "Next" without tabbing away first — the ViewModel property was still empty because the TextBox never lost focus to flush the binding. Any TwoWay-bound `TextBox.Text` that gates a `CanExecute` (or is read immediately after by other logic) needs `UpdateSourceTrigger=PropertyChanged` explicitly: `Text="{x:Bind ViewModel.Foo, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"`. `CheckBox.IsChecked`, `ComboBox.SelectedItem`, and `NumberBox.Value` all commit immediately on their own interaction and don't need this. -- **`Pivot` silently clips overflowing headers into a scroll strip** (small `‹›` chevrons) once header text is too wide for the host column — a clipped-away `PivotItem` is not click-reachable at its "logical" screen position even though `FindFirst`-by-name still returns a seemingly valid `BoundingRectangle` for it (UI Automation doesn't know it's visually clipped). If a Pivot has 3+ items in a narrow side panel, size the column generously and keep header text short, or the last tab(s) become effectively unreachable by mouse. +- **`Pivot` silently clips overflowing headers into a scroll strip** (small `‹›` chevrons) once header text is too wide for the host column — a clipped-away `PivotItem` is not click-reachable at its "logical" screen position even though `FindFirst`-by-name still returns a seemingly valid `BoundingRectangle` for it (UI Automation doesn't know it's visually clipped). If a Pivot has 3+ items in a narrow side panel, size the column generously and keep header text short, or the last tab(s) become effectively unreachable by mouse. **This app no longer uses `Pivot` anywhere** (replaced by `TabView` in the Workspace side panel, 2026-09-02, precisely to remove this failure mode) — kept here as a general WinUI note in case a future control choice reintroduces it. ## Non-negotiable, every change @@ -56,6 +56,9 @@ Don't invent a convention to fill a gap. Name what's missing, propose the smalle - **Live manual QA pass (2026-09-01):** the app's own developer did a real click-through (not automation) and found two real bugs, both fixed same-day: (1) the dead Surveys tab above; (2) Settings' "Open Logs Folder" button silently did nothing on a fresh install, because `FileAppLogger` only creates its log directory lazily on first write, and `Launcher.LaunchFolderPathAsync` fails on a path that doesn't exist yet — fixed at the call site (`SettingsViewModel.OpenLogsFolderAsync` now `Directory.CreateDirectory`s before launching) rather than inside `FileAppLogger` itself, since creating the directory eagerly in `FileAppLogger`'s constructor would have broken an existing `FileAppLoggerTests` case that deliberately blocks the log path with a plain file to prove the logger degrades gracefully when its directory can't be created. The floor-plan image/PDF rendering and the 4 Export buttons weren't specifically called out as broken or working in this pass — still not explicitly confirmed either way, unlike the two bugs above. - **Per-wall material editing and guided-walk improvements (added 2026-09-02, DONE, merged to staging via PR #8):** the Select tool (previously a no-op placeholder) now does two things depending on what's clicked: a wall toggles into a multi-select set (highlighted blue), with a toolbar panel to batch-apply one material/thickness to everything selected; a test point instead prompts to recapture its reading (rescans via the currently selected adapter and replaces `Measurements`/`InterferenceReadings`, keeping `Position`). Guided walk gained a Skip button (dequeues without capturing, stays active), renders its remaining queue as faint markers on the canvas, and now persists its pending queue on `Floor.PendingGuidedWalkPoints`/`PendingGuidedWalkBand` — saved on every dequeue/skip/cancel, so closing and reopening the app mid-walk resumes instead of losing the remaining points; `OnSelectedFloorChanged` auto-resumes/pauses as the floor selector switches. 144 tests now on that branch (75 App/69 Core). - **In-app update check (added 2026-09-02, DONE, merged to staging via PR #7 — see ADR-0004):** Pulsemap's first-ever outbound network call, explicitly cleared with the PM given ADR-0003 had deliberately left this question open. Home now checks `https://api.github.com/repos/DotifyBIZ/pulsemap/releases/latest` on load (via new `IUpdateCheckService`/`GitHubUpdateCheckService`, App-layer, `IHttpClientFactory`-based) and shows a dismissible banner + release link if the tag is newer than the running build (`SemanticVersionComparer`, Core, pure/testable, compares `Assembly.GetName().Version` against a `vX.Y.Z` tag). Gated behind a new `AppSettings.CheckForUpdatesEnabled` (default on, Core, `FileAppSettingsService` at `%LocalAppData%\Pulsemap\settings.json` — Pulsemap's first app-preferences store, distinct from survey data and from `FileAppLogger`'s logs), with a visible toggle on the Settings page ("Check for updates on launch"). A real prerequisite gap found and fixed along the way: `scripts/build-installer.ps1`'s `dotnet publish` never passed `-p:Version=$Version`, so the published assembly's version was always the SDK's static default regardless of the actual release — without that fix, the whole comparison would have been meaningless. README's "won't phone home" bullet is amended accordingly (telemetry/account/server-free claim still holds; the literal zero-network-calls reading no longer does). 82 Core / 76 App tests on that branch — includes a stub-`HttpMessageHandler`-backed test of the actual HTTP/JSON path, not just the pure version-comparison logic. + +**UX pass (started 2026-09-02, in progress)** — a full-codebase UX review turned up several friction points (Select tool overloaded with undiscoverable per-click-target behavior, no undo anywhere, no onboarding, inconsistent delete confirmation, silent AP-suggestion replacement, no survey-type context in Workspace, the Pivot-clipping bug). Being fixed incrementally, each as its own branch: +- **Pivot → TabView migration (added 2026-09-02, on branch `feature/workspace-tabview-migration`, not yet merged to staging):** replaces `Pivot` with `TabView` (`IsAddTabButtonVisible="False"`, `IsClosable="False"` per tab) in the Workspace side panel — same 3 tabs (Coverage/Suggestions/Adapter), same `x:Uid`-driven `Header`s, no ViewModel changes, purely a control swap to eliminate the documented header-clipping failure mode for good rather than keep managing around it. Also removes the "deliberately not a 4th Pivot tab" constraint noted on the CSV/JSON/PDF export and multi-floor entries above — that specific limitation no longer applies, though nothing currently plans to add a 4th tab. - **Cross-floor channel coordination (added 2026-09-02, on branch `feature/cross-floor-channel-coordination`, not yet merged to staging):** `IApPlacementOptimizer.SuggestPlacements` gained an `allFloors` parameter (mirroring `CoverageGridCalculator.ComputeGrid`'s own `floor`+`allFloors` shape) so `GreedyCoverageApPlacementOptimizer`'s channel ranking can see channels already assigned to APs on other floors, not just this floor's own measured interference. Placement itself stays per-floor-only, per explicit scope decision — this is channel-assignment coordination, not a joint cross-floor optimizer. `CrossFloorChannelUsageScore` reuses `CoverageGridCalculator`'s same skip rules (same-level-different-floor and outdoor floors don't participate) and the same 25dB/level attenuation figure (duplicated as its own constant, not shared across layers — matches how `WorkspaceViewModel` already duplicates `ReliableCoverageThresholdDbm`), scored in the same linear-power units as the existing same-floor `MeasuredInterferenceScore` so the two combine meaningfully instead of one dominating on scale. Known, accepted limitation: since AP suggestion is still a per-floor, user-triggered action, a floor suggested *before* its neighbor won't retroactively pick up the neighbor's channels — re-running suggestions on the earlier floor afterward does. - **Outdoor-area bounds editor (added 2026-09-02, on branch `feature/outdoor-bounds-editor`, not yet merged to staging):** outdoor areas previously had no way to resize/reposition their extent from the UI — a new floor always got a fixed 40x40m default with no follow-up editor. `FloorPlanCanvas` now draws an outdoor floor's bounds as a dashed rectangle with one resize handle at its far corner (deliberately one anchored corner + one dragged corner, not four independent handles — the smallest interaction that still covers both resize and reposition); dragging inside the rectangle moves the whole area instead. A click first checks for an existing test point/AP within the usual 0.5m tolerance so precise clicks on markers placed inside the area still work — only a click that misses every marker starts a bounds drag. Drag state and a live preview stay canvas-local (mirrors the "canvas owns rendering/gesture recognition only" rule already documented on `FloorPlanCanvas`); only on release does a new `OutdoorBoundsChanged` event fire, which `WorkspaceViewModel.UpdateOutdoorBoundsAsync` persists to `Floor.OutdoorBoundsMin`/`Max`. - **Per-wall material editing and guided-walk improvements (added 2026-09-02, on branch `feature/wall-material-and-guided-walk`, not yet merged to staging):** the Select tool (previously a no-op placeholder) now does two things depending on what's clicked: a wall toggles into a multi-select set (highlighted blue), with a toolbar panel to batch-apply one material/thickness to everything selected; a test point instead prompts to recapture its reading (rescans via the currently selected adapter and replaces `Measurements`/`InterferenceReadings`, keeping `Position`). Guided walk gained a Skip button (dequeues without capturing, stays active), renders its remaining queue as faint markers on the canvas, and now persists its pending queue on `Floor.PendingGuidedWalkPoints`/`PendingGuidedWalkBand` — saved on every dequeue/skip/cancel, so closing and reopening the app mid-walk resumes instead of losing the remaining points; `OnSelectedFloorChanged` auto-resumes/pauses as the floor selector switches. 144 tests now on that branch (75 App/69 Core). diff --git a/src/Pulsemap.App/Views/WorkspacePage.xaml b/src/Pulsemap.App/Views/WorkspacePage.xaml index b84d300..af3073e 100644 --- a/src/Pulsemap.App/Views/WorkspacePage.xaml +++ b/src/Pulsemap.App/Views/WorkspacePage.xaml @@ -95,8 +95,12 @@ - - + + + @@ -127,9 +131,9 @@ FontSize="{StaticResource FontSizeXs}" Foreground="{StaticResource Gray400Brush}" /> - + - +