diff --git a/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.test.tsx b/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.test.tsx new file mode 100644 index 00000000000..dbadd5def5e --- /dev/null +++ b/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.test.tsx @@ -0,0 +1,128 @@ +import { dashboardEditActions } from '../../edit-pane/shared'; + +import { RowItem } from './RowItem'; +import { RowsLayoutManager } from './RowsLayoutManager'; + +let lastUndo: (() => void) | undefined; +let ungroupLayoutCalled = false; + +jest.mock('../../edit-pane/shared', () => ({ + dashboardEditActions: { + addElement: jest.fn(({ perform, undo }) => { + perform(); + lastUndo = undo; + }), + removeElement: jest.fn(({ perform, undo }) => { + perform(); + lastUndo = undo; + }), + }, +})); + +jest.mock('../layouts-shared/utils', () => ({ + ...jest.requireActual('../layouts-shared/utils'), + ungroupLayout: jest.fn(() => { + ungroupLayoutCalled = true; + }), +})); + +describe('RowsLayoutManager', () => { + describe('addNewRow', () => { + beforeEach(() => { + lastUndo = undefined; + }); + + it('should add a new row with default title when no title is provided', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const newRow = manager.addNewRow(); + + expect(newRow).toBeInstanceOf(RowItem); + expect(newRow.state.title).toBe('New row'); + expect(manager.state.rows).toHaveLength(1); + expect(manager.state.rows[0]).toBe(newRow); + }); + + it('should add a row with the provided title if it is unique', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const newRow = manager.addNewRow(new RowItem({ title: 'Unique Title' })); + + expect(newRow.state.title).toBe('Unique Title'); + expect(manager.state.rows).toHaveLength(1); + expect(manager.state.rows[0]).toBe(newRow); + }); + + it('should generate a unique title when adding a row with a duplicate title', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const firstRow = manager.addNewRow(new RowItem({ title: 'Test Title' })); + const secondRow = manager.addNewRow(new RowItem({ title: 'Test Title' })); + + expect(firstRow.state.title).toBe('Test Title'); + expect(secondRow.state.title).toBe('Test Title 1'); + expect(manager.state.rows).toHaveLength(2); + }); + + it('should increment the number in the title for multiple duplicates', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const firstRow = manager.addNewRow(new RowItem({ title: 'Test Title' })); + const secondRow = manager.addNewRow(new RowItem({ title: 'Test Title' })); + const thirdRow = manager.addNewRow(new RowItem({ title: 'Test Title' })); + + expect(firstRow.state.title).toBe('Test Title'); + expect(secondRow.state.title).toBe('Test Title 1'); + expect(thirdRow.state.title).toBe('Test Title 2'); + expect(manager.state.rows).toHaveLength(3); + }); + + it('should handle undo action correctly', () => { + const manager = new RowsLayoutManager({ rows: [] }); + manager.addNewRow(new RowItem({ title: 'Test Title' })); + + expect(manager.state.rows).toHaveLength(1); + + // Use the real undo function from the mock + expect(typeof lastUndo).toBe('function'); + lastUndo && lastUndo(); + + expect(manager.state.rows).toHaveLength(0); + }); + }); + + describe('removeRow', () => { + beforeEach(() => { + lastUndo = undefined; + ungroupLayoutCalled = false; + jest.clearAllMocks(); + }); + + it('should remove a row and call removeElement', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const row1 = manager.addNewRow(new RowItem({ title: 'Row 1' })); + const row2 = manager.addNewRow(new RowItem({ title: 'Row 2' })); + expect(manager.state.rows).toHaveLength(2); + manager.removeRow(row1); + expect(manager.state.rows).toHaveLength(1); + expect(manager.state.rows[0]).toBe(row2); + expect(dashboardEditActions.removeElement).toHaveBeenCalled(); + }); + + it('should handle undo action correctly', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const row1 = manager.addNewRow(new RowItem({ title: 'Row 1' })); + const row2 = manager.addNewRow(new RowItem({ title: 'Row 2' })); + manager.removeRow(row1); + expect(manager.state.rows).toHaveLength(1); + lastUndo && lastUndo(); + expect(manager.state.rows).toHaveLength(2); + expect(manager.state.rows).toContain(row1); + expect(manager.state.rows).toContain(row2); + }); + + it('should call ungroupLayout when removing the last row', () => { + const manager = new RowsLayoutManager({ rows: [] }); + const row = manager.addNewRow(new RowItem({ title: 'Only Row' })); + expect(manager.state.rows).toHaveLength(1); + manager.removeRow(row); + expect(ungroupLayoutCalled).toBe(true); + }); + }); +}); diff --git a/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.tsx b/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.tsx index d468e922ac9..474109e1a5e 100644 --- a/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.tsx +++ b/public/app/features/dashboard-scene/scene/layout-rows/RowsLayoutManager.tsx @@ -9,11 +9,7 @@ import { } from '@grafana/scenes'; import { Spec as DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha1/types.spec.gen'; -import { - dashboardEditActions, - ObjectRemovedFromCanvasEvent, - ObjectsReorderedOnCanvasEvent, -} from '../../edit-pane/shared'; +import { dashboardEditActions, ObjectsReorderedOnCanvasEvent } from '../../edit-pane/shared'; import { serializeRowsLayout } from '../../serialization/layoutSerializers/RowsLayoutSerializer'; import { isClonedKey, joinCloneKeys } from '../../utils/clone'; import { getDashboardSceneFor } from '../../utils/utils'; @@ -151,9 +147,18 @@ export class RowsLayoutManager extends SceneObjectBase i return; } - const rows = this.state.rows.filter((r) => r !== row); - this.setState({ rows }); - this.publishEvent(new ObjectRemovedFromCanvasEvent(row), true); + const indexOfRowToRemove = this.state.rows.findIndex((r) => r === row); + + dashboardEditActions.removeElement({ + removedObject: row, + source: this, + perform: () => this.setState({ rows: this.state.rows.filter((r) => r !== row) }), + undo: () => { + const rows = [...this.state.rows]; + rows.splice(indexOfRowToRemove, 0, row); + this.setState({ rows }); + }, + }); } public moveRow(_rowKey: string, fromIndex: number, toIndex: number) { diff --git a/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.test.tsx b/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.test.tsx index 079207b03cb..8c4b2fb97f3 100644 --- a/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.test.tsx +++ b/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.test.tsx @@ -1,9 +1,25 @@ +import { dashboardEditActions } from '../../edit-pane/shared'; import { RowItem } from '../layout-rows/RowItem'; import { RowsLayoutManager } from '../layout-rows/RowsLayoutManager'; import { TabItem } from './TabItem'; import { TabsLayoutManager } from './TabsLayoutManager'; +let lastUndo: (() => void) | undefined; + +jest.mock('../../edit-pane/shared', () => ({ + dashboardEditActions: { + addElement: jest.fn(({ perform, undo }) => { + perform(); + lastUndo = undo; + }), + removeElement: jest.fn(({ perform, undo }) => { + perform(); + lastUndo = undo; + }), + }, +})); + describe('TabsLayoutManager', () => { describe('url sync', () => { it('when on top level', () => { @@ -42,4 +58,125 @@ describe('TabsLayoutManager', () => { }); }); }); + + describe('addNewTab', () => { + beforeEach(() => { + lastUndo = undefined; + }); + + it('should add a new tab with default title when no title is provided', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const newTab = manager.addNewTab(); + + expect(newTab).toBeInstanceOf(TabItem); + expect(newTab.state.title).toBe('New tab'); + expect(manager.state.tabs).toHaveLength(1); + expect(manager.state.tabs[0]).toBe(newTab); + }); + + it('should add a tab with the provided title if it is unique', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const newTab = manager.addNewTab(new TabItem({ title: 'Unique Title' })); + + expect(newTab.state.title).toBe('Unique Title'); + expect(manager.state.tabs).toHaveLength(1); + expect(manager.state.tabs[0]).toBe(newTab); + }); + + it('should generate a unique title when adding a tab with a duplicate title', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const firstTab = manager.addNewTab(new TabItem({ title: 'Test Title' })); + const secondTab = manager.addNewTab(new TabItem({ title: 'Test Title' })); + + expect(firstTab.state.title).toBe('Test Title'); + expect(secondTab.state.title).toBe('Test Title 1'); + expect(manager.state.tabs).toHaveLength(2); + }); + + it('should increment the number in the title for multiple duplicates', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const firstTab = manager.addNewTab(new TabItem({ title: 'Test Title' })); + const secondTab = manager.addNewTab(new TabItem({ title: 'Test Title' })); + const thirdTab = manager.addNewTab(new TabItem({ title: 'Test Title' })); + + expect(firstTab.state.title).toBe('Test Title'); + expect(secondTab.state.title).toBe('Test Title 1'); + expect(thirdTab.state.title).toBe('Test Title 2'); + expect(manager.state.tabs).toHaveLength(3); + }); + + it('should handle undo action correctly', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + manager.addNewTab(new TabItem({ title: 'Test Title' })); + + expect(manager.state.tabs).toHaveLength(1); + + // Use the real undo function from the mock + expect(typeof lastUndo).toBe('function'); + lastUndo && lastUndo(); + + expect(manager.state.tabs).toHaveLength(0); + }); + }); + + describe('removeTab', () => { + beforeEach(() => { + lastUndo = undefined; + }); + + it('should remove a non-current tab without using removeElement', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const tab1 = manager.addNewTab(new TabItem({ title: 'Tab 1' })); + const tab2 = manager.addNewTab(new TabItem({ title: 'Tab 2' })); + + expect(manager.state.tabs).toHaveLength(2); + expect(manager.state.currentTabIndex).toBe(1); // tab2 is current + + manager.removeTab(tab1); + + expect(manager.state.tabs).toHaveLength(1); + expect(manager.state.tabs[0]).toBe(tab2); + expect(manager.state.currentTabIndex).toBe(0); + expect(dashboardEditActions.removeElement).not.toHaveBeenCalled(); + }); + + it('should remove the current tab using removeElement', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const tab1 = manager.addNewTab(new TabItem({ title: 'Tab 1' })); + const tab2 = manager.addNewTab(new TabItem({ title: 'Tab 2' })); + + expect(manager.state.tabs).toHaveLength(2); + expect(manager.state.currentTabIndex).toBe(1); // tab2 is current + + manager.removeTab(tab2); + + expect(manager.state.tabs).toHaveLength(1); + expect(manager.state.tabs[0]).toBe(tab1); + expect(manager.state.currentTabIndex).toBe(0); + expect(dashboardEditActions.removeElement).toHaveBeenCalled(); + }); + + it('should handle undo action correctly when removing the current tab', () => { + const manager = new TabsLayoutManager({ tabs: [] }); + const tab1 = manager.addNewTab(new TabItem({ title: 'Tab 1' })); + const tab2 = manager.addNewTab(new TabItem({ title: 'Tab 2' })); + + expect(manager.state.tabs).toHaveLength(2); + expect(manager.state.currentTabIndex).toBe(1); // tab2 is current + + manager.removeTab(tab2); + + expect(manager.state.tabs).toHaveLength(1); + expect(manager.state.tabs[0]).toBe(tab1); + + // Use the real undo function from the mock + expect(typeof lastUndo).toBe('function'); + lastUndo && lastUndo(); + + expect(manager.state.tabs).toHaveLength(2); + expect(manager.state.tabs).toContain(tab1); + expect(manager.state.tabs).toContain(tab2); + expect(manager.state.currentTabIndex).toBe(1); // tab2 should be current again + }); + }); }); diff --git a/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.tsx b/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.tsx index de4cb8157df..913b8469931 100644 --- a/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.tsx +++ b/public/app/features/dashboard-scene/scene/layout-tabs/TabsLayoutManager.tsx @@ -9,11 +9,7 @@ import { } from '@grafana/scenes'; import { Spec as DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha1/types.spec.gen'; -import { - NewObjectAddedToCanvasEvent, - ObjectRemovedFromCanvasEvent, - ObjectsReorderedOnCanvasEvent, -} from '../../edit-pane/shared'; +import { dashboardEditActions, ObjectsReorderedOnCanvasEvent } from '../../edit-pane/shared'; import { serializeTabsLayout } from '../../serialization/layoutSerializers/TabsLayoutSerializer'; import { isClonedKey, joinCloneKeys } from '../../utils/clone'; import { getDashboardSceneFor } from '../../utils/utils'; @@ -146,8 +142,23 @@ export class TabsLayoutManager extends SceneObjectBase i newTab.setState({ title: newTitle }); } - this.setState({ tabs: [...this.state.tabs, newTab], currentTabIndex: this.state.tabs.length }); - this.publishEvent(new NewObjectAddedToCanvasEvent(newTab), true); + dashboardEditActions.addElement({ + addedObject: newTab, + source: this, + perform: () => this.setState({ tabs: [...this.state.tabs, newTab], currentTabIndex: this.state.tabs.length }), + undo: () => { + const indexOfNewTab = this.state.tabs.findIndex((t) => t === newTab); + this.setState({ + tabs: this.state.tabs.filter((t) => t !== newTab), + // if the new tab was the current tab, set the current tab to the previous tab + currentTabIndex: + this.state.currentTabIndex === indexOfNewTab + ? Math.max(0, this.state.currentTabIndex - 1) + : this.state.currentTabIndex, + }); + }, + }); + return newTab; } @@ -191,9 +202,25 @@ export class TabsLayoutManager extends SceneObjectBase i const currentTab = this.getCurrentTab(); if (currentTab === tabToRemove) { - const nextTabIndex = this.state.currentTabIndex > 0 ? this.state.currentTabIndex - 1 : 0; - this.setState({ tabs: this.state.tabs.filter((t) => t !== tabToRemove), currentTabIndex: nextTabIndex }); - this.publishEvent(new ObjectRemovedFromCanvasEvent(tabToRemove), true); + const currentTabIndex = this.state.currentTabIndex; + const indexOfTabToRemove = this.state.tabs.findIndex((t) => t === tabToRemove); + const nextTabIndex = currentTabIndex > 0 ? currentTabIndex - 1 : 0; + + dashboardEditActions.removeElement({ + removedObject: tabToRemove, + source: this, + perform: () => + this.setState({ + tabs: this.state.tabs.filter((t) => t !== tabToRemove), + currentTabIndex: currentTabIndex === indexOfTabToRemove ? nextTabIndex : currentTabIndex, + }), + undo: () => { + const tabs = [...this.state.tabs]; + tabs.splice(indexOfTabToRemove, 0, tabToRemove); + this.setState({ tabs, currentTabIndex }); + }, + }); + return; } @@ -201,7 +228,6 @@ export class TabsLayoutManager extends SceneObjectBase i const tabs = filteredTab.length === 0 ? [new TabItem()] : filteredTab; this.setState({ tabs, currentTabIndex: 0 }); - this.publishEvent(new ObjectRemovedFromCanvasEvent(tabToRemove), true); } public moveTab(_tabKey: string, fromIndex: number, toIndex: number) {