fix: null guards, aria-labels, and missing tests for DetailPanel
- Extract const services = data.services ?? [] for consistent null safety - Add aria-label to close and delete buttons - Add tests: close, edit callback, delete (confirm/cancel), add service, remove service, undefined services
This commit is contained in:
@@ -29,6 +29,7 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
const editingIndex = editingFor?.nodeId === node.id ? editingFor.index : null
|
||||
|
||||
const { data } = node
|
||||
const services = data.services ?? []
|
||||
const statusColor = STATUS_COLORS[data.status]
|
||||
const host = data.ip ?? data.hostname
|
||||
|
||||
@@ -46,19 +47,19 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
protocol: newSvc.protocol,
|
||||
service_name: newSvc.service_name.trim(),
|
||||
}
|
||||
updateNode(node.id, { services: [...(data.services ?? []), svc] })
|
||||
updateNode(node.id, { services: [...services, svc] })
|
||||
setNewSvc(EMPTY_FORM)
|
||||
setAddingForNode(null)
|
||||
}
|
||||
|
||||
const handleRemoveService = (index: number) => {
|
||||
const updated = (data.services ?? []).filter((_, i) => i !== index)
|
||||
const updated = services.filter((_, i) => i !== index)
|
||||
updateNode(node.id, { services: updated })
|
||||
if (editingIndex === index) setEditingFor(null)
|
||||
}
|
||||
|
||||
const handleStartEdit = (index: number) => {
|
||||
const svc = (data.services ?? [])[index]
|
||||
const svc = services[index]
|
||||
if (!svc) return
|
||||
setEditSvc({ port: String(svc.port), protocol: svc.protocol, service_name: svc.service_name })
|
||||
setEditingFor({ nodeId: node.id, index })
|
||||
@@ -69,7 +70,7 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
if (editingIndex === null) return
|
||||
const port = parseInt(editSvc.port, 10)
|
||||
if (!editSvc.service_name.trim() || isNaN(port) || port < 1 || port > 65535) return
|
||||
const updated = (data.services ?? []).map((svc, i) =>
|
||||
const updated = services.map((svc, i) =>
|
||||
i === editingIndex
|
||||
? { ...svc, port, protocol: editSvc.protocol, service_name: editSvc.service_name.trim() }
|
||||
: svc
|
||||
@@ -84,6 +85,7 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
<div className="flex items-center justify-between px-4 py-3 border-b border-border">
|
||||
<span className="font-semibold text-sm text-foreground truncate">{data.label}</span>
|
||||
<button
|
||||
aria-label="Close panel"
|
||||
onClick={() => setSelectedNode(null)}
|
||||
className="text-muted-foreground hover:text-foreground transition-colors"
|
||||
>
|
||||
@@ -142,7 +144,7 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
<div className="px-4 py-3 border-t border-border">
|
||||
<div className="flex items-center justify-between mb-2">
|
||||
<span className="text-xs text-muted-foreground">
|
||||
Services{(data.services ?? []).length > 0 ? ` (${data.services.length})` : ''}
|
||||
Services{services.length > 0 ? ` (${services.length})` : ''}
|
||||
</span>
|
||||
<button
|
||||
onClick={() => { setAddingForNode((v) => v === node.id ? null : node.id); setEditingFor(null) }}
|
||||
@@ -164,9 +166,9 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
/>
|
||||
)}
|
||||
|
||||
{(data.services ?? []).length > 0 && (
|
||||
{services.length > 0 && (
|
||||
<div className="flex flex-col gap-1.5">
|
||||
{(data.services ?? []).map((svc, i) =>
|
||||
{services.map((svc, i) =>
|
||||
editingIndex === i ? (
|
||||
<ServiceForm
|
||||
key={`edit-${i}`}
|
||||
@@ -190,7 +192,7 @@ export function DetailPanel({ onEdit }: DetailPanelProps) {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{(data.services ?? []).length === 0 && !addingService && (
|
||||
{services.length === 0 && !addingService && (
|
||||
<p className="text-[10px] text-muted-foreground/50">No services — click Add to register one.</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -116,6 +116,116 @@ describe('DetailPanel', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('Panel actions', () => {
|
||||
it('calls setSelectedNode(null) when close button is clicked', () => {
|
||||
const setSelectedNode = vi.fn()
|
||||
vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
|
||||
nodes: [makeNode({})],
|
||||
selectedNodeId: 'n1',
|
||||
setSelectedNode,
|
||||
deleteNode: vi.fn(),
|
||||
updateNode: vi.fn(),
|
||||
} as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
|
||||
render(<DetailPanel onEdit={vi.fn()} />)
|
||||
fireEvent.click(screen.getByLabelText('Close panel'))
|
||||
expect(setSelectedNode).toHaveBeenCalledWith(null)
|
||||
})
|
||||
|
||||
it('calls onEdit with node id when Edit button is clicked', () => {
|
||||
setupStore({})
|
||||
const onEdit = vi.fn()
|
||||
render(<DetailPanel onEdit={onEdit} />)
|
||||
fireEvent.click(screen.getByRole('button', { name: /edit/i }))
|
||||
expect(onEdit).toHaveBeenCalledWith('n1')
|
||||
})
|
||||
|
||||
it('calls deleteNode when delete confirmed', () => {
|
||||
const deleteNode = vi.fn()
|
||||
vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
|
||||
nodes: [makeNode({ label: 'My Server' })],
|
||||
selectedNodeId: 'n1',
|
||||
setSelectedNode: vi.fn(),
|
||||
deleteNode,
|
||||
updateNode: vi.fn(),
|
||||
} as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
|
||||
vi.spyOn(window, 'confirm').mockReturnValue(true)
|
||||
render(<DetailPanel onEdit={vi.fn()} />)
|
||||
fireEvent.click(screen.getByLabelText('Delete node'))
|
||||
expect(deleteNode).toHaveBeenCalledWith('n1')
|
||||
})
|
||||
|
||||
it('does not call deleteNode when delete is cancelled', () => {
|
||||
const deleteNode = vi.fn()
|
||||
vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
|
||||
nodes: [makeNode({})],
|
||||
selectedNodeId: 'n1',
|
||||
setSelectedNode: vi.fn(),
|
||||
deleteNode,
|
||||
updateNode: vi.fn(),
|
||||
} as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
|
||||
vi.spyOn(window, 'confirm').mockReturnValue(false)
|
||||
render(<DetailPanel onEdit={vi.fn()} />)
|
||||
fireEvent.click(screen.getByLabelText('Delete node'))
|
||||
expect(deleteNode).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Services — add/remove', () => {
|
||||
it('shows add form when Add is clicked', () => {
|
||||
setupStore({})
|
||||
render(<DetailPanel onEdit={vi.fn()} />)
|
||||
fireEvent.click(screen.getByText('Add'))
|
||||
expect(screen.getByPlaceholderText('Service name')).toBeDefined()
|
||||
})
|
||||
|
||||
it('calls updateNode with new service on Add confirm', () => {
|
||||
const updateNode = vi.fn()
|
||||
vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
|
||||
nodes: [makeNode({})],
|
||||
selectedNodeId: 'n1',
|
||||
setSelectedNode: vi.fn(),
|
||||
deleteNode: vi.fn(),
|
||||
updateNode,
|
||||
} as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
|
||||
render(<DetailPanel onEdit={vi.fn()} />)
|
||||
fireEvent.click(screen.getByText('Add'))
|
||||
fireEvent.change(screen.getByPlaceholderText('Service name'), { target: { value: 'nginx' } })
|
||||
fireEvent.change(screen.getByPlaceholderText('Port'), { target: { value: '80' } })
|
||||
// Two "Add" buttons exist: the header toggle and the form confirm — pick the form's
|
||||
const addButtons = screen.getAllByRole('button', { name: 'Add' })
|
||||
fireEvent.click(addButtons[addButtons.length - 1])
|
||||
expect(updateNode).toHaveBeenCalledOnce()
|
||||
expect(updateNode.mock.calls[0][1].services[0]).toMatchObject({ service_name: 'nginx', port: 80, protocol: 'tcp' })
|
||||
})
|
||||
|
||||
it('calls updateNode without the removed service when X is clicked', () => {
|
||||
const updateNode = vi.fn()
|
||||
const svc = { port: 80, protocol: 'tcp' as const, service_name: 'nginx' }
|
||||
vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
|
||||
nodes: [makeNode({ services: [svc] })],
|
||||
selectedNodeId: 'n1',
|
||||
setSelectedNode: vi.fn(),
|
||||
deleteNode: vi.fn(),
|
||||
updateNode,
|
||||
} as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
|
||||
render(<DetailPanel onEdit={vi.fn()} />)
|
||||
fireEvent.click(screen.getByTitle('Remove service'))
|
||||
expect(updateNode).toHaveBeenCalledOnce()
|
||||
expect(updateNode.mock.calls[0][1].services).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('does not crash when data.services is undefined', () => {
|
||||
vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
|
||||
nodes: [makeNode({ services: undefined as unknown as [] })],
|
||||
selectedNodeId: 'n1',
|
||||
setSelectedNode: vi.fn(),
|
||||
deleteNode: vi.fn(),
|
||||
updateNode: vi.fn(),
|
||||
} as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
|
||||
expect(() => render(<DetailPanel onEdit={vi.fn()} />)).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Services — edit', () => {
|
||||
const svc = { port: 80, protocol: 'tcp' as const, service_name: 'nginx' }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user