From 7dd636917bd7a5c66ea474fb93110fc9e77223eb Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 27 Nov 2024 14:12:15 -0700 Subject: [PATCH] [Dashboard][Collapsable Panels] Fix bug on `layout` update (#202049) ## Summary When the `layout` prop updates, we cannot assume that it is "safe" (i.e. we cannot assume it has no floating panels and/or colliding panels). Because of this, we need to resolve each grid row when this prop changes. When I added this in https://github.com/elastic/kibana/pull/200793, I accidentally only **compacted** the rows, which did not account for possible collisions that the Dashboard's panel placement strategies do not account for. This PR fixes that by calling `resolveGridRow` (which compacts **and** detects collisions) instead of just `compactGridRow` (which, as the name suggests, only compacts the rows) **Before:** https://github.com/user-attachments/assets/bcea4efd-35fa-425d-ac04-41434ffaa810 **After:** https://github.com/user-attachments/assets/6cd205c6-d1d5-4a97-8d14-425c2a4bbeda ### Checklist - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks There are no risks to this PR, since all work is contained in the `examples` plugin. (cherry picked from commit 57b8bdad3fb64d26b8fe3b0bf12c071857fff9e2) --- packages/kbn-grid-layout/grid/grid_layout.tsx | 4 +- .../grid/utils/resolve_grid_row.ts | 2 +- .../grid/utils/run_panel_placement.ts | 116 ------------------ 3 files changed, 3 insertions(+), 119 deletions(-) delete mode 100644 packages/kbn-grid-layout/grid/utils/run_panel_placement.ts diff --git a/packages/kbn-grid-layout/grid/grid_layout.tsx b/packages/kbn-grid-layout/grid/grid_layout.tsx index fc67c5b134606..68c4a38e8d45d 100644 --- a/packages/kbn-grid-layout/grid/grid_layout.tsx +++ b/packages/kbn-grid-layout/grid/grid_layout.tsx @@ -17,7 +17,7 @@ import { GridLayoutData, GridSettings } from './types'; import { useGridLayoutEvents } from './use_grid_layout_events'; import { useGridLayoutState } from './use_grid_layout_state'; import { isLayoutEqual } from './utils/equality_checks'; -import { compactGridRow } from './utils/resolve_grid_row'; +import { resolveGridRow } from './utils/resolve_grid_row'; interface GridLayoutProps { layout: GridLayoutData; @@ -53,7 +53,7 @@ export const GridLayout = ({ * so, we need to loop through each row and ensure it is compacted */ newLayout.forEach((row, rowIndex) => { - newLayout[rowIndex] = compactGridRow(row); + newLayout[rowIndex] = resolveGridRow(row); }); gridLayoutStateManager.gridLayout$.next(newLayout); } diff --git a/packages/kbn-grid-layout/grid/utils/resolve_grid_row.ts b/packages/kbn-grid-layout/grid/utils/resolve_grid_row.ts index 3037a52c27c69..29374a22356c7 100644 --- a/packages/kbn-grid-layout/grid/utils/resolve_grid_row.ts +++ b/packages/kbn-grid-layout/grid/utils/resolve_grid_row.ts @@ -57,7 +57,7 @@ const getKeysInOrder = (rowData: GridRowData, draggedId?: string): string[] => { }); }; -export const compactGridRow = (originalLayout: GridRowData) => { +const compactGridRow = (originalLayout: GridRowData) => { const nextRowData = { ...originalLayout, panels: { ...originalLayout.panels } }; // compact all vertical space. const sortedKeysAfterMove = getKeysInOrder(nextRowData); diff --git a/packages/kbn-grid-layout/grid/utils/run_panel_placement.ts b/packages/kbn-grid-layout/grid/utils/run_panel_placement.ts deleted file mode 100644 index 69ecddd1f5ffb..0000000000000 --- a/packages/kbn-grid-layout/grid/utils/run_panel_placement.ts +++ /dev/null @@ -1,116 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ -import { i18n } from '@kbn/i18n'; -import { GridRowData } from '../..'; -import { GridPanelData, PanelPlacementStrategy } from '../types'; -import { compactGridRow, resolveGridRow } from './resolve_grid_row'; - -export const runPanelPlacementStrategy = ( - originalRowData: GridRowData, - newPanel: Omit, - columnCount: number, - strategy: PanelPlacementStrategy = PanelPlacementStrategy.findTopLeftMostOpenSpace -): GridRowData => { - const nextRowData = { ...originalRowData, panels: { ...originalRowData.panels } }; // prevent mutation of original row object - switch (strategy) { - case PanelPlacementStrategy.placeAtTop: - // move all other panels down by the height of the new panel to make room for the new panel - Object.keys(nextRowData.panels).forEach((key) => { - const panel = nextRowData.panels[key]; - panel.row += newPanel.height; - }); - - // some panels might need to be pushed back up because they are now floating - so, compact the row - return compactGridRow({ - ...nextRowData, - // place the new panel at the top left corner, since there is now space - panels: { ...nextRowData.panels, [newPanel.id]: { ...newPanel, row: 0, column: 0 } }, - }); - - case PanelPlacementStrategy.findTopLeftMostOpenSpace: - // find the max row - let maxRow = -1; - const currentPanelsArray = Object.values(nextRowData.panels); - currentPanelsArray.forEach((panel) => { - maxRow = Math.max(panel.row + panel.height, maxRow); - }); - - // handle case of empty grid by placing the panel at the top left corner - if (maxRow < 0) { - return { - ...nextRowData, - panels: { [newPanel.id]: { ...newPanel, row: 0, column: 0 } }, - }; - } - - // find a spot in the grid where the entire panel will fit - const { row, column } = (() => { - // create a 2D array representation of the grid filled with zeros - const grid = new Array(maxRow); - for (let y = 0; y < maxRow; y++) { - grid[y] = new Array(columnCount).fill(0); - } - - // fill in the 2D array with ones wherever a panel is - currentPanelsArray.forEach((panel) => { - for (let x = panel.column; x < panel.column + panel.width; x++) { - for (let y = panel.row; y < panel.row + panel.height; y++) { - grid[y][x] = 1; - } - } - }); - - // now find the first empty spot where there are enough zeros (unoccupied spaces) to fit the whole panel - for (let y = 0; y < maxRow; y++) { - for (let x = 0; x < columnCount; x++) { - if (grid[y][x] === 1) { - // space is filled, so skip this spot - continue; - } else { - for (let h = y; h < Math.min(y + newPanel.height, maxRow); h++) { - for (let w = x; w < Math.min(x + newPanel.width, columnCount); w++) { - const spaceIsEmpty = grid[h][w] === 0; - const fitsPanelWidth = w === x + newPanel.width - 1; - // if the panel is taller than any other panel in the current grid, it can still fit in the space, hence - // we check the minimum of maxY and the panel height. - const fitsPanelHeight = h === Math.min(y + newPanel.height - 1, maxRow - 1); - - if (spaceIsEmpty && fitsPanelWidth && fitsPanelHeight) { - // found an empty space where the entire panel will fit - return { column: x, row: y }; - } else if (grid[h][w] === 1) { - // x, y is already occupied - break out of the loop and move on to the next starting point - break; - } - } - } - } - } - } - - return { column: 0, row: maxRow }; - })(); - - // some panels might need to be pushed down to accomodate the height of the new panel; - // so, resolve the entire row to remove any potential collisions - return resolveGridRow({ - ...nextRowData, - // place the new panel at the top left corner, since there is now space - panels: { ...nextRowData.panels, [newPanel.id]: { ...newPanel, row, column } }, - }); - - default: - throw new Error( - i18n.translate('kbnGridLayout.panelPlacement.unknownStrategyError', { - defaultMessage: 'Unknown panel placement strategy: {strategy}', - values: { strategy }, - }) - ); - } -};