Let a member leave a group from the UI #76
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/leave-group-ui"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes task 160.
GroupMemberRownow offers a "Leave group" control on the caller's own row (any role — the API alone enforces the last-Owner rule for an Owner's own row), with an inline confirmation naming the consequence ("Its places will disappear from your map"). A personal group still offers no roster at all, so no leave control either. An Owner refused with 409 sees a reworded reason ("You're the only Owner...") pointing at the "Make owner" control already visible on another row in the same panel.Leaving patches
MapPlacesStateandCallerGroupsStatedirectly (RemoveGroup) rather than reloading — the map, drawer accordion and groups list all drop the group with no page reload.GroupsSectionis folded ontoCallerGroupsState(ADR-0068, partially supersedes ADR-0067): leaving is a second mutation path that doesn't go throughDrawer's explicitHandleGroupSavedAsyncreload, exactly the trigger ADR-0067 named for doing this.Drawerno longer holds a@reftoGroupsSectionat all.Not verified in a browser (task 156): focus landing on the confirm button and what a screen reader announces from
aria-describedbyare asserted only as "the component asked the browser to do this."Verdict: changes needed
Build is clean (0 warnings/errors) and
PlaceMark.WebUI.Testspasses 351/355 (4 pre-existing skips) under the pinned 10.0.100 SDK. TracedRemoveGroupon bothMapPlacesStateandCallerGroupsState,GroupMemberRow's self-row rendering,GroupMembersPanel.LeaveAsync, and theGroupsSectionfold against every consumer; also mutation-tested by removing theCallerGroups.Changedsubscription fromGroupsSection—GroupsSectionTestsreddened (2 failures) as expected, confirming the fold doesn't bypass the reset.ADR-0068 is missing its half of the supersession, and the README index wasn't updated (ADR-0001, both required). ADR-0067 correctly gained a
Partially superseded by: [ADR-0068]line, but ADR-0068's own header has no reciprocalPartially supersedes: [ADR-0067]field (compare ADR-0017, which statesSupersedes: [ADR-0002] in part). Anddocs/adr/README.md's index row for 0067 still reads plainAccepted— per the README's own convention ("Write its status in the table asAccepted (partly superseded by 00NN)") it should readAccepted (partly superseded by 0068). Only the new 0068 row was added. Fix both before merging.Everything else checks out:
MapPlacesState.RemoveGroup/CallerGroupsState.RemoveGroupare simple predicate filters — idempotent by construction (removing an absent id is a no-op), covered by tests for both the present and absent cases, andGroupMembersPanel_LeaveConfirmed_...exercises the full path end-to-end against the real component.GET /api/groups/{groupId}after a successful leave —LeaveAsyncpatches state and closes; confirmed by reading the method and by the panel-level test.DefaultMembers()in the test fixture includes a second accepted non-Owner member, so tracingShowOwnerControlsconfirms "Make owner" genuinely renders in the same roster list as the reworded error — the test itself only asserts the roster is non-empty, not the "Make owner" text specifically, which would have made this easier to verify from the test alone rather than by tracing.GroupMembersPanelgates the entire roster (and thus every row) behind!group.IsPersonal, so no leave control today. As ADR-0068 itself says,GroupMemberRowhas no personal-group awareness of its own — if a roster were ever rendered for one, "Leave group" would appear with nothing to stop it. That's a named, accepted gap, not a silent one, so no action needed here.aria-describedby, Escape, native<button>) mirrors the existing remove/promote pattern exactly and is covered the same way.Not verifiable without a browser (task 156, as the PR body already says): focus actually landing on the confirm button, what a screen reader announces from
aria-describedby, and whether leaving reads sensibly if the group's detail is open elsewhere on the page. bUnit only proves the component asks the browser to do these things.Addressed: ADR-0068 now carries
Partially supersedes: ADR-0067, ADR-0067's own metadata names the exact paragraphs it displaces, and the README index row for 0067 readsAccepted (partly superseded by 0068). Also fixed a wording slip in ADR-0068's own Consequences found while doing this (miscountedGET /api/groupsreaders). Full CI sequence re-run clean, includingdotnet format --verify-no-changes.Verdict: mergeable
Delta since
42d89b4is exactly the three files expected (docs/adr/0067, docs/adr/0068, docs/adr/README.md), 12/-7 lines, no code touched.Partially supersedes: [ADR-0067], naming the specific displaced paragraphs by their opening words — matches the establishedPartially supersedesprecedent (ADR-0038's own field, superseding ADR-0033), which ADR-0001 permits alongside theSupersedes ... in partstyle used elsewhere (ADR-0017, ADR-0022, ADR-0034).Partially superseded byline naming ADR-0068 and the same paragraphs, plus what replaces them. Its frozen body (the "TwoGET /api/groupsreaders exist..." paragraph) is untouched, as ADR-0001 requires — the metadata line is what tells a reader it's now displaced, not an edit to the paragraph itself.Accepted (partly superseded by 0068), matching the exact format used elsewhere in the same table (e.g. 0029, 0033).GET /api/groupshad two independent readers (CallerGroupsState,GroupsSection); after folding, one (CallerGroupsState) —MapPlacesState'sGET /api/placeswas never aGET /api/groupsreader and is now correctly pulled out of that count rather than counted toward it. Checked against the source:CallerGroupsStateis the only production type that independently loadsGET /api/groups;GroupsSection,PlaceAccordionandHomeall read it via that shared state. Both mentions of the count inside ADR-0068 (Decision and Consequences) were updated consistently, and no stale "three" reference remains anywhere in the file.Nothing else to act on.