[kbn-grid-layout] Cleanup memoization and styling (elastic#210285)

## Summary This PR cleans up the `kbn-grid-layout` code in two ways: 1. Rather than memoizing components in their parents, I swapped to using `React.memo` for all components, which accomplishes the same behaviour in a slightly cleaner way. 2. I moved all Emotion style definitions **outside** of the React components so that we no longer have to re-parse the CSS string on every render (see [this comment](elastic/eui#6828 (comment))). ### 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)
drewdaemon · Feb 10, 2025 · 219f31a · 219f31a
1 parent d2c5132
commit 219f31a
Show file tree

Hide file tree

Showing 10 changed files with 671 additions and 673 deletions.
diff --git a/src/platform/packages/private/kbn-grid-layout/grid/drag_preview.tsx b/src/platform/packages/private/kbn-grid-layout/grid/drag_preview.tsx
@@ -14,54 +14,51 @@ import { css } from '@emotion/react';
 
 import { GridLayoutStateManager } from './types';
 
-export const DragPreview = ({
-  rowIndex,
-  gridLayoutStateManager,
-}: {
-  rowIndex: number;
-  gridLayoutStateManager: GridLayoutStateManager;
-}) => {
-  const dragPreviewRef = useRef<HTMLDivElement | null>(null);
+export const DragPreview = React.memo(
+  ({
+    rowIndex,
+    gridLayoutStateManager,
+  }: {
+    rowIndex: number;
+    gridLayoutStateManager: GridLayoutStateManager;
+  }) => {
+    const dragPreviewRef = useRef<HTMLDivElement | null>(null);
 
-  useEffect(
-    () => {
-      /** Update the styles of the drag preview via a subscription to prevent re-renders */
-      const styleSubscription = combineLatest([
-        gridLayoutStateManager.activePanel$,
-        gridLayoutStateManager.proposedGridLayout$,
-      ])
-        .pipe(skip(1)) // skip the first emit because the drag preview is only rendered after a user action
-        .subscribe(([activePanel, proposedGridLayout]) => {
-          if (!dragPreviewRef.current) return;
+    useEffect(
+      () => {
+        /** Update the styles of the drag preview via a subscription to prevent re-renders */
+        const styleSubscription = combineLatest([
+          gridLayoutStateManager.activePanel$,
+          gridLayoutStateManager.proposedGridLayout$,
+        ])
+          .pipe(skip(1)) // skip the first emit because the drag preview is only rendered after a user action
+          .subscribe(([activePanel, proposedGridLayout]) => {
+            if (!dragPreviewRef.current) return;
 
-          if (!activePanel || !proposedGridLayout?.[rowIndex].panels[activePanel.id]) {
-            dragPreviewRef.current.style.display = 'none';
-          } else {
-            const panel = proposedGridLayout[rowIndex].panels[activePanel.id];
-            dragPreviewRef.current.style.display = 'block';
-            dragPreviewRef.current.style.gridColumnStart = `${panel.column + 1}`;
-            dragPreviewRef.current.style.gridColumnEnd = `${panel.column + 1 + panel.width}`;
-            dragPreviewRef.current.style.gridRowStart = `${panel.row + 1}`;
-            dragPreviewRef.current.style.gridRowEnd = `${panel.row + 1 + panel.height}`;
-          }
-        });
+            if (!activePanel || !proposedGridLayout?.[rowIndex].panels[activePanel.id]) {
+              dragPreviewRef.current.style.display = 'none';
+            } else {
+              const panel = proposedGridLayout[rowIndex].panels[activePanel.id];
+              dragPreviewRef.current.style.display = 'block';
+              dragPreviewRef.current.style.gridColumnStart = `${panel.column + 1}`;
+              dragPreviewRef.current.style.gridColumnEnd = `${panel.column + 1 + panel.width}`;
+              dragPreviewRef.current.style.gridRowStart = `${panel.row + 1}`;
+              dragPreviewRef.current.style.gridRowEnd = `${panel.row + 1 + panel.height}`;
+            }
+          });
 
-      return () => {
-        styleSubscription.unsubscribe();
-      };
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    []
-  );
+        return () => {
+          styleSubscription.unsubscribe();
+        };
+      },
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+      []
+    );
 
-  return (
-    <div
-      ref={dragPreviewRef}
-      className={'kbnGridPanel--dragPreview'}
-      css={css`
-        display: none;
-        pointer-events: none;
-      `}
-    />
-  );
-};
+    return <div ref={dragPreviewRef} className={'kbnGridPanel--dragPreview'} css={styles} />;
+  }
+);
+
+const styles = css({ display: 'none', pointerEvents: 'none' });
+
+DragPreview.displayName = 'KbnGridLayoutDragPreview';
diff --git a/src/platform/packages/private/kbn-grid-layout/grid/grid_height_smoother.tsx b/src/platform/packages/private/kbn-grid-layout/grid/grid_height_smoother.tsx
@@ -12,58 +12,67 @@ import React, { PropsWithChildren, useEffect, useRef } from 'react';
 import { combineLatest } from 'rxjs';
 import { GridLayoutStateManager } from './types';
 
-export const GridHeightSmoother = ({
-  children,
-  gridLayoutStateManager,
-}: PropsWithChildren<{ gridLayoutStateManager: GridLayoutStateManager }>) => {
-  // set the parent div size directly to smooth out height changes.
-  const smoothHeightRef = useRef<HTMLDivElement | null>(null);
+export const GridHeightSmoother = React.memo(
+  ({
+    children,
+    gridLayoutStateManager,
+  }: PropsWithChildren<{ gridLayoutStateManager: GridLayoutStateManager }>) => {
+    // set the parent div size directly to smooth out height changes.
+    const smoothHeightRef = useRef<HTMLDivElement | null>(null);
 
-  useEffect(() => {
-    /**
-     * When the user is interacting with an element, the page can grow, but it cannot
-     * shrink. This is to stop a behaviour where the page would scroll up automatically
-     * making the panel shrink or grow unpredictably.
-     */
-    const interactionStyleSubscription = combineLatest([
-      gridLayoutStateManager.gridDimensions$,
-      gridLayoutStateManager.interactionEvent$,
-    ]).subscribe(([dimensions, interactionEvent]) => {
-      if (!smoothHeightRef.current || gridLayoutStateManager.expandedPanelId$.getValue()) return;
+    useEffect(() => {
+      /**
+       * When the user is interacting with an element, the page can grow, but it cannot
+       * shrink. This is to stop a behaviour where the page would scroll up automatically
+       * making the panel shrink or grow unpredictably.
+       */
+      const interactionStyleSubscription = combineLatest([
+        gridLayoutStateManager.gridDimensions$,
+        gridLayoutStateManager.interactionEvent$,
+      ]).subscribe(([dimensions, interactionEvent]) => {
+        if (!smoothHeightRef.current || gridLayoutStateManager.expandedPanelId$.getValue()) return;
 
-      if (!interactionEvent) {
-        smoothHeightRef.current.style.minHeight = `${dimensions.height}px`;
-        return;
-      }
-      smoothHeightRef.current.style.minHeight = `${
-        smoothHeightRef.current.getBoundingClientRect().height
-      }px`;
-    });
+        if (!interactionEvent) {
+          smoothHeightRef.current.style.minHeight = `${dimensions.height}px`;
+          return;
+        }
+        smoothHeightRef.current.style.minHeight = `${
+          smoothHeightRef.current.getBoundingClientRect().height
+        }px`;
+      });
 
-    return () => {
-      interactionStyleSubscription.unsubscribe();
-    };
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+      return () => {
+        interactionStyleSubscription.unsubscribe();
+      };
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
 
-  return (
-    <div
-      ref={smoothHeightRef}
-      className={'kbnGridWrapper'}
-      css={css`
-        height: 100%;
-        overflow-anchor: none;
-        transition: min-height 500ms linear;
+    return (
+      <div
+        ref={smoothHeightRef}
+        className={'kbnGridWrapper'}
+        css={[styles.heightSmoothing, styles.hasActivePanel]}
+      >
+        {children}
+      </div>
+    );
+  }
+);
 
-        &:has(.kbnGridPanel--expanded) {
-          min-height: 100% !important;
-          max-height: 100vh; // fallback in case if the parent doesn't set the height correctly
-          position: relative;
-          transition: none;
-        }
-      `}
-    >
-      {children}
-    </div>
-  );
+const styles = {
+  heightSmoothing: css({
+    height: '100%',
+    overflowAnchor: 'none',
+    transition: 'min-height 500ms linear',
+  }),
+  hasActivePanel: css({
+    '&:has(.kbnGridPanel--expanded)': {
+      minHeight: '100% !important',
+      maxHeight: '100vh', // fallback in case if the parent doesn't set the height correctly
+      position: 'relative',
+      transition: 'none',
+    },
+  }),
 };
+
+GridHeightSmoother.displayName = 'KbnGridLayoutHeightSmoother';
diff --git a/src/platform/packages/private/kbn-grid-layout/grid/grid_layout.tsx b/src/platform/packages/private/kbn-grid-layout/grid/grid_layout.tsx
@@ -9,7 +9,7 @@
 
 import classNames from 'classnames';
 import { cloneDeep } from 'lodash';
-import React, { useEffect, useMemo, useRef, useState } from 'react';
+import React, { useEffect, useRef, useState } from 'react';
 import { combineLatest, distinctUntilChanged, map, pairwise, skip } from 'rxjs';
 
 import { css } from '@emotion/react';
@@ -134,22 +134,6 @@ export const GridLayout = ({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
-  /**
-   * Memoize row children components to prevent unnecessary re-renders
-   */
-  const children = useMemo(() => {
-    return Array.from({ length: rowCount }, (_, rowIndex) => {
-      return (
-        <GridRow
-          key={rowIndex}
-          rowIndex={rowIndex}
-          renderPanelContents={renderPanelContents}
-          gridLayoutStateManager={gridLayoutStateManager}
-        />
-      );
-    });
-  }, [rowCount, gridLayoutStateManager, renderPanelContents]);
-
   return (
     <GridHeightSmoother gridLayoutStateManager={gridLayoutStateManager}>
       <div
@@ -158,72 +142,83 @@ export const GridLayout = ({
           setDimensionsRef(divElement);
         }}
         className={classNames('kbnGrid', className)}
-        css={css`
-          padding: calc(var(--kbnGridGutterSize) * 1px);
-
-          // disable pointer events and user select on drag + resize
-          &:has(.kbnGridPanel--active) {
-            user-select: none;
-            pointer-events: none;
-          }
-
-          &:has(.kbnGridPanel--expanded) {
-            ${expandedPanelStyles}
-          }
-          &.kbnGrid--mobileView {
-            ${singleColumnStyles}
-          }
-        `}
+        css={[
+          styles.layoutPadding,
+          styles.hasActivePanel,
+          styles.singleColumn,
+          styles.hasExpandedPanel,
+        ]}
       >
-        {children}
+        {Array.from({ length: rowCount }, (_, rowIndex) => {
+          return (
+            <GridRow
+              key={rowIndex}
+              rowIndex={rowIndex}
+              renderPanelContents={renderPanelContents}
+              gridLayoutStateManager={gridLayoutStateManager}
+            />
+          );
+        })}
       </div>
     </GridHeightSmoother>
   );
 };
 
-const singleColumnStyles = css`
-  .kbnGridRow {
-    grid-template-columns: 100%;
-    grid-template-rows: auto;
-    grid-auto-flow: row;
-    grid-auto-rows: auto;
-  }
-
-  .kbnGridPanel {
-    grid-area: unset !important;
-  }
-`;
-
-const expandedPanelStyles = css`
-  height: 100%;
-
-  & .kbnGridRowContainer:has(.kbnGridPanel--expanded) {
-    // targets the grid row container that contains the expanded panel
-    .kbnGridRowHeader {
-      height: 0px; // used instead of 'display: none' due to a11y concerns
-    }
-    .kbnGridRow {
-      display: block !important; // overwrite grid display
-      height: 100%;
-      .kbnGridPanel {
-        &.kbnGridPanel--expanded {
-          height: 100% !important;
-        }
-        &:not(.kbnGridPanel--expanded) {
-          // hide the non-expanded panels
-          position: absolute;
-          top: -9999px;
-          left: -9999px;
-          visibility: hidden; // remove hidden panels and their contents from tab order for a11y
-        }
-      }
-    }
-  }
-
-  & .kbnGridRowContainer:not(:has(.kbnGridPanel--expanded)) {
-    // targets the grid row containers that **do not** contain the expanded panel
-    position: absolute;
-    top: -9999px;
-    left: -9999px;
-  }
-`;
+const styles = {
+  layoutPadding: css({
+    padding: 'calc(var(--kbnGridGutterSize) * 1px)',
+  }),
+  hasActivePanel: css({
+    '&:has(.kbnGridPanel--active)': {
+      // disable pointer events and user select on drag + resize
+      userSelect: 'none',
+      pointerEvents: 'none',
+    },
+  }),
+  singleColumn: css({
+    '&.kbnGrid--mobileView': {
+      '.kbnGridRow': {
+        gridTemplateAreas: '100%',
+        gridTemplateRows: 'auto',
+        gridAutoFlow: 'row',
+        gridAutoRows: 'auto',
+      },
+      '.kbnGridPanel': {
+        gridArea: 'unset !important',
+      },
+    },
+  }),
+  hasExpandedPanel: css({
+    '&:has(.kbnGridPanel--expanded)': {
+      height: '100%',
+      // targets the grid row container that contains the expanded panel
+      '& .kbnGridRowContainer:has(.kbnGridPanel--expanded)': {
+        '.kbnGridRowHeader': {
+          height: '0px', // used instead of 'display: none' due to a11y concerns
+        },
+        '.kbnGridRow': {
+          display: 'block !important', // overwrite grid display
+          height: '100%',
+          '.kbnGridPanel': {
+            '&.kbnGridPanel--expanded': {
+              height: '100% !important',
+            },
+            // hide the non-expanded panels
+            '&:not(.kbnGridPanel--expanded)': {
+              position: 'absolute',
+              top: '-9999px',
+              left: '-9999px',
+              visibility: 'hidden', // remove hidden panels and their contents from tab order for a11y
+            },
+          },
+        },
+      },
+      // targets the grid row containers that **do not** contain the expanded panel
+      '& .kbnGridRowContainer:not(:has(.kbnGridPanel--expanded))': {
+        position: 'absolute',
+        top: '-9999px',
+        left: '-9999px',
+      },
+    },
+  }),
+};