Merge pull request #611 from balsa-asanovic/use-node-siblings-hook

Use component siblings custom hook

web/package/cockpit-agama.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Jun 21 09:05:21 UTC 2023 - Balsa Asanovic <balsaasanovic95@gmail.com>
+
+- Moved the logic for adding and removing HTML element's
+  attributes from sidebar component to custom hook
+  (gh#openSUSE/agama#565) .
+
 -------------------------------------------------------------------
 Tue Jun 13 15:40:01 UTC 2023 - David Diaz <dgonzalez@suse.com>
 

web/src/components/core/Sidebar.jsx

@@ -19,23 +19,12 @@
  * find current contact information at www.suse.com.
  */
 
-import React, { useEffect, useLayoutEffect, useRef, useState } from "react";
+import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from "react";
 import { Button, Text } from "@patternfly/react-core";
 import { Icon, AppActions } from "~/components/layout";
 import { If, NotificationMark } from "~/components/core";
 import { useNotification } from "~/context/notification";
-
-/**
- * Returns siblings for given HTML node
- *
- * @param {HTMLElement} node
- * @returns {HTMLElement[]}
- */
-const siblingsFor = (node) => {
-  const parent = node?.parentNode;
-
-  return parent ? [...parent.children].filter(n => n !== node) : [];
-};
+import useNodeSiblings from "~/hooks/useNodeSiblings";
 
 /**
  * Agama sidebar navigation
@@ -49,35 +38,30 @@ export default function Sidebar ({ children }) {
   const asideRef = useRef(null);
   const closeButtonRef = useRef(null);
   const [notification] = useNotification();
+  const [addAttribute, removeAttribute] = useNodeSiblings(asideRef.current);
 
   /**
    * Set siblings as not interactive and not discoverable
    */
-  const makeSiblingsInert = () => {
-    siblingsFor(asideRef.current).forEach(s => {
-      s.setAttribute('inert', '');
-      s.setAttribute('aria-hidden', true);
-    });
-  };
+  const makeSiblingsInert = useCallback(() => {
+    addAttribute('inert', '');
+    addAttribute('aria-hidden', true);
+  }, [addAttribute]);
 
   /**
    * Set siblings as interactive and discoverable
    */
-  const makeSiblingsAlive = () => {
-    siblingsFor(asideRef.current).forEach(s => {
-      s.removeAttribute('inert');
-      s.removeAttribute('aria-hidden');
-    });
-  };
+  const makeSiblingsAlive = useCallback(() => {
+    removeAttribute('inert');
+    removeAttribute('aria-hidden');
+  }, [removeAttribute]);
 
   const open = () => {
     setIsOpen(true);
-    makeSiblingsInert();
   };
 
   const close = () => {
     setIsOpen(false);
-    makeSiblingsAlive();
   };
 
   /**
@@ -97,16 +81,21 @@ export default function Sidebar ({ children }) {
   };
 
   useEffect(() => {
-    if (isOpen) closeButtonRef.current.focus();
-  }, [isOpen]);
+    if (isOpen) {
+      closeButtonRef.current.focus();
+      makeSiblingsInert();
+    } else {
+      makeSiblingsAlive();
+    }
+  }, [isOpen, makeSiblingsInert, makeSiblingsAlive]);
 
   useLayoutEffect(() => {
     // Ensure siblings do not remain inert when the component is unmounted.
     // Using useLayoutEffect over useEffect for allowing the cleanup function to
     // be executed immediately BEFORE unmounting the component and still having
     // access to siblings.
     return () => makeSiblingsAlive();
-  }, []);
+  }, [makeSiblingsAlive]);
 
   // display additional info when running in a development server
   let targetInfo = null;

web/src/hooks/useNodeSiblings.js

@@ -0,0 +1,48 @@
+import { noop } from "~/utils";
+
+/**
+ * Function for adding an attribute to a sibling
+ *
+ * @typedef {function} addAttributeFn
+ * @param {string} attribute - attribute name
+ * @param {*} value - value to set
+ */
+
+/**
+ * Function for removing an attribute from a sibling
+ *
+ * @typedef {function} removeAttributeFn
+ * @param {string} attribute - attribute name
+ */
+
+/**
+ * A hook for working with siblings of the node passed as parameter
+ *
+ * It returns an array with exactly two functions:
+ *   - First for adding given attribute to siblings
+ *   - Second for removing given attributes from siblings
+ *
+ * @param {HTMLElement} node
+ * @returns {[addAttributeFn, removeAttributeFn]}
+ */
+const useNodeSiblings = (node) => {
+  if (!node) return [noop, noop];
+
+  const siblings = [...node.parentNode.children].filter(n => n !== node);
+
+  const addAttribute = (attribute, value) => {
+    siblings.forEach(sibling => {
+      sibling.setAttribute(attribute, value);
+    });
+  };
+
+  const removeAttribute = (attribute) => {
+    siblings.forEach(sibling => {
+      sibling.removeAttribute(attribute);
+    });
+  };
+
+  return [addAttribute, removeAttribute];
+};
+
+export default useNodeSiblings;

web/src/hooks/useNodeSiblings.test.js

@@ -0,0 +1,54 @@
+import { renderHook } from '@testing-library/react';
+import useNodeSiblings from './useNodeSiblings';
+
+// Mocked HTMLElement for testing
+const mockNode = {
+  parentNode: {
+    children: [
+      { setAttribute: jest.fn(), removeAttribute: jest.fn() }, // sibling 1
+      { setAttribute: jest.fn(), removeAttribute: jest.fn() }, // sibling 2
+      { setAttribute: jest.fn(), removeAttribute: jest.fn() }, // sibling 3
+    ],
+  },
+};
+
+describe('useNodeSiblings', () => {
+  it('should return noop functions when node is not provided', () => {
+    const { result } = renderHook(() => useNodeSiblings(null));
+    const [addAttribute, removeAttribute] = result.current;
+
+    expect(addAttribute).toBeInstanceOf(Function);
+    expect(removeAttribute).toBeInstanceOf(Function);
+    expect(addAttribute).toEqual(expect.any(Function));
+    expect(removeAttribute).toEqual(expect.any(Function));
+
+    // Call the noop functions to ensure they don't throw any errors
+    expect(() => addAttribute('attribute', 'value')).not.toThrow();
+    expect(() => removeAttribute('attribute')).not.toThrow();
+  });
+
+  it('should add attribute to all siblings when addAttribute is called', () => {
+    const { result } = renderHook(() => useNodeSiblings(mockNode));
+    const [addAttribute] = result.current;
+    const attributeName = 'attribute';
+    const attributeValue = 'value';
+
+    addAttribute(attributeName, attributeValue);
+
+    expect(mockNode.parentNode.children[0].setAttribute).toHaveBeenCalledWith(attributeName, attributeValue);
+    expect(mockNode.parentNode.children[1].setAttribute).toHaveBeenCalledWith(attributeName, attributeValue);
+    expect(mockNode.parentNode.children[2].setAttribute).toHaveBeenCalledWith(attributeName, attributeValue);
+  });
+
+  it('should remove attribute from all siblings when removeAttribute is called', () => {
+    const { result } = renderHook(() => useNodeSiblings(mockNode));
+    const [, removeAttribute] = result.current;
+    const attributeName = 'attribute';
+
+    removeAttribute(attributeName);
+
+    expect(mockNode.parentNode.children[0].removeAttribute).toHaveBeenCalledWith(attributeName);
+    expect(mockNode.parentNode.children[1].removeAttribute).toHaveBeenCalledWith(attributeName);
+    expect(mockNode.parentNode.children[2].removeAttribute).toHaveBeenCalledWith(attributeName);
+  });
+});