Expand a place's group when a filter hides it after create or move #84

Merged

rob merged 1 commit from fix/filtered-group-marker-disagreement into main

2026-08-06 06:21:50 +00:00

rob commented

2026-08-06 06:17:20 +00:00

Owner

Vikunja task 162: creating or moving a place into a group the accordion currently filters out left it selected and panned to but plotted no marker for it — MapPlacesState.VisiblePlaces disagreed with the unconditional selection ADR-0067/ADR-0071 make for create, and with the patch Home.MoveSelectedPlaceAsync makes for move.

Chose expand the destination group (MapPlacesState.EnsureGroupVisible, called from both routes) over clearing the filter or refusing the operation: creating or moving into a group is the caller naming it as somewhere they want to look, expanding only that group keeps whatever else was already open, and refusing a permitted create/move because of an unrelated accordion display state would be worse than the defect. It's a no-op while nothing is currently filtered (ADR-0062's zero-expanded-means-unfiltered), so an unfiltered map is never narrowed by a create or move. Full reasoning in ADR-0073, which answers rather than supersedes ADR-0071's own "create-flow bypass" paragraph — that paragraph already predicted this shape and left it for task 162.

One test per route (PlaceFormPanelTests, HomeTests), both confirmed failing before the fix.

Vikunja task 162: creating or moving a place into a group the accordion currently filters out left it selected and panned to but plotted no marker for it — `MapPlacesState.VisiblePlaces` disagreed with the unconditional selection ADR-0067/ADR-0071 make for create, and with the patch `Home.MoveSelectedPlaceAsync` makes for move. Chose **expand the destination group** (`MapPlacesState.EnsureGroupVisible`, called from both routes) over clearing the filter or refusing the operation: creating or moving into a group is the caller naming it as somewhere they want to look, expanding only that group keeps whatever else was already open, and refusing a permitted create/move because of an unrelated accordion display state would be worse than the defect. It's a no-op while nothing is currently filtered (ADR-0062's zero-expanded-means-unfiltered), so an unfiltered map is never narrowed by a create or move. Full reasoning in ADR-0073, which answers rather than supersedes ADR-0071's own "create-flow bypass" paragraph — that paragraph already predicted this shape and left it for task 162. One test per route (`PlaceFormPanelTests`, `HomeTests`), both confirmed failing before the fix.

rob added 1 commit

2026-08-06 06:17:20 +00:00

Expand a place's group when a filter hides it after create or move

CI / build (pull_request) Successful in 2m29s

Details

e17e666403

rob reviewed

2026-08-06 06:21:33 +00:00

rob left a comment

Verdict: mergeable

Nothing to act on.

Checked and confirmed:

EnsureGroupVisible's no-op path (_expandedGroupIds.Count == 0) matches VisiblePlaces's own zero-expanded-means-unfiltered rule; already-expanded destination is a genuine no-op (HashSet.Add returning false short-circuits before Changed?.Invoke()) — no redundant render.
Re-expanding a deliberately collapsed group on create/move is a real UX trade-off, but it's reasoned about explicitly in ADR-0073, visible (the accordion section opens, not just the marker), and one click to reverse — not treated as a defect.
Create and move both land on the same final state via the one method, despite differing call order (EnsureGroupVisible before SelectPlace for create; after UpsertPlace for move, with no separate select call since the id-based lookup already tracks the patched place) — no observable divergence between the two routes.
Reverted both call sites and confirmed PlaceFormPanel_AddSavedIntoAGroupTheCurrentFilterHides_ExpandsItSoTheMarkerIsPlotted and Home_MoveConfirmedIntoAGroupTheCurrentFilterHides_ExpandsItSoTheMarkerIsPlotted go red, then restored — full WebUI suite (440, 4 skipped) green with the pinned SDK (10.0.100).
CI run #397 on e17e666 is green.
ADR-0073 is 0073 (0072 is main's highest, no collision), and its Alternatives section engages the real options (clear filter, refuse the operation, diverge by route) rather than just narrating the choice made.
Stale deferral comments in both Home.MoveSelectedPlaceAsync and PlaceFormPanel.HandleSavedAsync are replaced, not left duplicated, and now describe the fix in place.
Neither call site reaches for requireVisible: true; PlaceFormPanel still passes false, move doesn't touch SelectPlace at all.
Merges cleanly against main.

Verdict: mergeable Nothing to act on. Checked and confirmed: - `EnsureGroupVisible`'s no-op path (`_expandedGroupIds.Count == 0`) matches `VisiblePlaces`'s own zero-expanded-means-unfiltered rule; already-expanded destination is a genuine no-op (`HashSet.Add` returning `false` short-circuits before `Changed?.Invoke()`) — no redundant render. - Re-expanding a deliberately collapsed group on create/move is a real UX trade-off, but it's reasoned about explicitly in ADR-0073, visible (the accordion section opens, not just the marker), and one click to reverse — not treated as a defect. - Create and move both land on the same final state via the one method, despite differing call order (`EnsureGroupVisible` before `SelectPlace` for create; after `UpsertPlace` for move, with no separate select call since the id-based lookup already tracks the patched place) — no observable divergence between the two routes. - Reverted both call sites and confirmed `PlaceFormPanel_AddSavedIntoAGroupTheCurrentFilterHides_ExpandsItSoTheMarkerIsPlotted` and `Home_MoveConfirmedIntoAGroupTheCurrentFilterHides_ExpandsItSoTheMarkerIsPlotted` go red, then restored — full WebUI suite (440, 4 skipped) green with the pinned SDK (10.0.100). - CI run #397 on `e17e666` is green. - ADR-0073 is 0073 (0072 is main's highest, no collision), and its Alternatives section engages the real options (clear filter, refuse the operation, diverge by route) rather than just narrating the choice made. - Stale deferral comments in both `Home.MoveSelectedPlaceAsync` and `PlaceFormPanel.HandleSavedAsync` are replaced, not left duplicated, and now describe the fix in place. - Neither call site reaches for `requireVisible: true`; `PlaceFormPanel` still passes `false`, move doesn't touch `SelectPlace` at all. - Merges cleanly against `main`.

rob merged commit 40fbe1f346 into main

2026-08-06 06:21:50 +00:00

rob deleted branch fix/filtered-group-marker-disagreement

2026-08-06 06:21:50 +00:00

rob referenced this pull request from a commit

2026-08-06 06:21:52 +00:00

Expand a filtered-out group so a place put into it is plotted (#84)

rob referenced this pull request

2026-08-20 11:44:48 +00:00

Prove the place marker's accessible name against the accessibility tree #200