Dynamic dashboards: Refactor ungroup rows and tabs (#112575)

* refactor ungroup

* deleting last row/tab no longer ungroups

* Change test for deleting last row to not test for ungroup

* fix comment with correct pull request

* use isLayoutGroup instead

* fix implementations

* missing import
This commit is contained in:
Oscar Kilhed
2025-10-28 13:55:20 +01:00
committed by GitHub
parent 68bc0f8076
commit 7df95261f3
9 changed files with 84 additions and 81 deletions
@@ -16,6 +16,7 @@ import {
} from '../../utils/utils';
import { DashboardGridItem } from '../layout-default/DashboardGridItem';
import { clearClipboard, getAutoGridItemFromClipboard } from '../layouts-shared/paste';
import { DashboardLayoutGrid } from '../types/DashboardLayoutGrid';
import { DashboardLayoutManager } from '../types/DashboardLayoutManager';
import { LayoutRegistryItem } from '../types/LayoutRegistryItem';
@@ -38,10 +39,7 @@ export const AUTO_GRID_DEFAULT_MAX_COLUMN_COUNT = 3;
export const AUTO_GRID_DEFAULT_COLUMN_WIDTH = 'standard';
export const AUTO_GRID_DEFAULT_ROW_HEIGHT = 'standard';
export class AutoGridLayoutManager
extends SceneObjectBase<AutoGridLayoutManagerState>
implements DashboardLayoutManager
{
export class AutoGridLayoutManager extends SceneObjectBase<AutoGridLayoutManagerState> implements DashboardLayoutGrid {
public static Component = AutoGridLayoutManagerRenderer;
public readonly isDashboardLayoutManager = true;
@@ -200,12 +198,11 @@ export class AutoGridLayoutManager
});
}
public merge(other: DashboardLayoutManager) {
if (!(other instanceof AutoGridLayoutManager)) {
throw new Error('Cannot merge non-auto grid layout');
}
const sourceLayout = other.state.layout;
public mergeGrid(other: DashboardLayoutGrid) {
const sourceLayout =
other instanceof AutoGridLayoutManager
? other.state.layout
: AutoGridLayoutManager.createFromLayout(other).state.layout;
const movedChildren = [...sourceLayout.state.children];
// Remove from source and append to destination
@@ -48,6 +48,7 @@ import { CanvasGridAddActions } from '../layouts-shared/CanvasGridAddActions';
import { clearClipboard, getDashboardGridItemFromClipboard } from '../layouts-shared/paste';
import { dashboardCanvasAddButtonHoverStyles } from '../layouts-shared/styles';
import { getIsLazy } from '../layouts-shared/utils';
import { DashboardLayoutGrid } from '../types/DashboardLayoutGrid';
import { DashboardLayoutManager } from '../types/DashboardLayoutManager';
import { LayoutRegistryItem } from '../types/LayoutRegistryItem';
@@ -62,7 +63,7 @@ interface DefaultGridLayoutManagerState extends SceneObjectState {
export class DefaultGridLayoutManager
extends SceneObjectBase<DefaultGridLayoutManagerState>
implements DashboardLayoutManager
implements DashboardLayoutGrid
{
public static Component = DefaultGridLayoutManagerRenderer;
@@ -93,11 +94,7 @@ export class DefaultGridLayoutManager
this.addActivationHandler(() => this._activationHandler());
}
public merge(other: DashboardLayoutManager) {
if (!(other instanceof DefaultGridLayoutManager)) {
throw new Error('Cannot merge non-default grid layout');
}
public mergeGrid(other: DashboardLayoutGrid) {
let offset = 0;
for (const child of this.state.grid.state.children) {
const newOffset = (child.state.y ?? 0) + (child.state.height ?? 0);
@@ -106,7 +103,10 @@ export class DefaultGridLayoutManager
}
}
const sourceGrid = other.state.grid;
const sourceGrid =
other instanceof DefaultGridLayoutManager
? other.state.grid
: DefaultGridLayoutManager.createFromLayout(other).state.grid;
const movedChildren = [...sourceGrid.state.children];
for (const child of movedChildren) {
@@ -117,12 +117,14 @@ describe('RowsLayoutManager', () => {
expect(manager.state.rows).toContain(row2);
});
it('should call ungroupLayout when removing the last row', () => {
it('should not 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);
// This behavior was changed in the PR https://github.com/grafana/grafana/pull/112575
// The delete row button should have one consistent behavior, no matter if it's the last row or not.
expect(ungroupLayoutCalled).toBe(false);
});
});
});
@@ -22,6 +22,8 @@ import { findAllGridTypes } from '../layouts-shared/findAllGridTypes';
import { getRowFromClipboard } from '../layouts-shared/paste';
import { showConvertMixedGridsModal, showUngroupConfirmation } from '../layouts-shared/ungroupConfirmation';
import { generateUniqueTitle, ungroupLayout, GridLayoutType, mapIdToGridLayoutType } from '../layouts-shared/utils';
import { isDashboardLayoutGrid } from '../types/DashboardLayoutGrid';
import { DashboardLayoutGroup, isDashboardLayoutGroup } from '../types/DashboardLayoutGroup';
import { DashboardLayoutManager } from '../types/DashboardLayoutManager';
import { isLayoutParent } from '../types/LayoutParent';
import { LayoutRegistryItem } from '../types/LayoutRegistryItem';
@@ -33,7 +35,7 @@ interface RowsLayoutManagerState extends SceneObjectState {
rows: RowItem[];
}
export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> implements DashboardLayoutManager {
export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> implements DashboardLayoutGroup {
public static Component = RowLayoutManagerRenderer;
public readonly isDashboardLayoutManager = true;
@@ -134,7 +136,7 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
return outlineChildren;
}
public convertAllRowsLayouts(gridLayoutType: GridLayoutType) {
public convertAllGridLayouts(gridLayoutType: GridLayoutType) {
for (const row of this.state.rows) {
switch (gridLayoutType) {
case GridLayoutType.AutoGridLayout:
@@ -189,7 +191,7 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
description: t('dashboard.rows-layout.edit.ungroup-rows', 'Ungroup rows'),
source: scene,
perform: () => {
this._ungroupRows(gridLayoutType);
this.ungroup(gridLayoutType);
},
undo: () => {
parent.switchLayout(previousLayout);
@@ -197,15 +199,15 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
});
}
private _ungroupRows(gridLayoutType: GridLayoutType) {
public ungroup(gridLayoutType: GridLayoutType) {
const hasNonGridLayout = this.state.rows.some((row) => !row.getLayout().descriptor.isGridLayout);
if (hasNonGridLayout) {
for (const row of this.state.rows) {
const layout = row.getLayout();
if (!layout.descriptor.isGridLayout) {
if (layout instanceof RowsLayoutManager) {
layout._ungroupRows(gridLayoutType);
if (isDashboardLayoutGroup(layout)) {
layout.ungroup(gridLayoutType);
} else {
throw new Error(`Ungrouping not supported for layout type: ${layout.descriptor.name}`);
}
@@ -213,7 +215,7 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
}
}
this.convertAllRowsLayouts(gridLayoutType);
this.convertAllGridLayouts(gridLayoutType);
const firstRow = this.state.rows[0];
const firstRowLayout = firstRow.getLayout();
@@ -221,8 +223,8 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
for (const row of otherRows) {
const layout = row.getLayout();
if (firstRowLayout.merge) {
firstRowLayout.merge(layout);
if (isDashboardLayoutGrid(firstRowLayout) && isDashboardLayoutGrid(layout)) {
firstRowLayout.mergeGrid(layout);
} else {
throw new Error(`Layout type ${firstRowLayout.descriptor.name} does not support merging`);
}
@@ -230,15 +232,10 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i
this.setState({ rows: [firstRow] });
this.removeRow(firstRow, true);
ungroupLayout(this, firstRow.state.layout, true);
}
public removeRow(row: RowItem, skipUndo?: boolean) {
// When removing last row replace ourselves with the inner row layout
if (this.shouldUngroup()) {
ungroupLayout(this, row.state.layout, skipUndo ?? false);
return;
}
const indexOfRowToRemove = this.state.rows.findIndex((r) => r === row);
const perform = () => this.setState({ rows: this.state.rows.filter((r) => r !== row) });
@@ -53,15 +53,6 @@ export function RowLayoutManagerRenderer({ model }: SceneComponentProps<RowsLayo
{dropProvided.placeholder}
{isEditing && !isClone && (
<div className="dashboard-canvas-add-button">
<Button
icon="layers-slash"
variant="primary"
fill="text"
onClick={() => model.ungroupRows()}
data-testid={selectors.components.CanvasGridAddActions.ungroupRows}
>
<Trans i18nKey="dashboard.canvas-actions.ungroup-rows">Ungroup rows</Trans>
</Button>
<Button
icon="plus"
variant="primary"
@@ -84,6 +75,15 @@ export function RowLayoutManagerRenderer({ model }: SceneComponentProps<RowsLayo
<Trans i18nKey="dashboard.canvas-actions.paste-row">Paste row</Trans>
</Button>
)}
<Button
icon="layers-slash"
variant="primary"
fill="text"
onClick={() => model.ungroupRows()}
data-testid={selectors.components.CanvasGridAddActions.ungroupRows}
>
<Trans i18nKey="dashboard.canvas-actions.ungroup-rows">Ungroup rows</Trans>
</Button>
</div>
)}
</div>
@@ -21,6 +21,8 @@ import { findAllGridTypes } from '../layouts-shared/findAllGridTypes';
import { getTabFromClipboard } from '../layouts-shared/paste';
import { showConvertMixedGridsModal, showUngroupConfirmation } from '../layouts-shared/ungroupConfirmation';
import { generateUniqueTitle, ungroupLayout, GridLayoutType, mapIdToGridLayoutType } from '../layouts-shared/utils';
import { isDashboardLayoutGrid } from '../types/DashboardLayoutGrid';
import { DashboardLayoutGroup, isDashboardLayoutGroup } from '../types/DashboardLayoutGroup';
import { DashboardLayoutManager } from '../types/DashboardLayoutManager';
import { isLayoutParent } from '../types/LayoutParent';
import { LayoutRegistryItem } from '../types/LayoutRegistryItem';
@@ -33,7 +35,7 @@ interface TabsLayoutManagerState extends SceneObjectState {
currentTabSlug?: string;
}
export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> implements DashboardLayoutManager {
export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> implements DashboardLayoutGroup {
public static Component = TabsLayoutManagerRenderer;
public readonly isDashboardLayoutManager = true;
@@ -71,21 +73,6 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
throw new Error('Method not implemented.');
}
public merge(other: DashboardLayoutManager) {
if (!(other instanceof TabsLayoutManager)) {
throw new Error('Cannot merge non-tabs layout');
}
// Merge all tabs from the other layout into this one
const otherTabs = other.state.tabs;
const mergedTabs = [...this.state.tabs, ...otherTabs];
// Clear parent from merged tabs to avoid conflicts
otherTabs.forEach((tab) => tab.clearParent());
this.setState({ tabs: mergedTabs });
}
public duplicateTab(tab: TabItem) {
const newTab = tab.duplicate();
this.addNewTab(newTab);
@@ -223,7 +210,7 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
return this.state.tabs.length === 1;
}
public convertAllTabsLayouts(gridLayoutType: GridLayoutType) {
public convertAllGridLayouts(gridLayoutType: GridLayoutType) {
for (const tab of this.state.tabs) {
switch (gridLayoutType) {
case GridLayoutType.AutoGridLayout:
@@ -278,7 +265,7 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
description: t('dashboard.tabs-layout.edit.ungroup-tabs', 'Ungroup tabs'),
source: scene,
perform: () => {
this._ungroupTabs(gridLayoutType);
this.ungroup(gridLayoutType);
},
undo: () => {
parent.switchLayout(previousLayout);
@@ -286,17 +273,15 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
});
}
private _ungroupTabs(gridLayoutType: GridLayoutType) {
public ungroup(gridLayoutType: GridLayoutType) {
const hasNonGridLayout = this.state.tabs.some((tab) => !tab.getLayout().descriptor.isGridLayout);
if (hasNonGridLayout) {
for (const tab of this.state.tabs) {
const layout = tab.getLayout();
if (!layout.descriptor.isGridLayout) {
if (layout instanceof TabsLayoutManager) {
layout._ungroupTabs(gridLayoutType);
} else if (layout instanceof RowsLayoutManager) {
layout.ungroupRows();
if (isDashboardLayoutGroup(layout)) {
layout.ungroup(gridLayoutType);
} else {
throw new Error(`Ungrouping not supported for layout type: ${layout.descriptor.name}`);
}
@@ -304,7 +289,7 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
}
}
this.convertAllTabsLayouts(gridLayoutType);
this.convertAllGridLayouts(gridLayoutType);
const firstTab = this.state.tabs[0];
const firstTabLayout = firstTab.getLayout();
@@ -312,24 +297,18 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i
for (const tab of otherTabs) {
const layout = tab.getLayout();
if (firstTabLayout.merge) {
firstTabLayout.merge(layout);
if (isDashboardLayoutGrid(firstTabLayout) && isDashboardLayoutGrid(layout)) {
firstTabLayout.mergeGrid(layout);
} else {
throw new Error(`Layout type ${firstTabLayout.descriptor.name} does not support merging`);
}
}
this.setState({ tabs: [firstTab] });
this.removeTab(firstTab, true);
ungroupLayout(this, firstTab.state.layout, true);
}
public removeTab(tabToRemove: TabItem, skipUndo?: boolean) {
// When removing last tab replace ourselves with the inner tab layout
if (this.shouldUngroup()) {
ungroupLayout(this, tabToRemove.state.layout, skipUndo ?? false);
return;
}
const tabIndex = this.state.tabs.findIndex((t) => t === tabToRemove);
const perform = () => {
@@ -0,0 +1,12 @@
import { DashboardLayoutManager } from './DashboardLayoutManager';
export interface DashboardLayoutGrid extends DashboardLayoutManager {
/**
* Merge the layout with another layout
*/
mergeGrid(other: DashboardLayoutGrid): void;
}
export function isDashboardLayoutGrid(obj: DashboardLayoutManager): obj is DashboardLayoutGrid {
return 'mergeGrid' in obj;
}
@@ -0,0 +1,21 @@
import { GridLayoutType } from '../layouts-shared/utils';
import { DashboardLayoutManager } from './DashboardLayoutManager';
export interface DashboardLayoutGroup extends DashboardLayoutManager {
/**
* Ungroup the group
* @param gridLayoutType
*/
ungroup(gridLayoutType: GridLayoutType): void;
/**
* Convert all layouts to the given grid layout type
* @param gridLayoutType
*/
convertAllGridLayouts(gridLayoutType: GridLayoutType): void;
}
export function isDashboardLayoutGroup(obj: DashboardLayoutManager): obj is DashboardLayoutGroup {
return 'ungroup' in obj && 'convertAllGridLayouts' in obj;
}
@@ -86,11 +86,6 @@ export interface DashboardLayoutManager<S = {}> extends SceneObject {
* Get children for outline
*/
getOutlineChildren(): SceneObject[];
/**
* Merge the layout with another layout
*/
merge?(other: DashboardLayoutManager): void;
}
export interface LayoutManagerSerializer {