Skip to content

editor: session multi-select groups (Ctrl+G) - #571

Merged
Aymericr merged 4 commits into
pascalorg:mainfrom
ActArtech:feat/editor-session-groups
Aug 4, 2026
Merged

editor: session multi-select groups (Ctrl+G)#571
Aymericr merged 4 commits into
pascalorg:mainfrom
ActArtech:feat/editor-session-groups

Conversation

@ActArtech

@ActArtech ActArtech commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds editor-only session selection groups so multi-select furniture and structure pieces can be remembered and reselected as a set (Figma-like, without scene-graph parents).

  • Ctrl/Cmd+G creates a session group from 2+ selected nodes (auto label Group N)
  • Ctrl/Cmd+Shift+G dissolves session groups that intersect the selection
  • Plain click on a member expands to all live members (3D, floor plan, scene tree)
  • Alt+click isolates a single member
  • Group / Ungroup icons on the multi-select floating pill (Move · Group · Copy · Delete) and the right multi-select panel
  • Capture-phase keyboard handler so browser Find does not steal Ctrl+G
  • Delete prunes dead members from session groups
  • Docs: wiki/architecture/selection-groups.md (+ links from selection-managers / architecture index)

Out of scope: persistent scene-graph groups, save/load with the project, MCP tools, named-group rename UI.

Related open PRs (same fork style): #569 mcp layout clearance, #570 light WebGPU preview.

How to test

  1. From repo root:
    bun test packages/editor/src/lib/session-groups.test.ts packages/editor/src/lib/selection-routing.test.ts packages/editor/src/lib/contextual-help.test.ts
    Expect all green.
  2. Run the standalone editor (bun + Next on port 3002), open a scene with furniture.
  3. Select mode (V). Multi-select 2+ items with Ctrl/Cmd+click.
  4. Confirm the floating pill shows Group next to Move / Copy / Delete (also on the right panel).
  5. Click Group or press Ctrl/Cmd+G. Panel title becomes like Group 1 · N.
  6. Click empty space, then click one member — whole group reselects.
  7. Alt+click one member — only that object is selected.
  8. Ctrl/Cmd+Shift+G or Ungroup — membership dissolves; selection is kept.
  9. Hard refresh — session groups are gone (session-only; expected).

Screenshots / screen recording

N/A attached here — interactive multi-select UX. Optional: short clip of Group icon + Ctrl+G expand-on-click if desired.

Checklist

  • I've tested this locally with bun test on the packages above and manual editor multi-select
  • My code follows the existing code style
  • I've updated relevant documentation (wiki/architecture/selection-groups.md)
  • This PR targets the main branch

Note

Medium Risk
Broad changes to selection click paths (3D, 2D, tree, keyboard) but session groups stay out of the scene graph and persistence; main risk is regressions in multi-select modifiers or stale expand after preview/load.

Overview
Adds editor-only session selection groups so a multi-select set can be remembered and re-selected without scene-graph parents or project save.

Create / dissolve: Ctrl/Cmd+G groups 2+ selected nodes (auto Group N); Ctrl/Cmd+Shift+G dissolves groups that intersect the selection while keeping the current selection. A capture-phase keydown handler on KeyG prevents the browser from stealing Ctrl/Cmd+G.

Reselect behavior: Plain click on a member expands to all live group members via expandSessionSelectionForNode, wired through 3D resolveSelectedIdsForNodeClick, 2D registry applyEntrySelection, floorplan background hits, and the site tree. Alt+click selects a single member without expanding. Alt is tracked in selection modifier keys alongside meta/ctrl/shift.

UI: Optional Group / Ungroup on NodeActionMenu and the 2D/3D multi-select floating pills and docked multi-selection panel (with group label when the selection matches a session group). Help text and keyboard shortcut docs updated.

State: New session-groups helpers and useSessionGroups zustand store; groups are cleared on scene load/switch and version preview. Stored membership is not rewritten when nodes are deleted—reads filter by live scene ids so delete+undo can restore group membership.

Docs: wiki/architecture/selection-groups.md and links from selection-managers / architecture index.

Reviewed by Cursor Bugbot for commit c804e3f. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread packages/editor/src/store/use-session-groups.ts
Comment thread packages/editor/src/components/editor/group-actions.ts Outdated
@ActArtech

Copy link
Copy Markdown
Contributor Author

Addressed Bugbot findings on this PR:

  • High - floorplan Cmd/Ctrl+click no-drag path now passes { shouldToggle: true, isolateMember: false } to applyEntrySelection (was a bare boolean, so toggle never ran).
  • Medium - cutSelectionToEditorClipboard prunes session groups after delete (same as deleteSelection).
  • Medium - clearGroups() on every scene load / project switch so session groups do not leak across scenes.

Comment thread packages/editor/src/components/editor/index.tsx
@ActArtech

Copy link
Copy Markdown
Contributor Author

Addressed the latest Bugbot finding:

  • Preview mode keeps session groups - entering version preview now calls clearGroups() before applying the preview scene graph, so edit-session groups cannot expand selection in preview.

Comment thread packages/editor/src/components/editor/selection-manager.tsx

@Aymericr Aymericr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core of this is good work, and the part I want to single out is the layering: session-groups.ts is pure, takes liveIds as a parameter rather than reaching for a store, and every function returns new arrays instead of mutating. That's why I could probe its behavior directly, and it's the reason the 25 tests are worth something. wiki/architecture/selection-groups.md naming persistent groups as explicitly out of scope is also the right instinct — it stops the next person assuming these save.

I ran the gates: bun run check fails on 9 errors, all mechanical (check:fix clears them). check-types fails on two real errors, both one-liners:

  • selection-manager.tsx:775useRef<SelectionModifierKeys>({meta, ctrl, shift}) is missing alt, which you added to the type at selection-routing.ts:13.
  • floorplan-registry-layer.tsx:597nextSelectedIds is AnyNodeId[], but the toggle branch feeds it currentSelectedIds.filter(...), which widens to string[]. The else branch already casts; the toggle branch needs the same treatment (or better, type the local as AnyNodeId[] and cast once at the expandSessionSelectionForNode boundary, since that helper returns string[]).

main typechecks clean, so both are from this branch. Your three new test files pass (25/25) — bun run test doesn't exist on your base; #548 added it after you branched, so a rebase will pick up the harness (you're 18 commits behind, and #579 moved to Next 16.3).

One design issue I'd like changed before this lands: delete is destructive to group membership, and undo doesn't restore it.

removeDeletedIdsFromSessionGroups (use-session-groups.ts:45) commits the pruned result back into the store. I ran the pure helpers to confirm what that costs:

group [a,b,c]; delete a        -> stored group becomes [b,c]
undo (a is live again)         -> stored group is still [b,c]
click a                        -> expands to null (a is no longer a member)

group [a,b,c]; delete a and b  -> group drops below the 2-member floor, removed entirely
undo                           -> group is gone for good

Deleting one member of a group and pressing Cmd+Z silently leaves it outside the group; delete two and the group is unrecoverable. Since these are session-only, there's no way to get it back short of re-grouping.

The fix is to delete code rather than add it. Pruning is already lazy at read time — pruneSessionGroups filters on liveIds (:80) and expandSessionGroupMembers re-filters (:172) — so if the store simply keeps full membership and every read filters, undo works with no hook on any delete path. Same probe with nothing committed:

delete a, click b   -> [b,c]        (a correctly excluded while dead)
delete a, click a   -> [a,b,c]      (a isn't clickable while deleted, so unreachable)
undo, click a       -> [a,b,c]      (membership never lost)
delete a and b      -> null         (below the floor, correctly inert)
undo                -> [a,b,c]      (group intact)

That drops removeDeletedIdsFromSessionGroups and its two call sites in group-actions.ts (:566, :586), and closes a coverage gap those hooks have anyway: deleteNodes is also called from editor-commands.tsx:154, zone-panel/index.tsx:165 and parametric-inspector.tsx:115, none of which call the prune hook — so membership is already inconsistent depending on how you delete. A read-time filter is correct for all of them at once. The one thing to keep committing is clearGroups() on scene load and version preview (editor/index.tsx:1202, :1240) — different node ids entirely, so stale membership there is genuinely garbage.

While you're in that file: pruneSessionGroupsToScene (:41) has no callers anywhere in the repo. It should go too.

A relevancy question I'd like your read on. packages/core already has a Collection type — {id, name, color?, nodeIds, controlNodeId?} (packages/core/src/schema/collections.ts) — with full store actions (createCollection, addToCollection, …) and a CollectionsPopover. It persists in project JSON, and it's nearly what a session group is, minus the persistence and the click-to-expand. The popover is exported from packages/editor but mounted by no app, so collections are effectively dormant UI over a live data model.

I don't think that makes this PR redundant — expand-on-click is the actual feature here and collections have no selection semantics at all — but two parallel grouping concepts in the same editor will confuse users the moment collections get a real surface, and your wiki page doesn't mention them. Before merging I'd want either a paragraph in selection-groups.md on how the two relate (and which one a user should reach for), or an argument that session groups should read/write collections and get persistence for free. Your call, and I'd genuinely like your opinion — you've been in this code more recently than the collections work.

Smaller notes, none blocking:

  • The capture-phase handler is justified, and I checked the neighbours: floorplan-hotkey-handlers.tsx:41, floorplan-group-move.tsx:384 and floorplan-registry-move-overlay.tsx:436 all use capture too, so this matches the local idiom. Two details though. The disabled guard is handled (the early return at :176 covers the whole effect), but e.stopPropagation() on a capture listener kills the event for every handler below, including those three. Ctrl/Cmd+G isn't a chord any of them claim, so it's fine today — worth a comment saying so, because it won't be obvious to whoever adds the next capture listener. And e.code === 'KeyG' || e.key.toLowerCase() === 'g' is broader than the comment claims: e.code is the layout-stable check, and the e.key fallback re-introduces exactly the layout dependence the comment says it avoids. Pick one.
  • contenteditable in the typing guard is a genuine fix beyond the stated scope — the bubble handler at :217 had the same hole. Good catch; worth calling out in the description so it isn't mistaken for drive-by scope.
  • Alt+click: I checked for collisions and the modifier is free in selection context (existing Alt bindings are all snap-bypass in measurement/number inputs), and both surfaces route it — floorplan-registry-layer.tsx:630 gates on event.altKey && !(meta||ctrl||shift), and 3D goes through resolveSelectedIdsForNodeClick. Parity is respected, which is what I'd have checked first.
  • getSelectionModifierKeys in floorplan-panel.tsx:842 still builds {meta, ctrl, shift} with no alt. It's structurally compatible so it typechecks, but it means 2D paths going through that helper can't see Alt. Worth aligning while the type is fresh.

To summarize what I need: fix the two type errors, run check:fix, rebase onto main, replace the destructive prune with read-time filtering (dropping the two delete hooks and the dead export), and add a line to the wiki page on collections. The pure-module design is what makes the prune change small — it's a deletion, not a rewrite. Happy to look again quickly once it's up.

alaa541 and others added 4 commits August 4, 2026 16:12
Add editor-only session selection groups so multi-select furniture/items can be regrouped with Ctrl/Cmd+G and reselected by plain click. Group/Ungroup icons sit on the multi-select floating pill and side panel. Not scene-graph; not saved with the project.
Clear session groups on scene load, prune memberships on cut, and pass
a proper options object for floorplan Cmd/Ctrl+click toggle.
Preview only applied the preview graph; groups from the edit session
could still expand selection. Clear groups on preview entry.
Deleting a group member committed the pruned membership back to the store, so
Cmd+Z restored the node outside its group — and deleting two of three dropped the
group past the two-member floor for good, unrecoverable since session groups are
not in the undo history.

Every read already filtered against the live scene, so keeping full membership and
filtering only at read fixes undo without new state: the delete hooks in
group-actions and the unused pruneSessionGroupsToScene export both go away, and
the several other deleteNodes callers that never called the hook are now correct
too. Renames pruneSessionGroups to liveSessionGroups to say what it is.

Also:
- add the missing `alt` to SelectionModifierKeys initializers, and track Alt in
  the 3D modifier ref so Alt+click actually reaches the 3D path
- thread expand + Alt through resolveFloorplanBackgroundSelection, the 2D
  background hit-test path that bypassed group expand entirely
- narrow the Ctrl/Cmd+G capture handler to e.code, and note why its
  stopPropagation is safe against the sibling capture listeners
- document how session groups relate to the persisted collections concept

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Aymericr
Aymericr force-pushed the feat/editor-session-groups branch from 100dcfd to c804e3f Compare August 4, 2026 20:31
@Aymericr

Aymericr commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

I've pushed the changes to your branch (c804e3f) rather than leaving you a chore list — the shape of the fix was clear from your own code, and it's a deletion. Rebased onto main too. Please look it over; if you disagree with any of it, say so and I'll revert that part.

One correction to my review above. I said the collections UI had no consumers. That's wrong — packages/nodes/src/item/parametrics.ts:14 registers the item panel via customPanel, and that panel mounts CollectionsPopover in a "Collections" section (item/panel.tsx:299). So collections are a live, reachable feature, which makes the overlap question more pointed, not less. I resolved it in the wiki rather than in code: selection-groups.md now carries a comparison table and states why Ctrl+G deliberately does not persist — an unnamed Group 4 landing in everyone's saved project on a stray keypress is a worse outcome than two concepts. If you think session groups should be the unsaved tier of collections instead, that's a reasonable argument and worth a follow-up issue; it isn't this PR.

What I changed:

The prune fix, as described. removeDeletedIdsFromSessionGroups and its two call sites are gone, pruneSessionGroupsToScene is gone, and pruneSessionGroups is renamed liveSessionGroups to say what it now is: a read-time view, not a mutation. I also made createSessionGroup and ungroupSessionSelection stop returning pruned lists — they were quietly destructive too, since the store commits whatever they return. ungroupSessionSelection now skips groups that aren't live-viable, so Ctrl+Shift+G can't dissolve a group the user can't see. clearGroups() on scene load and version preview stays, as discussed.

Tests: replaced the removeMembersFromSessionGroups test with one that walks the actual scenario — delete a member, confirm reads narrow, confirm the group goes inert at one live member, then confirm it comes back whole. Plus one for the inert-ungroup case and one pinning the "grouping a member dissolves its old group" behavior your kept filter implements. 592 editor tests pass, 944 in packages/nodes.

Alt didn't actually work in 3D. Beyond the type error, modifierKeysRef never tracked Alt — the keydown/keyup handlers only set meta/ctrl/shift (selection-manager.tsx:1194). Since computeNextIds falls back to that ref when the event doesn't carry the flag, Alt+click on the 3D path was reaching resolveSelectedIdsForNodeClick as alt: false and expanding anyway. Added Alt to all three (down, up, blur-clear).

A third click path was missing expand. 2D has two routes, not one: registry entries go through applyEntrySelection, but anything the registry layer doesn't catch falls to the SVG background handler → resolveFloorplanBackgroundSelection, which built its own selection array and knew nothing about groups. So group expand worked on registry-rendered items and silently didn't on the rest. That helper now takes the same expandIdsForNode callback and honors Alt, with two tests. This is the parity rule (AGENTS.md: 2D and 3D must share applicable behavior) applied within 2D itself. getSelectionModifierKeys in floorplan-panel.tsx now carries alt, which is what feeds it.

Keyboard. Dropped the e.key.toLowerCase() === 'g' half of the guard — e.code alone is the layout-stable check the comment claims, and the fallback undid it. Expanded the comment to record why stopPropagation on a capture listener is safe here (three sibling capture listeners exist; none claim Ctrl/Cmd+G) so the next person adding one has the constraint in front of them.

I left your contenteditable guard, the capture-phase approach, the pill/panel placement, the label scheme and the shortcut-dialog entry alone — all fine as submitted. Once CI is green I'll merge. Thanks for the clean pure-module split; it's the reason this review could be evidence-based rather than speculative.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c804e3f. Configure here.

canSelectElementFloorplanGeometry,
canSelectFloorplanZones,
currentSelectedIds: useViewer.getState().selection.selectedIds,
expandIdsForNode: expandSessionSelectionForNode,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marquee click skips group expand

Medium Severity

When the floor-plan selection tool is marquee, registry entry handlers are disabled and no-drag clicks go through toggleFloorplanSelection, which always commits a single id. That path never calls expandSessionSelectionForNode and never honors Alt isolate, so plain click does not reselect a session group in marquee mode even though click-tool, 3D, and tree paths do.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c804e3f. Configure here.

} else {
groupCurrentSelection()
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ctrl+G leaks clean Ctrl tap

Medium Severity

The capture-phase Ctrl/Cmd+G handler calls stopPropagation, so the bubble handleKeyDown never clears ctrlTapClean when G is pressed. Releasing Ctrl/Meta then still looks like a clean tap and can call cycleGridSnapStep whenever a snap context is active.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c804e3f. Configure here.

@Aymericr
Aymericr merged commit 0ae2ce0 into pascalorg:main Aug 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants