From 7ea1fc2d6919a4d55bccb7a23941405a2e1618f5 Mon Sep 17 00:00:00 2001
From: Pouzor <remy.jardinet@gmail.com>
Date: Wed, 10 Jun 2026 16:50:36 +0200
Subject: [PATCH 1/2] feat: editable node groups (add/remove members,
 description)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make Node Groups mutable instead of fixed-at-creation:
- removeFromGroup / addToGroup store actions (inverse pair, history-aware)
- right panel: per-member remove button + editable group description
  (reuses data.notes, no backend/serializer change)
- drag a node over a group → confirm modal to add it (getIntersectingNodes)

Tests: +21 (store, panel, canvas detection, modal).

ha-relevant: yes
---
 frontend/src/App.tsx                          |  16 ++-
 .../src/components/canvas/CanvasContainer.tsx |  18 ++-
 .../canvas/__tests__/CanvasContainer.test.tsx |  55 ++++++++-
 .../modals/ConfirmAddToGroupModal.tsx         |  47 ++++++++
 .../__tests__/ConfirmAddToGroupModal.test.tsx |  38 +++++++
 .../src/components/panels/DetailPanel.tsx     |  61 ++++++++--
 .../panels/__tests__/GroupPanels.test.tsx     |  42 +++++++
 .../src/stores/__tests__/canvasStore.test.ts  | 107 ++++++++++++++++++
 frontend/src/stores/canvasStore.ts            |  73 ++++++++++++
 9 files changed, 443 insertions(+), 14 deletions(-)
 create mode 100644 frontend/src/components/modals/ConfirmAddToGroupModal.tsx
 create mode 100644 frontend/src/components/modals/__tests__/ConfirmAddToGroupModal.test.tsx

diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
index afca1b5..91c0cd7 100644
--- a/frontend/src/App.tsx
+++ b/frontend/src/App.tsx
@@ -28,6 +28,7 @@ import { ThemeModal } from '@/components/modals/ThemeModal'
 import { SearchModal } from '@/components/modals/SearchModal'
 import { PendingDevicesModal } from '@/components/modals/PendingDevicesModal'
 import { ShortcutsModal } from '@/components/modals/ShortcutsModal'
+import { ConfirmAddToGroupModal } from '@/components/modals/ConfirmAddToGroupModal'
 import { useCanvasStore } from '@/stores/canvasStore'
 import { useDesignStore } from '@/stores/designStore'
 import { useAuthStore } from '@/stores/authStore'
@@ -42,7 +43,7 @@ const STANDALONE = import.meta.env.VITE_STANDALONE === 'true'
 const STANDALONE_STORAGE_KEY = 'homelable_canvas'
 
 export default function App() {
-  const { loadCanvas, markSaved, markUnsaved, selectedNodeId, selectedNodeIds, addNode, updateNode, deleteNode, onConnect, updateEdge, deleteEdge, setProxmoxContainerMode, setNodeZIndex, editingGroupRectId, setEditingGroupRectId, editingTextId, setEditingTextId, nodes, edges, snapshotHistory, undo, redo } = useCanvasStore()
+  const { loadCanvas, markSaved, markUnsaved, selectedNodeId, selectedNodeIds, addNode, updateNode, deleteNode, onConnect, updateEdge, deleteEdge, setProxmoxContainerMode, setNodeZIndex, editingGroupRectId, setEditingGroupRectId, editingTextId, setEditingTextId, nodes, edges, snapshotHistory, undo, redo, addToGroup } = useCanvasStore()
   const canvasRef = useRef<HTMLDivElement>(null)
   const { isAuthenticated } = useAuthStore()
   const { activeTheme, setTheme, customStyle, setCustomStyle } = useThemeStore()
@@ -68,6 +69,7 @@ export default function App() {
   const [addTextOpen, setAddTextOpen] = useState(false)
   const [editNodeId, setEditNodeId] = useState<string | null>(null)
   const [pendingConnection, setPendingConnection] = useState<Connection | null>(null)
+  const [pendingGroupAdd, setPendingGroupAdd] = useState<{ nodeId: string; groupId: string } | null>(null)
   const [editEdgeId, setEditEdgeId] = useState<string | null>(null)
   const [scanConfigOpen, setScanConfigOpen] = useState(false)
   const [settingsOpen, setSettingsOpen] = useState(false)
@@ -631,6 +633,7 @@ export default function App() {
                   onEdgeDoubleClick={handleEdgeDoubleClick}
                   onNodeDoubleClick={handleNodeDoubleClick}
                   onNodeDragStart={snapshotHistory}
+                  onRequestAddToGroup={setPendingGroupAdd}
                   onOpenPending={(deviceId) => openPendingModal(deviceId)}
                 />
               </div>
@@ -804,6 +807,17 @@ export default function App() {
         />
         <ShortcutsModal open={shortcutsOpen} onClose={() => setShortcutsOpen(false)} />
 
+        <ConfirmAddToGroupModal
+          open={!!pendingGroupAdd}
+          nodeLabel={pendingGroupAdd ? (nodes.find((n) => n.id === pendingGroupAdd.nodeId)?.data.label ?? '') : ''}
+          groupLabel={pendingGroupAdd ? (nodes.find((n) => n.id === pendingGroupAdd.groupId)?.data.label ?? '') : ''}
+          onConfirm={() => {
+            if (pendingGroupAdd) addToGroup(pendingGroupAdd.groupId, pendingGroupAdd.nodeId)
+            setPendingGroupAdd(null)
+          }}
+          onCancel={() => setPendingGroupAdd(null)}
+        />
+
         {!STANDALONE && (
           <SettingsModal open={settingsOpen} onClose={() => setSettingsOpen(false)} />
         )}
diff --git a/frontend/src/components/canvas/CanvasContainer.tsx b/frontend/src/components/canvas/CanvasContainer.tsx
index b789680..320576a 100644
--- a/frontend/src/components/canvas/CanvasContainer.tsx
+++ b/frontend/src/components/canvas/CanvasContainer.tsx
@@ -30,10 +30,11 @@ interface CanvasContainerProps {
   onEdgeDoubleClick?: (edge: Edge<EdgeData>) => void
   onNodeDoubleClick?: (node: Node<NodeData>) => void
   onNodeDragStart?: () => void
+  onRequestAddToGroup?: (payload: { nodeId: string; groupId: string }) => void
   onOpenPending?: (deviceId: string) => void
 }
 
-export function CanvasContainer({ onConnect: onConnectProp, onEdgeDoubleClick, onNodeDoubleClick, onNodeDragStart, onOpenPending }: CanvasContainerProps) {
+export function CanvasContainer({ onConnect: onConnectProp, onEdgeDoubleClick, onNodeDoubleClick, onNodeDragStart, onRequestAddToGroup, onOpenPending }: CanvasContainerProps) {
   const [lassoMode, setLassoMode] = useState(true)
   const {
     nodes, edges,
@@ -42,7 +43,7 @@ export function CanvasContainer({ onConnect: onConnectProp, onEdgeDoubleClick, o
     fitViewPending, clearFitViewPending,
     copySelectedNodes, pasteNodes,
   } = useCanvasStore()
-  const { fitView, screenToFlowPosition } = useReactFlow()
+  const { fitView, screenToFlowPosition, getIntersectingNodes } = useReactFlow<Node<NodeData>>()
 
   // Track the last cursor position over the canvas so paste lands under it.
   const cursorRef = useRef<{ x: number; y: number } | null>(null)
@@ -125,6 +126,17 @@ export function CanvasContainer({ onConnect: onConnectProp, onEdgeDoubleClick, o
 
   const { guides, onNodeDrag, onNodeDragStop } = useAlignmentGuides()
 
+  // Drop a top-level node onto a group → ask App to confirm adding it. Runs
+  // before the alignment snap so detection uses the dropped position.
+  const handleNodeDragStop = useCallback<NonNullable<typeof onNodeDragStop>>((event, dragNode, dragNodes) => {
+    if (onRequestAddToGroup && dragNode && !dragNode.parentId &&
+        dragNode.data.type !== 'group' && dragNode.data.type !== 'groupRect') {
+      const group = getIntersectingNodes(dragNode).find((n) => n.data.type === 'group')
+      if (group) onRequestAddToGroup({ nodeId: dragNode.id, groupId: group.id })
+    }
+    onNodeDragStop(event, dragNode, dragNodes)
+  }, [onRequestAddToGroup, getIntersectingNodes, onNodeDragStop])
+
   return (
     <div className="w-full h-full" style={{ background: theme.colors.canvasBackground }} onMouseMove={onMouseMove}>
       <ReactFlow
@@ -139,7 +151,7 @@ export function CanvasContainer({ onConnect: onConnectProp, onEdgeDoubleClick, o
         onNodeDoubleClick={handleNodeDoubleClick}
         onNodeDragStart={onNodeDragStart}
         onNodeDrag={onNodeDrag}
-        onNodeDragStop={onNodeDragStop}
+        onNodeDragStop={handleNodeDragStop}
         nodeTypes={nodeTypes}
         edgeTypes={edgeTypes}
         deleteKeyCode={['Backspace', 'Delete']}
diff --git a/frontend/src/components/canvas/__tests__/CanvasContainer.test.tsx b/frontend/src/components/canvas/__tests__/CanvasContainer.test.tsx
index caf9ac8..318bb76 100644
--- a/frontend/src/components/canvas/__tests__/CanvasContainer.test.tsx
+++ b/frontend/src/components/canvas/__tests__/CanvasContainer.test.tsx
@@ -9,6 +9,9 @@ import type { NodeData, EdgeData } from '@/types'
 // Capture props passed to ReactFlow so we can test the callbacks
 let rfProps: Record<string, unknown> = {}
 
+// Hoisted holder so the mock factory can read the configurable intersection set.
+const rf = vi.hoisted(() => ({ intersecting: [] as unknown[] }))
+
 vi.mock('@xyflow/react', () => ({
   ReactFlow: (props: Record<string, unknown>) => {
     rfProps = props
@@ -21,7 +24,13 @@ vi.mock('@xyflow/react', () => ({
   ConnectionMode: { Loose: 'loose' },
   SelectionMode: { Partial: 'partial' },
   Position: { Top: 'top', Right: 'right', Bottom: 'bottom', Left: 'left' },
-  useReactFlow: () => ({ fitView: vi.fn() }),
+  useReactFlow: () => ({
+    fitView: vi.fn(),
+    screenToFlowPosition: vi.fn(),
+    getIntersectingNodes: () => rf.intersecting,
+    setNodes: vi.fn(),
+    getNodes: () => [],
+  }),
 }))
 
 vi.mock('@xyflow/react/dist/style.css', () => ({}))
@@ -42,6 +51,7 @@ function makeEdge(id: string): Edge<EdgeData> {
 describe('CanvasContainer', () => {
   beforeEach(() => {
     rfProps = {}
+    rf.intersecting = []
     useCanvasStore.setState({ nodes: [], edges: [], selectedNodeId: null })
     useThemeStore.setState({ activeTheme: 'default' })
   })
@@ -154,6 +164,49 @@ describe('CanvasContainer', () => {
     expect(rfProps.onNodeDragStart).toBe(onNodeDragStart)
   })
 
+  // ── Drag onto group → onRequestAddToGroup ─────────────────────────────────
+
+  function groupNode(id: string): Node<NodeData> {
+    return { id, type: 'group', position: { x: 0, y: 0 }, data: { label: id, type: 'group', status: 'unknown', services: [] } }
+  }
+
+  it('fires onRequestAddToGroup when a node is dropped over a group', () => {
+    const onRequestAddToGroup = vi.fn()
+    const node = makeNode('n1')
+    const group = groupNode('g1')
+    rf.intersecting = [group]
+    render(<CanvasContainer onRequestAddToGroup={onRequestAddToGroup} />)
+    ;(rfProps.onNodeDragStop as (...args: unknown[]) => unknown)({} as MouseEvent, node, [node])
+    expect(onRequestAddToGroup).toHaveBeenCalledWith({ nodeId: 'n1', groupId: 'g1' })
+  })
+
+  it('does not fire onRequestAddToGroup when no group is under the node', () => {
+    const onRequestAddToGroup = vi.fn()
+    const node = makeNode('n1')
+    rf.intersecting = [makeNode('n2')]
+    render(<CanvasContainer onRequestAddToGroup={onRequestAddToGroup} />)
+    ;(rfProps.onNodeDragStop as (...args: unknown[]) => unknown)({} as MouseEvent, node, [node])
+    expect(onRequestAddToGroup).not.toHaveBeenCalled()
+  })
+
+  it('does not fire onRequestAddToGroup for an already-parented node', () => {
+    const onRequestAddToGroup = vi.fn()
+    const node = { ...makeNode('n1'), parentId: 'gOther' }
+    rf.intersecting = [groupNode('g1')]
+    render(<CanvasContainer onRequestAddToGroup={onRequestAddToGroup} />)
+    ;(rfProps.onNodeDragStop as (...args: unknown[]) => unknown)({} as MouseEvent, node, [node])
+    expect(onRequestAddToGroup).not.toHaveBeenCalled()
+  })
+
+  it('does not fire onRequestAddToGroup when the dragged node is itself a group', () => {
+    const onRequestAddToGroup = vi.fn()
+    const node = groupNode('g2')
+    rf.intersecting = [groupNode('g1')]
+    render(<CanvasContainer onRequestAddToGroup={onRequestAddToGroup} />)
+    ;(rfProps.onNodeDragStop as (...args: unknown[]) => unknown)({} as MouseEvent, node, [node])
+    expect(onRequestAddToGroup).not.toHaveBeenCalled()
+  })
+
   // ── Canvas settings ───────────────────────────────────────────────────────
 
   it('enables snapToGrid', () => {
diff --git a/frontend/src/components/modals/ConfirmAddToGroupModal.tsx b/frontend/src/components/modals/ConfirmAddToGroupModal.tsx
new file mode 100644
index 0000000..c5f3913
--- /dev/null
+++ b/frontend/src/components/modals/ConfirmAddToGroupModal.tsx
@@ -0,0 +1,47 @@
+import { Layers } from 'lucide-react'
+import {
+  Dialog,
+  DialogContent,
+  DialogHeader,
+  DialogTitle,
+  DialogDescription,
+  DialogFooter,
+} from '@/components/ui/dialog'
+import { Button } from '@/components/ui/button'
+
+interface ConfirmAddToGroupModalProps {
+  open: boolean
+  nodeLabel: string
+  groupLabel: string
+  onConfirm: () => void
+  onCancel: () => void
+}
+
+export function ConfirmAddToGroupModal({ open, nodeLabel, groupLabel, onConfirm, onCancel }: ConfirmAddToGroupModalProps) {
+  return (
+    <Dialog open={open} onOpenChange={(o) => { if (!o) onCancel() }}>
+      <DialogContent className="max-w-sm">
+        <DialogHeader>
+          <DialogTitle className="flex items-center gap-2">
+            <Layers size={16} className="text-[#00d4ff]" />
+            Add to group
+          </DialogTitle>
+          <DialogDescription>
+            Add <span className="font-medium text-foreground">{nodeLabel}</span> to the group{' '}
+            <span className="font-medium text-foreground">{groupLabel}</span>?
+          </DialogDescription>
+        </DialogHeader>
+        <DialogFooter>
+          <Button variant="ghost" size="sm" onClick={onCancel}>Cancel</Button>
+          <Button
+            size="sm"
+            className="bg-[#00d4ff] text-[#0d1117] hover:bg-[#00d4ff]/90"
+            onClick={onConfirm}
+          >
+            Add to group
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  )
+}
diff --git a/frontend/src/components/modals/__tests__/ConfirmAddToGroupModal.test.tsx b/frontend/src/components/modals/__tests__/ConfirmAddToGroupModal.test.tsx
new file mode 100644
index 0000000..04c6a39
--- /dev/null
+++ b/frontend/src/components/modals/__tests__/ConfirmAddToGroupModal.test.tsx
@@ -0,0 +1,38 @@
+import { describe, it, expect, vi } from 'vitest'
+import { render, screen, fireEvent } from '@testing-library/react'
+import { ConfirmAddToGroupModal } from '../ConfirmAddToGroupModal'
+
+describe('ConfirmAddToGroupModal', () => {
+  it('renders nothing when closed', () => {
+    render(
+      <ConfirmAddToGroupModal open={false} nodeLabel="Router" groupLabel="DMZ" onConfirm={vi.fn()} onCancel={vi.fn()} />,
+    )
+    expect(screen.queryByText('Add to group')).toBeNull()
+  })
+
+  it('shows node and group labels when open', () => {
+    render(
+      <ConfirmAddToGroupModal open nodeLabel="Router" groupLabel="DMZ" onConfirm={vi.fn()} onCancel={vi.fn()} />,
+    )
+    expect(screen.getByText('Router')).toBeDefined()
+    expect(screen.getByText('DMZ')).toBeDefined()
+  })
+
+  it('calls onConfirm when the confirm button is clicked', () => {
+    const onConfirm = vi.fn()
+    render(
+      <ConfirmAddToGroupModal open nodeLabel="Router" groupLabel="DMZ" onConfirm={onConfirm} onCancel={vi.fn()} />,
+    )
+    fireEvent.click(screen.getByRole('button', { name: /add to group/i }))
+    expect(onConfirm).toHaveBeenCalledOnce()
+  })
+
+  it('calls onCancel when the cancel button is clicked', () => {
+    const onCancel = vi.fn()
+    render(
+      <ConfirmAddToGroupModal open nodeLabel="Router" groupLabel="DMZ" onConfirm={vi.fn()} onCancel={onCancel} />,
+    )
+    fireEvent.click(screen.getByRole('button', { name: /cancel/i }))
+    expect(onCancel).toHaveBeenCalledOnce()
+  })
+})
diff --git a/frontend/src/components/panels/DetailPanel.tsx b/frontend/src/components/panels/DetailPanel.tsx
index 190731e..5fefe6f 100644
--- a/frontend/src/components/panels/DetailPanel.tsx
+++ b/frontend/src/components/panels/DetailPanel.tsx
@@ -21,7 +21,7 @@ type PropForm = { key: string; value: string; icon: string | null; visible: bool
 const EMPTY_PROP: PropForm = { key: '', value: '', icon: null, visible: true }
 
 export function DetailPanel({ onEdit }: DetailPanelProps) {
-  const { nodes, selectedNodeId, selectedNodeIds, setSelectedNode, deleteNode, updateNode, snapshotHistory, createGroup, ungroup } = useCanvasStore()
+  const { nodes, selectedNodeId, selectedNodeIds, setSelectedNode, deleteNode, updateNode, snapshotHistory, createGroup, ungroup, removeFromGroup } = useCanvasStore()
   const serviceStatuses = useCanvasStore((s) => s.serviceStatuses)
 
   const [addingForNode, setAddingForNode] = useState<string | null>(null)
@@ -65,6 +65,11 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
         node={node}
         nodes={nodes}
         onUngroup={() => { ungroup(node.id) }}
+        onRemoveChild={(id) => { snapshotHistory(); removeFromGroup(node.id, id) }}
+        onChangeDescription={(value) => {
+          snapshotHistory()
+          updateNode(node.id, { notes: value })
+        }}
         onToggleBorder={() => {
           snapshotHistory()
           updateNode(node.id, {
@@ -426,17 +431,27 @@ interface GroupDetailPanelProps {
   node: Node<NodeData>
   nodes: Node<NodeData>[]
   onUngroup: () => void
+  onRemoveChild: (id: string) => void
+  onChangeDescription: (value: string) => void
   onToggleBorder: () => void
   onClose: () => void
   onSelectChild: (id: string) => void
 }
 
-function GroupDetailPanel({ node, nodes, onUngroup, onToggleBorder, onClose, onSelectChild }: GroupDetailPanelProps) {
+function GroupDetailPanel({ node, nodes, onUngroup, onRemoveChild, onChangeDescription, onToggleBorder, onClose, onSelectChild }: GroupDetailPanelProps) {
   const children = nodes.filter((n) => n.parentId === node.id)
   const onlineCount = children.filter((n) => n.data.status === 'online').length
   const offlineCount = children.filter((n) => n.data.status === 'offline').length
   const showBorder = node.data.custom_colors?.show_border !== false
 
+  // Description reuses data.notes, which already round-trips to the backend.
+  // Uncontrolled textarea (keyed by node id) so we only snapshot history on blur,
+  // not on every keystroke. `key` resets the field when switching groups.
+  const commitDescription = (value: string) => {
+    if (value === (node.data.notes ?? '')) return
+    onChangeDescription(value)
+  }
+
   const handleUngroup = () => {
     if (confirm(`Ungroup "${node.data.label}"? Nodes will be released to the canvas.`)) {
       onUngroup()
@@ -462,20 +477,48 @@ function GroupDetailPanel({ node, nodes, onUngroup, onToggleBorder, onClose, onS
         {offlineCount > 0 && <span style={{ color: STATUS_COLORS.offline }}>● {offlineCount} offline</span>}
       </div>
 
+      {/* Description */}
+      <div className="px-4 py-3 border-b border-border">
+        <label htmlFor="group-description" className="text-[10px] font-semibold uppercase tracking-wider text-muted-foreground/50">
+          Description
+        </label>
+        <textarea
+          id="group-description"
+          key={node.id}
+          defaultValue={node.data.notes ?? ''}
+          onBlur={(e) => commitDescription(e.target.value)}
+          placeholder="Add a description for this group…"
+          rows={3}
+          className="mt-1.5 w-full resize-y rounded-md bg-[#21262d] border border-[#30363d] px-2 py-1.5 text-xs text-foreground placeholder:text-muted-foreground/40 focus:outline-none focus:border-[#00d4ff]/50"
+        />
+      </div>
+
       {/* Children list */}
       <div className="flex-1 px-4 py-3 space-y-1.5 overflow-y-auto">
         <span className="text-[10px] font-semibold uppercase tracking-wider text-muted-foreground/50">Members</span>
         {children.length === 0 && <p className="text-xs text-muted-foreground/50">No nodes in this group.</p>}
         {children.map((child) => (
-          <button
+          <div
             key={child.id}
-            onClick={() => onSelectChild(child.id)}
-            className="w-full flex items-center gap-2 px-2 py-1.5 rounded-md bg-[#21262d] text-xs hover:bg-[#30363d] transition-colors text-left"
+            className="group/member w-full flex items-center gap-2 px-2 py-1.5 rounded-md bg-[#21262d] text-xs hover:bg-[#30363d] transition-colors"
           >
-            <span className="w-1.5 h-1.5 rounded-full shrink-0" style={{ backgroundColor: STATUS_COLORS[child.data.status] }} />
-            <span className="truncate text-foreground font-medium">{child.data.label}</span>
-            <span className="ml-auto text-muted-foreground shrink-0">{NODE_TYPE_LABELS[child.data.type] ?? child.data.type}</span>
-          </button>
+            <button
+              onClick={() => onSelectChild(child.id)}
+              className="flex items-center gap-2 min-w-0 flex-1 text-left cursor-pointer"
+            >
+              <span className="w-1.5 h-1.5 rounded-full shrink-0" style={{ backgroundColor: STATUS_COLORS[child.data.status] }} />
+              <span className="truncate text-foreground font-medium">{child.data.label}</span>
+              <span className="ml-auto text-muted-foreground shrink-0">{NODE_TYPE_LABELS[child.data.type] ?? child.data.type}</span>
+            </button>
+            <button
+              onClick={() => onRemoveChild(child.id)}
+              aria-label={`Remove ${child.data.label} from group`}
+              title="Remove from group"
+              className="shrink-0 opacity-0 group-hover/member:opacity-100 transition-opacity text-[#8b949e] hover:text-[#f85149] cursor-pointer"
+            >
+              <X size={12} />
+            </button>
+          </div>
         ))}
       </div>
 
diff --git a/frontend/src/components/panels/__tests__/GroupPanels.test.tsx b/frontend/src/components/panels/__tests__/GroupPanels.test.tsx
index 41ce35c..52d054e 100644
--- a/frontend/src/components/panels/__tests__/GroupPanels.test.tsx
+++ b/frontend/src/components/panels/__tests__/GroupPanels.test.tsx
@@ -42,6 +42,7 @@ const mockStore = {
   snapshotHistory: vi.fn(),
   createGroup: vi.fn(),
   ungroup: vi.fn(),
+  removeFromGroup: vi.fn(),
 }
 
 function setupStore(overrides = {}) {
@@ -230,4 +231,45 @@ describe('GroupDetailPanel', () => {
     fireEvent.click(screen.getByText('Child Node Alpha'))
     expect(setSelectedNode).toHaveBeenCalledWith('c1')
   })
+
+  it('removes a child from the group via the remove button', () => {
+    const removeFromGroup = vi.fn()
+    const snapshotHistory = vi.fn()
+    const group = makeGroupNode()
+    const child = makeNode('c1', { parentId: 'g1', data: { label: 'Router', type: 'router', status: 'online', services: [] } })
+    setupStore({ nodes: [group, child], selectedNodeId: 'g1', selectedNodeIds: ['g1'], removeFromGroup, snapshotHistory })
+    renderPanel()
+    fireEvent.click(screen.getByRole('button', { name: /remove router from group/i }))
+    expect(removeFromGroup).toHaveBeenCalledWith('g1', 'c1')
+    expect(snapshotHistory).toHaveBeenCalled()
+  })
+
+  it('renders the existing group description from notes', () => {
+    const group = makeGroupNode()
+    group.data = { ...group.data, notes: 'Critical DMZ hosts' } as typeof group.data
+    setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'] })
+    renderPanel()
+    expect((screen.getByLabelText('Description') as HTMLTextAreaElement).value).toBe('Critical DMZ hosts')
+  })
+
+  it('commits the description on blur', () => {
+    const updateNode = vi.fn()
+    const snapshotHistory = vi.fn()
+    const group = makeGroupNode()
+    setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode, snapshotHistory })
+    renderPanel()
+    const textarea = screen.getByLabelText('Description')
+    fireEvent.change(textarea, { target: { value: 'New notes' } })
+    fireEvent.blur(textarea)
+    expect(updateNode).toHaveBeenCalledWith('g1', { notes: 'New notes' })
+  })
+
+  it('does not commit the description on blur when unchanged', () => {
+    const updateNode = vi.fn()
+    const group = makeGroupNode()
+    setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode })
+    renderPanel()
+    fireEvent.blur(screen.getByLabelText('Description'))
+    expect(updateNode).not.toHaveBeenCalled()
+  })
 })
diff --git a/frontend/src/stores/__tests__/canvasStore.test.ts b/frontend/src/stores/__tests__/canvasStore.test.ts
index 8b8d05c..c08c8eb 100644
--- a/frontend/src/stores/__tests__/canvasStore.test.ts
+++ b/frontend/src/stores/__tests__/canvasStore.test.ts
@@ -514,6 +514,113 @@ describe('canvasStore', () => {
     expect(useCanvasStore.getState().hasUnsavedChanges).toBe(true)
   })
 
+  // ── addToGroup ──────────────────────────────────────────────────────────────
+
+  it('addToGroup nests a top-level node with parent-relative position', () => {
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 76, y: 52 }, width: 448, height: 252 }
+    const child = { ...makeNode('n1'), position: { x: 300, y: 200 } }
+    useCanvasStore.setState({ nodes: [group, child] })
+
+    useCanvasStore.getState().addToGroup('g1', 'n1')
+
+    const moved = useCanvasStore.getState().nodes.find((n) => n.id === 'n1')
+    expect(moved?.parentId).toBe('g1')
+    expect(moved?.extent).toBe('parent')
+    expect(moved?.data.parent_id).toBe('g1')
+    // 300-76=224, 200-52=148
+    expect(moved?.position).toEqual({ x: 224, y: 148 })
+  })
+
+  it('addToGroup places the group before the child in the array', () => {
+    const child = { ...makeNode('n1'), position: { x: 300, y: 200 } }
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 0, y: 0 } }
+    // child first to prove reordering
+    useCanvasStore.setState({ nodes: [child, group] })
+
+    useCanvasStore.getState().addToGroup('g1', 'n1')
+
+    const { nodes } = useCanvasStore.getState()
+    expect(nodes.findIndex((n) => n.id === 'g1')).toBeLessThan(nodes.findIndex((n) => n.id === 'n1'))
+  })
+
+  it('addToGroup is a no-op when target is not a group', () => {
+    const notGroup = { ...makeNode('s1'), position: { x: 0, y: 0 } }
+    const child = { ...makeNode('n1'), position: { x: 50, y: 50 } }
+    useCanvasStore.setState({ nodes: [notGroup, child] })
+    useCanvasStore.getState().markSaved()
+
+    useCanvasStore.getState().addToGroup('s1', 'n1')
+
+    expect(useCanvasStore.getState().nodes.find((n) => n.id === 'n1')?.parentId).toBeUndefined()
+    expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false)
+  })
+
+  it('addToGroup is a no-op when child already belongs to the group', () => {
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 0, y: 0 } }
+    const child = { ...makeNode('n1'), position: { x: 50, y: 50 }, parentId: 'g1', extent: 'parent' as const }
+    useCanvasStore.setState({ nodes: [group, child] })
+    useCanvasStore.getState().markSaved()
+
+    useCanvasStore.getState().addToGroup('g1', 'n1')
+
+    expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false)
+  })
+
+  it('addToGroup snapshots history and marks unsaved', () => {
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 0, y: 0 } }
+    const child = { ...makeNode('n1'), position: { x: 50, y: 50 } }
+    useCanvasStore.setState({ nodes: [group, child] })
+    useCanvasStore.getState().markSaved()
+
+    useCanvasStore.getState().addToGroup('g1', 'n1')
+
+    expect(useCanvasStore.getState().past).toHaveLength(1)
+    expect(useCanvasStore.getState().hasUnsavedChanges).toBe(true)
+  })
+
+  // ── removeFromGroup ─────────────────────────────────────────────────────────
+
+  it('removeFromGroup releases the child to absolute coords and keeps the group', () => {
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 76, y: 52 } }
+    const child = { ...makeNode('n1'), position: { x: 224, y: 148 }, parentId: 'g1', extent: 'parent' as const }
+    useCanvasStore.setState({ nodes: [group, child] })
+
+    useCanvasStore.getState().removeFromGroup('g1', 'n1')
+
+    const { nodes } = useCanvasStore.getState()
+    const released = nodes.find((n) => n.id === 'n1')
+    expect(released?.parentId).toBeUndefined()
+    expect(released?.extent).toBeUndefined()
+    expect(released?.data.parent_id).toBeUndefined()
+    // 224+76=300, 148+52=200
+    expect(released?.position).toEqual({ x: 300, y: 200 })
+    // group survives
+    expect(nodes.find((n) => n.id === 'g1')).toBeDefined()
+  })
+
+  it('removeFromGroup is a no-op when child is not in the group', () => {
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 0, y: 0 } }
+    const child = { ...makeNode('n1'), position: { x: 50, y: 50 } }
+    useCanvasStore.setState({ nodes: [group, child] })
+    useCanvasStore.getState().markSaved()
+
+    useCanvasStore.getState().removeFromGroup('g1', 'n1')
+
+    expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false)
+  })
+
+  it('removeFromGroup snapshots history and marks unsaved', () => {
+    const group = { ...makeNode('g1', { type: 'group', label: 'G' }), position: { x: 0, y: 0 } }
+    const child = { ...makeNode('n1'), position: { x: 50, y: 50 }, parentId: 'g1', extent: 'parent' as const }
+    useCanvasStore.setState({ nodes: [group, child] })
+    useCanvasStore.getState().markSaved()
+
+    useCanvasStore.getState().removeFromGroup('g1', 'n1')
+
+    expect(useCanvasStore.getState().past).toHaveLength(1)
+    expect(useCanvasStore.getState().hasUnsavedChanges).toBe(true)
+  })
+
   it('updateEdge updates edge data and marks unsaved', () => {
     useCanvasStore.setState((s) => ({ edges: [...s.edges, makeEdge('e1', 'n1', 'n2')] }))
     useCanvasStore.getState().markSaved()
diff --git a/frontend/src/stores/canvasStore.ts b/frontend/src/stores/canvasStore.ts
index 24d8fa0..309c137 100644
--- a/frontend/src/stores/canvasStore.ts
+++ b/frontend/src/stores/canvasStore.ts
@@ -68,6 +68,8 @@ interface CanvasState {
   toggleNodeCollapsed: (id: string) => void
   createGroup: (nodeIds: string[], name: string) => void
   ungroup: (groupId: string) => void
+  addToGroup: (groupId: string, childId: string) => void
+  removeFromGroup: (groupId: string, childId: string) => void
   markSaved: () => void
   markUnsaved: () => void
   loadCanvas: (nodes: Node<NodeData>[], edges: Edge<EdgeData>[]) => void
@@ -583,6 +585,77 @@ export const useCanvasStore = create<CanvasState>((set) => ({
       }
     }),
 
+  // Nest an existing top-level node inside a group. Inverse of removeFromGroup.
+  addToGroup: (groupId, childId) =>
+    set((state) => {
+      const group = state.nodes.find((n) => n.id === groupId)
+      const child = state.nodes.find((n) => n.id === childId)
+      if (!group || !child || group.data.type !== 'group') return state
+      if (child.id === groupId || child.parentId === groupId) return state
+
+      const updatedNodes = state.nodes.map((n) => {
+        if (n.id !== childId) return n
+        return {
+          ...n,
+          parentId: groupId,
+          extent: 'parent' as const,
+          // Absolute → group-relative. Clamp so the node stays inside the box.
+          position: {
+            x: Math.max(8, n.position.x - group.position.x),
+            y: Math.max(8, n.position.y - group.position.y),
+          },
+          selected: false,
+          data: { ...n.data, parent_id: groupId },
+        }
+      })
+
+      // React Flow requires the parent to precede its children in the array.
+      const others = updatedNodes.filter((n) => n.id !== childId)
+      const movedChild = updatedNodes.find((n) => n.id === childId)!
+      const groupIdx = others.findIndex((n) => n.id === groupId)
+      const nodes = [
+        ...others.slice(0, groupIdx + 1),
+        movedChild,
+        ...others.slice(groupIdx + 1),
+      ]
+
+      return {
+        nodes,
+        hasUnsavedChanges: true,
+        past: [...state.past.slice(-49), { nodes: state.nodes, edges: state.edges }],
+        future: [],
+      }
+    }),
+
+  // Release a single child from a group back to the canvas. Group stays.
+  removeFromGroup: (groupId, childId) =>
+    set((state) => {
+      const group = state.nodes.find((n) => n.id === groupId)
+      const child = state.nodes.find((n) => n.id === childId)
+      if (!group || !child || child.parentId !== groupId) return state
+
+      const nodes = state.nodes.map((n) => {
+        if (n.id !== childId) return n
+        return {
+          ...n,
+          parentId: undefined,
+          extent: undefined,
+          position: {
+            x: n.position.x + group.position.x,
+            y: n.position.y + group.position.y,
+          },
+          data: { ...n.data, parent_id: undefined },
+        }
+      })
+
+      return {
+        nodes,
+        hasUnsavedChanges: true,
+        past: [...state.past.slice(-49), { nodes: state.nodes, edges: state.edges }],
+        future: [],
+      }
+    }),
+
   markSaved: () => set({ hasUnsavedChanges: false }),
 
   markUnsaved: () => set({ hasUnsavedChanges: true }),

From fe3787f0cd7c84b4799d3478fb06c35dfa742ae7 Mon Sep 17 00:00:00 2001
From: Pouzor <remy.jardinet@gmail.com>
Date: Thu, 11 Jun 2026 01:48:22 +0200
Subject: [PATCH 2/2] fix: persist group description with Ctrl+S, not just on
 blur

The description field committed to the store only on textarea blur, so
saving via Ctrl+S (the primary shortcut, which never blurs the field)
dropped the edit. Make it controlled and commit on each change; snapshot
history once per edit session (on focus) so undo stays a single step.

ha-relevant: yes
---
 .../src/components/panels/DetailPanel.tsx     | 30 +++++++++++--------
 .../panels/__tests__/GroupPanels.test.tsx     | 27 +++++++++--------
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/frontend/src/components/panels/DetailPanel.tsx b/frontend/src/components/panels/DetailPanel.tsx
index 5fefe6f..6508675 100644
--- a/frontend/src/components/panels/DetailPanel.tsx
+++ b/frontend/src/components/panels/DetailPanel.tsx
@@ -1,4 +1,4 @@
-import { createElement, useState } from 'react'
+import { createElement, useRef, useState } from 'react'
 import { X, Edit, Trash2, ExternalLink, Plus, Pencil, Layers, Ungroup, Eye, EyeOff } from 'lucide-react'
 import { Button } from '@/components/ui/button'
 import { Input } from '@/components/ui/input'
@@ -66,10 +66,8 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
         nodes={nodes}
         onUngroup={() => { ungroup(node.id) }}
         onRemoveChild={(id) => { snapshotHistory(); removeFromGroup(node.id, id) }}
-        onChangeDescription={(value) => {
-          snapshotHistory()
-          updateNode(node.id, { notes: value })
-        }}
+        onChangeDescription={(value) => updateNode(node.id, { notes: value })}
+        onSnapshotBeforeEdit={snapshotHistory}
         onToggleBorder={() => {
           snapshotHistory()
           updateNode(node.id, {
@@ -433,22 +431,28 @@ interface GroupDetailPanelProps {
   onUngroup: () => void
   onRemoveChild: (id: string) => void
   onChangeDescription: (value: string) => void
+  onSnapshotBeforeEdit: () => void
   onToggleBorder: () => void
   onClose: () => void
   onSelectChild: (id: string) => void
 }
 
-function GroupDetailPanel({ node, nodes, onUngroup, onRemoveChild, onChangeDescription, onToggleBorder, onClose, onSelectChild }: GroupDetailPanelProps) {
+function GroupDetailPanel({ node, nodes, onUngroup, onRemoveChild, onChangeDescription, onSnapshotBeforeEdit, onToggleBorder, onClose, onSelectChild }: GroupDetailPanelProps) {
   const children = nodes.filter((n) => n.parentId === node.id)
   const onlineCount = children.filter((n) => n.data.status === 'online').length
   const offlineCount = children.filter((n) => n.data.status === 'offline').length
   const showBorder = node.data.custom_colors?.show_border !== false
 
   // Description reuses data.notes, which already round-trips to the backend.
-  // Uncontrolled textarea (keyed by node id) so we only snapshot history on blur,
-  // not on every keystroke. `key` resets the field when switching groups.
-  const commitDescription = (value: string) => {
-    if (value === (node.data.notes ?? '')) return
+  // Controlled + committed on every keystroke so ANY save path (incl. Ctrl+S,
+  // which never blurs the field) captures it. History is snapshotted once at the
+  // start of an edit session so the whole edit is a single undo step.
+  const snappedRef = useRef(false)
+  const handleDescriptionChange = (value: string) => {
+    if (!snappedRef.current) {
+      onSnapshotBeforeEdit()
+      snappedRef.current = true
+    }
     onChangeDescription(value)
   }
 
@@ -484,9 +488,9 @@ function GroupDetailPanel({ node, nodes, onUngroup, onRemoveChild, onChangeDescr
         </label>
         <textarea
           id="group-description"
-          key={node.id}
-          defaultValue={node.data.notes ?? ''}
-          onBlur={(e) => commitDescription(e.target.value)}
+          value={node.data.notes ?? ''}
+          onFocus={() => { snappedRef.current = false }}
+          onChange={(e) => handleDescriptionChange(e.target.value)}
           placeholder="Add a description for this group…"
           rows={3}
           className="mt-1.5 w-full resize-y rounded-md bg-[#21262d] border border-[#30363d] px-2 py-1.5 text-xs text-foreground placeholder:text-muted-foreground/40 focus:outline-none focus:border-[#00d4ff]/50"
diff --git a/frontend/src/components/panels/__tests__/GroupPanels.test.tsx b/frontend/src/components/panels/__tests__/GroupPanels.test.tsx
index 52d054e..ca73963 100644
--- a/frontend/src/components/panels/__tests__/GroupPanels.test.tsx
+++ b/frontend/src/components/panels/__tests__/GroupPanels.test.tsx
@@ -252,24 +252,25 @@ describe('GroupDetailPanel', () => {
     expect((screen.getByLabelText('Description') as HTMLTextAreaElement).value).toBe('Critical DMZ hosts')
   })
 
-  it('commits the description on blur', () => {
+  it('commits the description to the store on each change (so Ctrl+S captures it)', () => {
+    const updateNode = vi.fn()
+    const group = makeGroupNode()
+    setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode })
+    renderPanel()
+    fireEvent.change(screen.getByLabelText('Description'), { target: { value: 'New notes' } })
+    expect(updateNode).toHaveBeenCalledWith('g1', { notes: 'New notes' })
+  })
+
+  it('snapshots history once at the start of an edit, not on every keystroke', () => {
     const updateNode = vi.fn()
     const snapshotHistory = vi.fn()
     const group = makeGroupNode()
     setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode, snapshotHistory })
     renderPanel()
     const textarea = screen.getByLabelText('Description')
-    fireEvent.change(textarea, { target: { value: 'New notes' } })
-    fireEvent.blur(textarea)
-    expect(updateNode).toHaveBeenCalledWith('g1', { notes: 'New notes' })
-  })
-
-  it('does not commit the description on blur when unchanged', () => {
-    const updateNode = vi.fn()
-    const group = makeGroupNode()
-    setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode })
-    renderPanel()
-    fireEvent.blur(screen.getByLabelText('Description'))
-    expect(updateNode).not.toHaveBeenCalled()
+    fireEvent.change(textarea, { target: { value: 'a' } })
+    fireEvent.change(textarea, { target: { value: 'ab' } })
+    fireEvent.change(textarea, { target: { value: 'abc' } })
+    expect(snapshotHistory).toHaveBeenCalledTimes(1)
   })
 })