fix: Improperly aligned unfolding sub-items in context menu in data browser (#2726)

Author: dblythy

src/components/ContextMenu/ContextMenu.react.js

@@ -5,80 +5,105 @@
  * This source code is licensed under the license found in the LICENSE file in
  * the root directory of this source tree.
  */
+
 import PropTypes from 'lib/PropTypes';
 import React, { useState, useEffect, useRef } from 'react';
 import styles from 'components/ContextMenu/ContextMenu.scss';
 
-const getPositionToFitVisibleScreen = ref => {
-  if (ref.current) {
-    const elBox = ref.current.getBoundingClientRect();
-    const y = elBox.y + elBox.height < window.innerHeight ? 0 : 0 - elBox.y + 100;
-
-    // If there's a previous element show current next to it.
-    // Try on right side first, then on left if there's no place.
-    const prevEl = ref.current.previousSibling;
-    if (prevEl) {
-      const prevElBox = prevEl.getBoundingClientRect();
-      const showOnRight = prevElBox.x + prevElBox.width + elBox.width < window.innerWidth;
-      return {
-        x: showOnRight ? prevElBox.width : -elBox.width,
-        y,
-      };
-    }
+const getPositionToFitVisibleScreen = (
+  ref,
+  offset = 0,
+  mainItemCount = 0,
+  subItemCount = 0
+) => {
+  if (!ref.current) {
+    return;
+  }
+
+  const elBox = ref.current.getBoundingClientRect();
+  const menuHeight = elBox.height;
+  const footerHeight = 50;
+  const lowerLimit = window.innerHeight - footerHeight;
+  const upperLimit = 0;
+
+  const shouldApplyOffset = mainItemCount === 0 || subItemCount > mainItemCount;
+  const prevEl = ref.current.previousSibling;
+
+  if (prevEl) {
+    const prevElBox = prevEl.getBoundingClientRect();
+    const showOnRight = prevElBox.x + prevElBox.width + elBox.width < window.innerWidth;
 
-    return { x: 0, y };
+    let proposedTop = shouldApplyOffset
+      ? prevElBox.top + offset
+      : prevElBox.top;
+
+    proposedTop = Math.max(upperLimit, Math.min(proposedTop, lowerLimit - menuHeight));
+
+    return {
+      x: showOnRight ? prevElBox.width : -elBox.width,
+      y: proposedTop - elBox.top,
+    };
   }
+
+  const proposedTop = elBox.top + offset;
+  const clampedTop = Math.max(upperLimit, Math.min(proposedTop, lowerLimit - menuHeight));
+  return {
+    x: 0,
+    y: clampedTop - elBox.top,
+  };
 };
 
-const MenuSection = ({ level, items, path, setPath, hide }) => {
+const MenuSection = ({ level, items, path, setPath, hide, parentItemCount = 0 }) => {
   const sectionRef = useRef(null);
-  const [position, setPosition] = useState();
+  const [position, setPosition] = useState(null);
+  const hasPositioned = useRef(false);
 
   useEffect(() => {
-    const newPosition = getPositionToFitVisibleScreen(sectionRef);
-    newPosition && setPosition(newPosition);
-  }, [sectionRef]);
+    if (!hasPositioned.current) {
+      const newPosition = getPositionToFitVisibleScreen(
+        sectionRef,
+        path[level] * 30,
+        parentItemCount,
+        items.length
+      );
+      if (newPosition) {
+        setPosition(newPosition);
+        hasPositioned.current = true;
+      }
+    }
+  }, []);
 
   const style = position
     ? {
-      left: position.x,
-      top: position.y + path[level] * 30,
+      transform: `translate(${position.x}px, ${position.y}px)`,
       maxHeight: '80vh',
-      overflowY: 'scroll',
+      overflowY: 'auto',
       opacity: 1,
+      position: 'absolute',
     }
     : {};
 
   return (
     <ul ref={sectionRef} className={styles.category} style={style}>
       {items.map((item, index) => {
-        if (item.items) {
-          return (
-            <li
-              key={`menu-section-${level}-${index}`}
-              className={styles.item}
-              onMouseEnter={() => {
-                const newPath = path.slice(0, level + 1);
-                newPath.push(index);
-                setPath(newPath);
-              }}
-            >
-              {item.text}
-            </li>
-          );
-        }
+        const handleHover = () => {
+          const newPath = path.slice(0, level + 1);
+          newPath.push(index);
+          setPath(newPath);
+        };
+
         return (
           <li
             key={`menu-section-${level}-${index}`}
-            className={styles.option}
+            className={item.items ? styles.item : styles.option}
             style={item.disabled ? { opacity: 0.5, cursor: 'not-allowed' } : {}}
             onClick={() => {
-              if (item.disabled === true) {
-                return;
+              if (!item.disabled) {
+                item.callback?.();
+                hide();
               }
-              item.callback && item.callback();
-              hide();
             }}
+            onMouseEnter={handleHover}
           >
             {item.text}
             {item.subtext && <span> - {item.subtext}</span>}
@@ -92,6 +117,8 @@ const MenuSection = ({ level, items, path, setPath, hide }) => {
 const ContextMenu = ({ x, y, items }) => {
   const [path, setPath] = useState([0]);
   const [visible, setVisible] = useState(true);
+  const menuRef = useRef(null);
+
   useEffect(() => {
     setVisible(true);
   }, [items]);
@@ -101,33 +128,26 @@ const ContextMenu = ({ x, y, items }) => {
     setPath([0]);
   };
 
-  //#region Closing menu after clicking outside it
-
-  const menuRef = useRef(null);
-
-  function handleClickOutside(event) {
-    if (menuRef.current && !menuRef.current.contains(event.target)) {
-      hide();
-    }
-  }
-
   useEffect(() => {
+    const handleClickOutside = event => {
+      if (menuRef.current && !menuRef.current.contains(event.target)) {
+        hide();
+      }
+    };
     document.addEventListener('mousedown', handleClickOutside);
     return () => {
       document.removeEventListener('mousedown', handleClickOutside);
     };
-  });
-
-  //#endregion
+  }, []);
 
   if (!visible) {
     return null;
   }
 
   const getItemsFromLevel = level => {
     let result = items;
-    for (let index = 1; index <= level; index++) {
-      result = result[path[index]].items;
+    for (let i = 1; i <= level; i++) {
+      result = result[path[i]]?.items || [];
     }
     return result;
   };
@@ -136,20 +156,22 @@ const ContextMenu = ({ x, y, items }) => {
     <div
       className={styles.menu}
       ref={menuRef}
-      style={{
-        left: x,
-        top: y,
-      }}
+      style={{ left: x, top: y, position: 'absolute' }}
     >
-      {path.map((position, level) => {
+      {path.map((_, level) => {
+        const itemsForLevel = getItemsFromLevel(level);
+        const parentItemCount =
+          level === 0 ? items.length : getItemsFromLevel(level - 1).length;
+
         return (
           <MenuSection
-            key={`section-${position}-${level}`}
+            key={`section-${path[level]}-${level}`}
             path={path}
             setPath={setPath}
             level={level}
-            items={getItemsFromLevel(level)}
+            items={itemsForLevel}
             hide={hide}
+            parentItemCount={parentItemCount}
           />
         );
       })}
@@ -160,9 +182,7 @@ const ContextMenu = ({ x, y, items }) => {
 ContextMenu.propTypes = {
   x: PropTypes.number.isRequired.describe('X context menu position.'),
   y: PropTypes.number.isRequired.describe('Y context menu position.'),
-  items: PropTypes.array.isRequired.describe(
-    'Array with tree representation of context menu items.'
-  ),
+  items: PropTypes.array.isRequired.describe('Array with tree representation of context menu items.'),
 };
 
 export default ContextMenu;