[EuiFlyout] Performance cleanup (#7462)

Co-authored-by: Cee Chen <constance.chen@elastic.co>
elastic · Mar 14, 2024 · f4e7570 · f4e7570
1 parent 6c86d6e
commit f4e7570
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 20 deletions.
diff --git a/changelogs/upcoming/7462.md b/changelogs/upcoming/7462.md
@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed `EuiFlyout` to not repeatedly remove/add a body class on resize
diff --git a/src/components/flyout/__snapshots__/flyout.test.tsx.snap b/src/components/flyout/__snapshots__/flyout.test.tsx.snap
@@ -1070,7 +1070,7 @@ Array [
 
 exports[`EuiFlyout renders extra screen reader instructions when fixed EuiHeaders headers exist on the page 1`] = `
 <body
-  class="euiBody--headerIsFixed euiBody--hasFlyout"
+  class="euiBody--hasFlyout euiBody--headerIsFixed"
 >
   <div>
     <div

diff --git a/src/components/flyout/flyout.test.tsx b/src/components/flyout/flyout.test.tsx
@@ -278,5 +278,34 @@ describe('EuiFlyout', () => {
         ).toMatchSnapshot();
       });
     });
+
+    describe('body class', () => {
+      it('adds `.euiBody--hasFlyout` class on mount', () => {
+        render(<EuiFlyout onClose={() => {}} />);
+        expect(document.body).toHaveClass('euiBody--hasFlyout');
+      });
+
+      it('removes `.euiBody--hasFlyout` class on unmount', () => {
+        const { unmount } = render(<EuiFlyout onClose={() => {}} />);
+        unmount();
+        expect(document.body).not.toHaveClass('euiBody--hasFlyout');
+      });
+
+      // Regression testing
+      it('should not remove and re-add `.euiBody--hasFlyout` class on resize', async () => {
+        const add = jest.spyOn(document.body.classList, 'add');
+        const remove = jest.spyOn(document.body.classList, 'remove');
+        const { rerender } = render(
+          <EuiFlyout onClose={() => {}} size={500} />
+        );
+
+        expect(add).toHaveBeenCalledTimes(1);
+        expect(add).toHaveBeenLastCalledWith('euiBody--hasFlyout');
+
+        rerender(<EuiFlyout onClose={() => {}} size={600} />);
+        expect(add).toHaveBeenCalledTimes(1);
+        expect(remove).not.toHaveBeenCalled();
+      });
+    });
   });
 });
diff --git a/src/components/flyout/flyout.tsx b/src/components/flyout/flyout.tsx
@@ -214,36 +214,33 @@ export const EuiFlyout = forwardRef(
       null
     );
     const setRef = useCombinedRefs([setResizeRef, ref]);
-    // TODO: Allow this hook to be conditional
-    const dimensions = useResizeObserver(resizeRef);
+    const { width } = useResizeObserver(isPushed ? resizeRef : null, 'width');
 
     useEffect(() => {
-      // This class doesn't actually do anything by EUI, but is nice to add for consumers (JIC)
-      document.body.classList.add('euiBody--hasFlyout');
-
       /**
        * Accomodate for the `isPushed` state by adding padding to the body equal to the width of the element
        */
       if (isPushed) {
-        if (side === 'right') {
-          document.body.style.paddingInlineEnd = `${dimensions.width}px`;
-        } else if (side === 'left') {
-          document.body.style.paddingInlineStart = `${dimensions.width}px`;
-        }
+        const paddingSide =
+          side === 'left' ? 'paddingInlineStart' : 'paddingInlineEnd';
+
+        document.body.style[paddingSide] = `${width}px`;
+        return () => {
+          document.body.style[paddingSide] = '';
+        };
       }
+    }, [isPushed, side, width]);
 
+    /**
+     * This class doesn't actually do anything by EUI, but is nice to add for consumers (JIC)
+     */
+    useEffect(() => {
+      document.body.classList.add('euiBody--hasFlyout');
       return () => {
+        // Remove the hasFlyout class when the flyout is unmounted
         document.body.classList.remove('euiBody--hasFlyout');
-
-        if (isPushed) {
-          if (side === 'right') {
-            document.body.style.paddingInlineEnd = '';
-          } else if (side === 'left') {
-            document.body.style.paddingInlineStart = '';
-          }
-        }
       };
-    }, [side, dimensions, isPushed]);
+    }, []);
 
     /**
      * ESC key closes flyout (always?)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		Bug fixes

		- Fixed `EuiFlyout` to not repeatedly remove/add a body class on resize
-Original file line number
+Diff line change
@@ Expand Up / @@ -1070,7 +1070,7 @@ Array [ @@
     exports[`EuiFlyout renders extra screen reader instructions when fixed EuiHeaders headers exist on the page 1`] = `
     <body
-      class="euiBody--headerIsFixed euiBody--hasFlyout"
+      class="euiBody--hasFlyout euiBody--headerIsFixed"
     >
       <div>
         <div
@@ Expand Down @@