Menu grid item (#506)

* focusing MenuItem when isActive changes to true * prevent changes to the activeIndex if the element is already focused * useLastNavigationDirection - using a ref instead of state to reduce unnecessary rerenders * Added MenuGridItem * fixed menu item locking focus sometimes when hovering without children
mondaycom · Feb 15, 2022 · ba23d10 · ba23d10
1 parent 8bc0a0f
commit ba23d10
Show file tree

Hide file tree

Showing 27 changed files with 945 additions and 84 deletions.
diff --git a/...nts/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.js b/...nts/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.js
@@ -12,40 +12,43 @@ const PADDING_PX = 24;
 
 const ON_CLICK = action("Selected");
 
-export const DummyNavigableGrid = forwardRef(({ itemsCount, numberOfItemsInLine, itemPrefix, disabled }, ref) => {
-  const width = useMemo(() => numberOfItemsInLine * ELEMENT_WIDTH_PX + 2 * PADDING_PX, [numberOfItemsInLine]);
-  const items = useMemo(() => range(itemsCount).map(num => `${itemPrefix} ${num}`), [itemPrefix, itemsCount]);
-  const getItemByIndex = useCallback(index => items[index], [items]);
-  const { activeIndex, onSelectionAction } = useGridKeyboardNavigation({
-    ref,
-    numberOfItemsInLine,
-    itemsCount,
-    getItemByIndex,
-    onItemClicked: ON_CLICK
-  });
-  const onClickByIndex = useCallback(index => () => onSelectionAction(index), [onSelectionAction]);
-  return (
-    <div
-      className="use-grid-keyboard-dummy-grid-wrapper"
-      style={{ width }}
-      data-disabled={disabled}
-      ref={ref}
-      tabIndex={-1}
-    >
-      {items.map((item, index) => (
-        <Button
-          key={item}
-          kind={Button.kinds.SECONDARY}
-          onClick={onClickByIndex(index)}
-          disabled={disabled}
-          className={cx("use-grid-keyboard-dummy-grid-item", { "active-item": index === activeIndex })}
-        >
-          {item}
-        </Button>
-      ))}
-    </div>
-  );
-});
+export const DummyNavigableGrid = forwardRef(
+  ({ itemsCount, numberOfItemsInLine, itemPrefix = "", disabled, disabledIndexes, withoutBorder = false }, ref) => {
+    const width = useMemo(() => numberOfItemsInLine * ELEMENT_WIDTH_PX + 2 * PADDING_PX, [numberOfItemsInLine]);
+    const items = useMemo(() => range(itemsCount).map(num => `${itemPrefix} ${num}`), [itemPrefix, itemsCount]);
+    const getItemByIndex = useCallback(index => items[index], [items]);
+    const { activeIndex, onSelectionAction } = useGridKeyboardNavigation({
+      ref,
+      numberOfItemsInLine,
+      itemsCount,
+      getItemByIndex,
+      onItemClicked: ON_CLICK,
+      disabledIndexes
+    });
+    const onClickByIndex = useCallback(index => () => onSelectionAction(index), [onSelectionAction]);
+    return (
+      <div
+        className={cx("use-grid-keyboard-dummy-grid-wrapper", { "without-border": withoutBorder })}
+        style={{ width }}
+        data-disabled={disabled}
+        ref={ref}
+        tabIndex={-1}
+      >
+        {items.map((item, index) => (
+          <Button
+            key={item}
+            kind={Button.kinds.SECONDARY}
+            onClick={onClickByIndex(index)}
+            disabled={disabled || disabledIndexes?.includes(index)}
+            className={cx("use-grid-keyboard-dummy-grid-item", { "active-item": index === activeIndex })}
+          >
+            {item}
+          </Button>
+        ))}
+      </div>
+    );
+  }
+);
 
 export const LayoutWithInnerKeyboardNavigation = forwardRef((_ignored, ref) => {
   const leftElRef = useRef(null);

diff --git a/...s/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.scss b/...s/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.scss
@@ -9,6 +9,10 @@
   outline: none;
   text-align: center;
   box-shadow: 0 0 0 1px rgba(0, 0, 0, 0.1), 0 0 0 1px var(--primary-hover-color) inset;
+
+  &.without-border {
+    box-shadow: none;
+  }
 }
 
 .use-grid-keyboard-dummy-grid-item {

diff --git a/src/components/Menu/Menu/Menu.jsx b/src/components/Menu/Menu/Menu.jsx
@@ -12,6 +12,7 @@ import useCloseMenuOnKeyEvent from "./hooks/useCloseMenuOnKeyEvent";
 import useMenuKeyboardNavigation from "./hooks/useMenuKeyboardNavigation";
 import useMouseLeave from "./hooks/useMouseLeave";
 import "./Menu.scss";
+import { useAdjacentSelectableMenuIndex } from "./hooks/useAdjacentSelectableMenuIndex";
 
 const Menu = forwardRef(
   (
@@ -76,9 +77,12 @@ const Menu = forwardRef(
 
     useClickOutside({ ref, callback: onCloseMenu });
     useCloseMenuOnKeyEvent(hasOpenSubMenu, onCloseMenu, ref, onClose, isSubMenu, useDocumentEventListeners);
+
+    const { getNextSelectableIndex, getPreviousSelectableIndex } = useAdjacentSelectableMenuIndex({ children });
     useMenuKeyboardNavigation(
       hasOpenSubMenu,
-      children,
+      getNextSelectableIndex,
+      getPreviousSelectableIndex,
       activeItemIndex,
       onSetActiveItemIndexCallback,
       isVisible,
@@ -91,12 +95,10 @@ const Menu = forwardRef(
       setIsInitialSelectedState(true);
     }, [setIsInitialSelectedState]);
 
-    useLayoutEffect(() => {
+    useEffect(() => {
       if (hasOpenSubMenu || useDocumentEventListeners) return;
       if (activeItemIndex > -1) {
-        requestAnimationFrame(() => {
-          ref && ref.current && ref.current.focus();
-        });
+        ref?.current?.focus();
       }
     }, [activeItemIndex, hasOpenSubMenu, useDocumentEventListeners]);
 
@@ -147,7 +149,10 @@ const Menu = forwardRef(
                   menuId: id,
                   useDocumentEventListeners,
                   isInitialSelectedState,
-                  shouldScrollMenu
+                  shouldScrollMenu,
+                  getNextSelectableIndex,
+                  getPreviousSelectableIndex,
+                  isUnderSubMenu: isSubMenu
                 })
               : null;
           })}

diff --git a/src/components/Menu/Menu/__stories__/Menu.stories.mdx b/src/components/Menu/Menu/__stories__/Menu.stories.mdx
@@ -7,13 +7,17 @@ import {
   menuWithIconsTemplate,
   ComponentRuleSimpleActions,
   ComponentRuleWithSearch,
-  ComponentRuleDefaultWidth, ComponentRuleLargeWidth, menuSizesTemplate
+  ComponentRuleDefaultWidth, 
+  ComponentRuleLargeWidth, 
+  menuSizesTemplate,
+  menuWithGridItems
 } from "./menu.stories";
 import {
   COMBOBOX,
   DROPDOWN,
   SPLIT_BUTTON
 } from "../../../../storybook/components/related-components/component-description-map";
+import classes from "./Menu.stories.module.scss";
 
 <Meta
   title="Navigation/Menu/Menu"
@@ -118,5 +122,14 @@ A menu is a navigatable contextual list of items that can be selected.
   </Story>
 </Canvas>
 
+### Menu with grid items and sub menu
+Grid menu items are navigable with a keyboard as well
+
+<Canvas>
+  <Story name="Menu with grid items and sub menu">
+    {menuWithGridItems.bind({})}
+  </Story>
+</Canvas>
+
 ## Related components
 <RelatedComponents componentsNames={[COMBOBOX, DROPDOWN, SPLIT_BUTTON]} />
diff --git a/src/components/Menu/Menu/__stories__/Menu.stories.module.scss b/src/components/Menu/Menu/__stories__/Menu.stories.module.scss
@@ -9,4 +9,8 @@
   &-large-menu {
     width: 328px;
   }
-}
+}
+
+.menu-long-story-wrapper {
+  height: 500px;
+}
diff --git a/src/components/Menu/Menu/__stories__/menu.stories.js b/src/components/Menu/Menu/__stories__/menu.stories.js
@@ -22,6 +22,8 @@ import Search from "components/Search/Search";
 import MenuTitle from "components/Menu/MenuTitle/MenuTitle";
 import MenuDivider from "components/Menu/MenuDivider/MenuDivider";
 import classes from "./Menu.stories.module.scss";
+import { DummyNavigableGrid } from "../../../GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories";
+import MenuGridItem from "components/Menu/MenuGridItem/MenuGridItem";
 
 export const menuTemplate = args => (
   <Menu {...args}>
@@ -109,6 +111,37 @@ export const menuWith2DepthSubMenuTemplate = args => (
   </DialogContentContainer>
 );
 
+export const menuWithGridItems = args => (
+  <div className={classes["menu-long-story-wrapper"]}>
+    <DialogContentContainer>
+      <Menu>
+        <MenuItem title="Menu item" icon={Favorite} />
+        <MenuTitle caption="Top level grid item" />
+        <MenuItem title="Hover me to see the sub menu" icon={Activity}>
+          <Menu>
+            <MenuItem icon={Feedback} title="More info" />
+            <MenuTitle caption="1st level grid item" />
+            <MenuGridItem>
+              <DummyNavigableGrid itemsCount={6} numberOfItemsInLine={3} withoutBorder />
+            </MenuGridItem>
+            <MenuItem icon={Code} title="Hover me to see the sub menu">
+              <Menu>
+                <MenuTitle caption="2nd level grid item" />
+                <MenuGridItem>
+                  <DummyNavigableGrid itemsCount={6} numberOfItemsInLine={3} withoutBorder />
+                </MenuGridItem>
+                <MenuItem icon={Invite} title="Another sub sub item" />
+                <MenuItem icon={Settings} title="More sub sub items" />
+              </Menu>
+            </MenuItem>
+          </Menu>
+        </MenuItem>
+        <MenuItem title="Another item" icon={Settings} />
+      </Menu>
+    </DialogContentContainer>
+  </div>
+);
+
 export const ComponentRuleSimpleActions = () => (
   <DialogContentContainer>
     <Menu>

diff --git a/src/components/Menu/Menu/hooks/__tests__/useAdjacentSelectableMenuIndex.jest.js b/src/components/Menu/Menu/hooks/__tests__/useAdjacentSelectableMenuIndex.jest.js
@@ -0,0 +1,126 @@
+import { renderHook } from "@testing-library/react-hooks";
+import { useAdjacentSelectableMenuIndex } from "../useAdjacentSelectableMenuIndex";
+
+describe("useAdjacentSelectableMenuIndex", () => {
+  const ENABLED_SELECTABLE_CHILD = { type: { isSelectable: true }, props: {} };
+  const DISABLED_SELECTABLE_CHILD = { type: { isSelectable: true }, props: { disabled: true } };
+  const ENABLED_NON_SELECTABLE_CHILD = { type: {}, props: {} };
+
+  describe("getNextSelectableIndex", () => {
+    it("should return the same index if there is only one child", () => {
+      const children = [ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 0;
+      const expectedResult = 0;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getNextSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the third index if there are three items and the second index is selected", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 1;
+      const expectedResult = 2;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getNextSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the first index if there are three children and the third index is selected", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 2;
+      const expectedResult = 0;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getNextSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the third index if there are three children, the first one is active and the second one is disabled", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, DISABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 0;
+      const expectedResult = 2;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getNextSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the first index if there are three children, the second is active and the third one is no selectable", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD, ENABLED_NON_SELECTABLE_CHILD];
+      const currentIndex = 1;
+      const expectedResult = 0;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getNextSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+  });
+
+  describe("getPreviousSelectableIndex", () => {
+    it("should return the null if there is only one child", () => {
+      const children = [ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 0;
+      const expectedResult = null;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getPreviousSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the second index if there are three items and the third index is selected", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 2;
+      const expectedResult = 1;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getPreviousSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the third index if there are three children and the first index is selected", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 0;
+      const expectedResult = 2;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getPreviousSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the first index if there are three children, the third one is active and the second one is disabled", () => {
+      const children = [ENABLED_SELECTABLE_CHILD, DISABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 2;
+      const expectedResult = 0;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getPreviousSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+
+    it("should return the third index if there are three children, the second is active and the first one is no selectable", () => {
+      const children = [ENABLED_NON_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD, ENABLED_SELECTABLE_CHILD];
+      const currentIndex = 1;
+      const expectedResult = 2;
+
+      const { result: hookResult } = renderHookForTest(children);
+      const result = hookResult.current.getPreviousSelectableIndex(currentIndex);
+
+      expect(result).toEqual(expectedResult);
+    });
+  });
+
+  function renderHookForTest(children) {
+    return renderHook(() => useAdjacentSelectableMenuIndex({ children }));
+  }
+});