fix(ui): avoid annotation re-renders

ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js

@@ -69,7 +69,6 @@ const Alert = ({
               value={a.value}
               visible={a.visible}
               afterUpdate={afterUpdate}
-              alertStore={alertStore}
             />
           ))}
       </div>

ui/src/Components/Grid/AlertGrid/AlertGroup/Annotation/index.js

@@ -12,11 +12,10 @@ import { faExternalLinkAlt } from "@fortawesome/free-solid-svg-icons/faExternalL
 import { faSearchPlus } from "@fortawesome/free-solid-svg-icons/faSearchPlus";
 import { faSearchMinus } from "@fortawesome/free-solid-svg-icons/faSearchMinus";
 
-import { AlertStore } from "Stores/AlertStore";
 import { TooltipWrapper } from "Components/TooltipWrapper";
 
 const RenderNonLinkAnnotation = observer(
-  ({ alertStore, name, value, visible, afterUpdate }) => {
+  ({ name, value, visible, afterUpdate }) => {
     const mountRef = useRef(false);
 
     useEffect(() => {
@@ -74,7 +73,6 @@ const RenderNonLinkAnnotation = observer(
   }
 );
 RenderNonLinkAnnotation.propTypes = {
-  alertStore: PropTypes.instanceOf(AlertStore).isRequired,
   name: PropTypes.string.isRequired,
   value: PropTypes.string.isRequired,
   visible: PropTypes.bool.isRequired,

ui/src/Components/Grid/AlertGrid/AlertGroup/Annotation/index.test.js

@@ -4,15 +4,8 @@ import { shallow, mount } from "enzyme";
 
 import toDiffableHtml from "diffable-html";
 
-import { AlertStore } from "Stores/AlertStore";
 import { RenderNonLinkAnnotation, RenderLinkAnnotation } from ".";
 
-let alertStore;
-
-beforeEach(() => {
-  alertStore = new AlertStore([]);
-});
-
 const ShallowLinkAnnotation = () => {
   return shallow(
     <RenderLinkAnnotation name="annotation name" value="http://localhost/foo" />
@@ -38,7 +31,6 @@ const MockAfterUpdate = jest.fn();
 const ShallowNonLinkAnnotation = (visible) => {
   return shallow(
     <RenderNonLinkAnnotation
-      alertStore={alertStore}
       name="foo"
       value="some long text"
       visible={visible}
@@ -50,7 +42,6 @@ const ShallowNonLinkAnnotation = (visible) => {
 const MountedNonLinkAnnotation = (visible) => {
   return mount(
     <RenderNonLinkAnnotation
-      alertStore={alertStore}
       name="foo"
       value="some long text"
       visible={visible}
@@ -62,7 +53,6 @@ const MountedNonLinkAnnotation = (visible) => {
 const MountedNonLinkAnnotationContainingLink = (visible) => {
   return mount(
     <RenderNonLinkAnnotation
-      alertStore={alertStore}
       name="foo"
       value="some long text with http://example.com link"
       visible={visible}

ui/src/Components/Grid/AlertGrid/AlertGroup/GroupFooter/index.js

@@ -30,7 +30,6 @@ const GroupFooter = ({
               value={a.value}
               visible={a.visible}
               afterUpdate={afterUpdate}
-              alertStore={alertStore}
             />
           ))}
       </div>

ui/src/Components/Grid/AlertGrid/AlertGroup/index.js

@@ -55,7 +55,7 @@ const AlertGroup = observer(
       alertStore: PropTypes.instanceOf(AlertStore).isRequired,
       settingsStore: PropTypes.instanceOf(Settings).isRequired,
       silenceFormStore: PropTypes.instanceOf(SilenceFormStore).isRequired,
-      style: PropTypes.object,
+      groupWidth: PropTypes.number.isRequired,
       gridLabelValue: PropTypes.string.isRequired,
     };
 
@@ -185,7 +185,7 @@ const AlertGroup = observer(
         silenceFormStore,
         alertStore,
         settingsStore,
-        style,
+        groupWidth,
         gridLabelValue,
       } = this.props;
 
@@ -221,19 +221,17 @@ const AlertGroup = observer(
         }
       }
 
-      let extraStyle = {};
-      if (this.renderConfig.isMenuOpen) {
-        extraStyle.zIndex = 100;
-      }
-
       return (
         <div
           className={`components-grid-alertgrid-alertgroup ${
             this.renderConfig.animationDone
               ? "components-animation-fade-appear-done"
               : ""
           }`}
-          style={{ ...style, ...extraStyle }}
+          style={{
+            width: groupWidth,
+            zIndex: this.renderConfig.isMenuOpen ? 100 : null,
+          }}
         >
           <Fade
             in={this.context.animations.in}

ui/src/Components/Grid/AlertGrid/AlertGroup/index.test.js

@@ -70,6 +70,7 @@ const MountedAlertGroup = (afterUpdate, showAlertmanagers) => {
       alertStore={alertStore}
       silenceFormStore={silenceFormStore}
       gridLabelValue=""
+      groupWidth={420}
     />,
     {
       wrappingComponent: ThemeContext.Provider,

ui/src/Components/Grid/AlertGrid/Grid.js

@@ -32,7 +32,7 @@ const Grid = ({
   outerPadding,
 }) => {
   const { ref, repack } = useGrid(gridSizesConfig);
-  const debouncedRepack = debounce(repack, 10);
+  const debouncedRepack = useCallback(debounce(repack, 10), [repack]);
 
   const [groupsToRender, setGroupsToRender] = useState(50);
 
@@ -115,23 +115,23 @@ const Grid = ({
         }}
       >
         {isExpanded || grid.labelName === ""
-          ? grid.alertGroups.slice(0, groupsToRender).map((group) => (
-              <AlertGroup
-                key={group.id}
-                group={group}
-                showAlertmanagers={
-                  Object.keys(alertStore.data.upstreams.clusters).length > 1
-                }
-                afterUpdate={debouncedRepack}
-                alertStore={alertStore}
-                settingsStore={settingsStore}
-                silenceFormStore={silenceFormStore}
-                style={{
-                  width: groupWidth,
-                }}
-                gridLabelValue={grid.labelValue}
-              />
-            ))
+          ? grid.alertGroups
+              .slice(0, groupsToRender)
+              .map((group) => (
+                <AlertGroup
+                  key={group.id}
+                  group={group}
+                  showAlertmanagers={
+                    Object.keys(alertStore.data.upstreams.clusters).length > 1
+                  }
+                  afterUpdate={debouncedRepack}
+                  alertStore={alertStore}
+                  settingsStore={settingsStore}
+                  silenceFormStore={silenceFormStore}
+                  groupWidth={groupWidth}
+                  gridLabelValue={grid.labelValue}
+                />
+              ))
           : []}
       </div>
       {isExpanded && grid.alertGroups.length > groupsToRender && (

ui/src/Components/Grid/AlertGrid/index.test.js

@@ -337,7 +337,7 @@ describe("<AlertGrid />", () => {
       groupWidth: wrapper.instance().viewport.groupWidth,
     });
 
-    expect(tree.find("AlertGroup").at(0).props().style.width).toBe(
+    expect(tree.find("AlertGroup").at(0).props().groupWidth).toBe(
       Math.floor(innerWidth / columns)
     );
   };
@@ -419,15 +419,15 @@ describe("<AlertGrid />", () => {
       gridSizesConfig: wrapper.instance().viewport.gridSizesConfig,
       groupWidth: wrapper.instance().viewport.groupWidth,
     });
-    expect(tree.find("AlertGroup").at(0).props().style.width).toBe(1980 / 4);
+    expect(tree.find("AlertGroup").at(0).props().groupWidth).toBe(1980 / 4);
 
     // then resize and verify if column count was changed
     wrapper.instance().viewport.updateWidths(1000, 1000);
     tree.setProps({
       gridSizesConfig: wrapper.instance().viewport.gridSizesConfig,
       groupWidth: wrapper.instance().viewport.groupWidth,
     });
-    expect(tree.find("AlertGroup").at(0).props().style.width).toBe(1000 / 2);
+    expect(tree.find("AlertGroup").at(0).props().groupWidth).toBe(1000 / 2);
   });
 
   it("scrollbar render doesn't resize alert groups", () => {
@@ -443,15 +443,15 @@ describe("<AlertGrid />", () => {
       gridSizesConfig: wrapper.instance().viewport.gridSizesConfig,
       groupWidth: wrapper.instance().viewport.groupWidth,
     });
-    expect(tree.find("AlertGroup").at(0).props().style.width).toBe(400);
+    expect(tree.find("AlertGroup").at(0).props().groupWidth).toBe(400);
 
     // then resize and verify if column count was changed
     wrapper.instance().viewport.updateWidths(1584, 1600);
     tree.setProps({
       gridSizesConfig: wrapper.instance().viewport.gridSizesConfig,
       groupWidth: wrapper.instance().viewport.groupWidth,
     });
-    expect(tree.find("AlertGroup").at(0).props().style.width).toBe(396);
+    expect(tree.find("AlertGroup").at(0).props().groupWidth).toBe(396);
   });
 
   it("window resize event calls updateWidths", () => {
@@ -480,7 +480,7 @@ describe("<AlertGrid />", () => {
         gridSizesConfig: wrapper.instance().viewport.gridSizesConfig,
         groupWidth: wrapper.instance().viewport.groupWidth,
       });
-      results.push(tree.find("AlertGroup").at(0).props().style.width);
+      results.push(tree.find("AlertGroup").at(0).props().groupWidth);
     }
 
     expect(results).toStrictEqual([

ui/src/Hooks/useGrid.js

@@ -1,16 +1,11 @@
-import { useEffect, useRef } from "react";
+import { useEffect, useRef, useState } from "react";
 
 import Bricks from "bricks.js";
 
 const useGrid = (sizes) => {
   const ref = useRef(null);
   const grid = useRef(null);
-
-  const repack = () => {
-    if (grid.current) {
-      grid.current.pack();
-    }
-  };
+  const [repack, setRepack] = useState(() => () => {});
 
   useEffect(() => {
     if (!grid.current && ref.current) {
@@ -20,12 +15,15 @@ const useGrid = (sizes) => {
         packed: "packed",
         position: false,
       });
-      window.addEventListener("resize", repack);
+      window.addEventListener("resize", grid.current.pack);
       grid.current.pack();
+      setRepack(() => () => {
+        grid.current && grid.current.pack();
+      });
     }
 
     return () => {
-      window.removeEventListener("resize", repack);
+      if (grid.current) window.removeEventListener("resize", grid.current.pack);
       grid.current = null;
     };
   }, [sizes]);

ui/src/Hooks/useGrid.test.js

@@ -1,6 +1,6 @@
 import React from "react";
 
-import { renderHook, act } from "@testing-library/react-hooks";
+import { renderHook } from "@testing-library/react-hooks";
 
 import { mount } from "enzyme";
 
@@ -57,4 +57,11 @@ describe("useGrid", () => {
     const tree = mount(<Component count={4} />);
     tree.unmount();
   });
+
+  it("repack after unmount does nothing", () => {
+    const tree = mount(<Component count={4} />);
+    const repack = tree.find("#root").props().onClick;
+    tree.unmount();
+    repack();
+  });
 });