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
This commit is contained in:
@@ -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 { X, Edit, Trash2, ExternalLink, Plus, Pencil, Layers, Ungroup, Eye, EyeOff } from 'lucide-react'
|
||||||
import { Button } from '@/components/ui/button'
|
import { Button } from '@/components/ui/button'
|
||||||
import { Input } from '@/components/ui/input'
|
import { Input } from '@/components/ui/input'
|
||||||
@@ -66,10 +66,8 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
|||||||
nodes={nodes}
|
nodes={nodes}
|
||||||
onUngroup={() => { ungroup(node.id) }}
|
onUngroup={() => { ungroup(node.id) }}
|
||||||
onRemoveChild={(id) => { snapshotHistory(); removeFromGroup(node.id, id) }}
|
onRemoveChild={(id) => { snapshotHistory(); removeFromGroup(node.id, id) }}
|
||||||
onChangeDescription={(value) => {
|
onChangeDescription={(value) => updateNode(node.id, { notes: value })}
|
||||||
snapshotHistory()
|
onSnapshotBeforeEdit={snapshotHistory}
|
||||||
updateNode(node.id, { notes: value })
|
|
||||||
}}
|
|
||||||
onToggleBorder={() => {
|
onToggleBorder={() => {
|
||||||
snapshotHistory()
|
snapshotHistory()
|
||||||
updateNode(node.id, {
|
updateNode(node.id, {
|
||||||
@@ -433,22 +431,28 @@ interface GroupDetailPanelProps {
|
|||||||
onUngroup: () => void
|
onUngroup: () => void
|
||||||
onRemoveChild: (id: string) => void
|
onRemoveChild: (id: string) => void
|
||||||
onChangeDescription: (value: string) => void
|
onChangeDescription: (value: string) => void
|
||||||
|
onSnapshotBeforeEdit: () => void
|
||||||
onToggleBorder: () => void
|
onToggleBorder: () => void
|
||||||
onClose: () => void
|
onClose: () => void
|
||||||
onSelectChild: (id: string) => 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 children = nodes.filter((n) => n.parentId === node.id)
|
||||||
const onlineCount = children.filter((n) => n.data.status === 'online').length
|
const onlineCount = children.filter((n) => n.data.status === 'online').length
|
||||||
const offlineCount = children.filter((n) => n.data.status === 'offline').length
|
const offlineCount = children.filter((n) => n.data.status === 'offline').length
|
||||||
const showBorder = node.data.custom_colors?.show_border !== false
|
const showBorder = node.data.custom_colors?.show_border !== false
|
||||||
|
|
||||||
// Description reuses data.notes, which already round-trips to the backend.
|
// Description reuses data.notes, which already round-trips to the backend.
|
||||||
// Uncontrolled textarea (keyed by node id) so we only snapshot history on blur,
|
// Controlled + committed on every keystroke so ANY save path (incl. Ctrl+S,
|
||||||
// not on every keystroke. `key` resets the field when switching groups.
|
// which never blurs the field) captures it. History is snapshotted once at the
|
||||||
const commitDescription = (value: string) => {
|
// start of an edit session so the whole edit is a single undo step.
|
||||||
if (value === (node.data.notes ?? '')) return
|
const snappedRef = useRef(false)
|
||||||
|
const handleDescriptionChange = (value: string) => {
|
||||||
|
if (!snappedRef.current) {
|
||||||
|
onSnapshotBeforeEdit()
|
||||||
|
snappedRef.current = true
|
||||||
|
}
|
||||||
onChangeDescription(value)
|
onChangeDescription(value)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -484,9 +488,9 @@ function GroupDetailPanel({ node, nodes, onUngroup, onRemoveChild, onChangeDescr
|
|||||||
</label>
|
</label>
|
||||||
<textarea
|
<textarea
|
||||||
id="group-description"
|
id="group-description"
|
||||||
key={node.id}
|
value={node.data.notes ?? ''}
|
||||||
defaultValue={node.data.notes ?? ''}
|
onFocus={() => { snappedRef.current = false }}
|
||||||
onBlur={(e) => commitDescription(e.target.value)}
|
onChange={(e) => handleDescriptionChange(e.target.value)}
|
||||||
placeholder="Add a description for this group…"
|
placeholder="Add a description for this group…"
|
||||||
rows={3}
|
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"
|
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"
|
||||||
|
|||||||
@@ -252,24 +252,25 @@ describe('GroupDetailPanel', () => {
|
|||||||
expect((screen.getByLabelText('Description') as HTMLTextAreaElement).value).toBe('Critical DMZ hosts')
|
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 updateNode = vi.fn()
|
||||||
const snapshotHistory = vi.fn()
|
const snapshotHistory = vi.fn()
|
||||||
const group = makeGroupNode()
|
const group = makeGroupNode()
|
||||||
setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode, snapshotHistory })
|
setupStore({ nodes: [group], selectedNodeId: 'g1', selectedNodeIds: ['g1'], updateNode, snapshotHistory })
|
||||||
renderPanel()
|
renderPanel()
|
||||||
const textarea = screen.getByLabelText('Description')
|
const textarea = screen.getByLabelText('Description')
|
||||||
fireEvent.change(textarea, { target: { value: 'New notes' } })
|
fireEvent.change(textarea, { target: { value: 'a' } })
|
||||||
fireEvent.blur(textarea)
|
fireEvent.change(textarea, { target: { value: 'ab' } })
|
||||||
expect(updateNode).toHaveBeenCalledWith('g1', { notes: 'New notes' })
|
fireEvent.change(textarea, { target: { value: 'abc' } })
|
||||||
})
|
expect(snapshotHistory).toHaveBeenCalledTimes(1)
|
||||||
|
|
||||||
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()
|
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user