Dashboard: Fix repeated row panel placement with larger number of rows (#72011)

Closes #69833

.betterer.results

@@ -2333,17 +2333,16 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "23"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "24"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "25"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "26"],
+      [0, 0, 0, "Do not use any type assertions.", "26"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "27"],
-      [0, 0, 0, "Do not use any type assertions.", "28"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "28"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "29"],
+      [0, 0, 0, "Do not use any type assertions.", "29"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "30"],
-      [0, 0, 0, "Do not use any type assertions.", "31"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "31"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "32"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "33"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "34"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "35"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "35"]
-      [0, 0, 0, "Unexpected any. Specify a different type.", "36"]
     ],
     "public/app/features/dashboard/state/PanelModel.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

public/app/features/dashboard/state/DashboardModel.ts

@@ -14,6 +14,7 @@ import {
   PanelModel as IPanelModel,
   TimeRange,
   TimeZone,
+  TypedVariableModel,
   UrlQueryValue,
 } from '@grafana/data';
 import { RefreshEvent, TimeRangeUpdatedEvent, config } from '@grafana/runtime';
@@ -42,7 +43,7 @@ import { getTimeSrv } from '../services/TimeSrv';
 import { mergePanels, PanelMergeInfo } from '../utils/panelMerge';
 
 import { DashboardMigrator } from './DashboardMigrator';
-import { GridPos, PanelModel, autoMigrateAngular } from './PanelModel';
+import { PanelModel, autoMigrateAngular } from './PanelModel';
 import { TimeModel } from './TimeModel';
 import { deleteScopeVars, isOnTheSameGridRow } from './utils';
 
@@ -667,12 +668,13 @@ export class DashboardModel implements TimeModel {
   }
 
   getRowRepeatClone(sourceRowPanel: PanelModel, valueIndex: number, sourcePanelIndex: number) {
-    // if first clone return source
+    // if first clone, return source
     if (valueIndex === 0) {
       if (!sourceRowPanel.collapsed) {
         const rowPanels = this.getRowPanels(sourcePanelIndex);
         sourceRowPanel.panels = rowPanels;
       }
+
       return sourceRowPanel;
     }
 
@@ -682,11 +684,13 @@ export class DashboardModel implements TimeModel {
     if (sourceRowPanel.collapsed) {
       rowPanels = cloneDeep(sourceRowPanel.panels) ?? [];
       clone.panels = rowPanels;
+
       // insert copied row after preceding row
       insertPos = sourcePanelIndex + valueIndex;
     } else {
       rowPanels = this.getRowPanels(sourcePanelIndex);
       clone.panels = rowPanels.map((panel) => panel.getSaveModel());
+
       // insert copied row after preceding row's panels
       insertPos = sourcePanelIndex + (rowPanels.length + 1) * valueIndex;
     }
@@ -757,59 +761,56 @@ export class DashboardModel implements TimeModel {
     }
   }
 
-  repeatRow(panel: PanelModel, panelIndex: number, variable: any) {
+  repeatRow(panel: PanelModel, panelIndex: number, variable: TypedVariableModel) {
     const selectedOptions = this.getSelectedVariableOptions(variable);
-    let yPos = panel.gridPos.y;
-
-    function setScopedVars(panel: PanelModel, variableOption: any) {
-      panel.scopedVars ??= {};
-      panel.scopedVars[variable.name] = variableOption;
-    }
 
     for (let optionIndex = 0; optionIndex < selectedOptions.length; optionIndex++) {
-      const option = selectedOptions[optionIndex];
+      const curOption = selectedOptions[optionIndex];
-      const rowCopy = this.getRowRepeatClone(panel, optionIndex, panelIndex);
+      const rowClone = this.getRowRepeatClone(panel, optionIndex, panelIndex);
 
-      setScopedVars(rowCopy, option);
+      setScopedVars(rowClone, variable, curOption);
 
-      const rowHeight = this.getRowHeight(rowCopy);
+      const rowHeight = this.getRowHeight(rowClone);
-      const rowPanels = rowCopy.panels || [];
+      const panelsInRow = rowClone.panels || [];
       let panelBelowIndex;
 
       if (panel.collapsed) {
-        // For collapsed row just copy its panels and set scoped vars and proper IDs
+        // For a collapsed row, just copy its panels, set scoped vars and proper IDs
-        for (const rowPanel of rowPanels) {
+        for (const panelInRow of panelsInRow) {
-          setScopedVars(rowPanel, option);
+          setScopedVars(panelInRow, variable, curOption);
           if (optionIndex > 0) {
-            this.updateRepeatedPanelIds(rowPanel, true);
+            this.updateRepeatedPanelIds(panelInRow, true);
           }
         }
-        rowCopy.gridPos.y += optionIndex;
+
-        yPos += optionIndex;
+        // push nth row clone's y-pos down by n
+        rowClone.gridPos.y += optionIndex;
         panelBelowIndex = panelIndex + optionIndex + 1;
       } else {
-        // insert after 'row' panel
+        // insert after row panel
-        const insertPos = panelIndex + (rowPanels.length + 1) * optionIndex + 1;
+        const insertPos = panelIndex + (panelsInRow.length + 1) * optionIndex + 1;
-        rowPanels.forEach((rowPanel: PanelModel, i: number) => {
+        panelsInRow.forEach((panelInRow, i) => {
-          setScopedVars(rowPanel, option);
+          setScopedVars(panelInRow, variable, curOption);
+
           if (optionIndex > 0) {
-            const cloneRowPanel = new PanelModel(rowPanel);
+            const panelInRowClone = new PanelModel(panelInRow);
-            this.updateRepeatedPanelIds(cloneRowPanel, true);
+            this.updateRepeatedPanelIds(panelInRowClone, true);
-            // For exposed row additionally set proper Y grid position and add it to dashboard panels
+
-            cloneRowPanel.gridPos.y += rowHeight * optionIndex;
+            // For exposed row, set correct grid y-position and add it to dashboard panels
-            this.panels.splice(insertPos + i, 0, cloneRowPanel);
+            panelInRowClone.gridPos.y += rowHeight * optionIndex;
+            this.panels.splice(insertPos + i, 0, panelInRowClone);
           }
         });
-        rowCopy.panels = [];
+
-        rowCopy.gridPos.y += rowHeight * optionIndex;
+        rowClone.panels = [];
-        yPos += rowHeight;
+        rowClone.gridPos.y += rowHeight * optionIndex;
-        panelBelowIndex = insertPos + rowPanels.length;
+        panelBelowIndex = insertPos + panelsInRow.length;
       }
 
       // Update gridPos for panels below if we inserted more than 1 repeated row panel
       if (selectedOptions.length > 1) {
         for (const panel of this.panels.slice(panelBelowIndex)) {
-          panel.gridPos.y += yPos;
+          panel.gridPos.y += rowHeight;
         }
       }
     }
@@ -841,12 +842,13 @@ export class DashboardModel implements TimeModel {
   getRowHeight(rowPanel: PanelModel): number {
     if (!rowPanel.panels || rowPanel.panels.length === 0) {
       return 0;
+    } else if (rowPanel.collapsed) {
+      // A collapsed row will always have height 1
+      return 1;
     }
 
-    const rowYPos = rowPanel.gridPos.y;
+    const maxYPos = maxBy(rowPanel.panels, ({ gridPos }) => gridPos.y + gridPos.h)!.gridPos;
-    const positions = map(rowPanel.panels, 'gridPos');
+    return maxYPos.y + maxYPos.h - rowPanel.gridPos.y;
-    const maxPos = maxBy(positions, (pos: GridPos) => pos.y + pos.h);
-    return maxPos!.y + maxPos!.h - rowYPos;
   }
 
   removePanel(panel: PanelModel) {
@@ -1300,3 +1302,8 @@ export class DashboardModel implements TimeModel {
 function isPanelWithLegend(panel: PanelModel): panel is PanelModel & Pick<Required<PanelModel>, 'legend'> {
   return Boolean(panel.legend);
 }
+
+function setScopedVars(panel: PanelModel, variable: TypedVariableModel, variableOption: any) {
+  panel.scopedVars ??= {};
+  panel.scopedVars[variable.name] = variableOption;
+}