From 61fab45f82761dcbdfc4aa2ff297f97cb3b672fd Mon Sep 17 00:00:00 2001
From: Omri Lavi <omrila@monday.com>
Date: Wed, 26 Jan 2022 17:25:02 +0200
Subject: [PATCH] Skipping disabled components while navigating (#488)

* implemented

* updated story
---
 .../GridKeyboardNavigationContext.js          |   5 +-
 ...seGridKeyboardNavigationContext.stories.js |  11 +-
 ...eGridKeyboardNavigationContext.stories.mdx | 119 +++++++++++++-----
 ...GridKeyboardNavigationContext.stories.scss |  22 ++--
 .../__tests__/helper.jest.js                  |  69 +++++++++-
 .../GridKeyboardNavigationContext/helper.js   |  28 ++---
 6 files changed, 192 insertions(+), 62 deletions(-)

diff --git a/src/components/GridKeyboardNavigationContext/GridKeyboardNavigationContext.js b/src/components/GridKeyboardNavigationContext/GridKeyboardNavigationContext.js
index ad43be448d..009e19c680 100644
--- a/src/components/GridKeyboardNavigationContext/GridKeyboardNavigationContext.js
+++ b/src/components/GridKeyboardNavigationContext/GridKeyboardNavigationContext.js
@@ -3,6 +3,7 @@ import useEventListener from "../../hooks/useEventListener";
 import {
   focusElementWithDirection,
   getDirectionMaps,
+  getNextElementToFocusInDirection,
   getOppositeDirection,
   getOutmostElementInDirection
 } from "./helper";
@@ -34,8 +35,8 @@ export const useGridKeyboardNavigationContext = (positions, wrapperRef) => {
 
   const onOutboundNavigation = useCallback(
     (elementRef, direction) => {
-      const maybeNextElement = directionMaps[direction].get(elementRef);
-      if (maybeNextElement?.current) {
+      const maybeNextElement = getNextElementToFocusInDirection(directionMaps[direction], elementRef);
+      if (maybeNextElement) {
         elementRef.current?.blur();
         focusElementWithDirection(maybeNextElement, direction);
         return;
diff --git a/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.js b/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.js
index 4377f90acd..2bbaf9a5ee 100644
--- a/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.js
+++ b/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.js
@@ -12,7 +12,7 @@ const PADDING_PX = 24;
 
 const ON_CLICK = action("Selected");
 
-export const DummyNavigableGrid = forwardRef(({ itemsCount, numberOfItemsInLine, itemPrefix }, ref) => {
+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]);
@@ -25,12 +25,19 @@ export const DummyNavigableGrid = forwardRef(({ itemsCount, numberOfItemsInLine,
   });
   const onClickByIndex = useCallback(index => () => onSelectionAction(index), [onSelectionAction]);
   return (
-    <div className="use-grid-keyboard-dummy-grid-wrapper" style={{ width }} ref={ref} tabIndex={-1}>
+    <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}
diff --git a/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.mdx b/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.mdx
index a4dd21e391..3b4c5ffbb8 100644
--- a/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.mdx
+++ b/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.mdx
@@ -9,12 +9,12 @@ export const metaSettings = createStoryMetaSettings({
   component: useGridKeyboardNavigationContext
 });
 
-<Meta 
-  title="Hooks/useGridKeyboardNavigationContext" 
-  component={ useGridKeyboardNavigationContext } 
-  argTypes={ metaSettings.argTypes }
-  decorators={ metaSettings.decorators } 
-  />
+<Meta
+  title="Hooks/useGridKeyboardNavigationContext"
+  component={useGridKeyboardNavigationContext}
+  argTypes={metaSettings.argTypes}
+  decorators={metaSettings.decorators}
+/>
 
 <!--- Component documentation -->
 
@@ -31,21 +31,25 @@ export const metaSettings = createStoryMetaSettings({
 A hook used to specify a connection between multiple navigable components, which are navigable between each other.
 
 <Canvas>
-  <Story name="Overview">{
-    () => {
+  <Story name="Overview">
+    {() => {
       const wrapperRef = useRef(null);
       const leftElRef = useRef(null);
       const rightElRef = useRef(null);
-      const keyboardContext = useGridKeyboardNavigationContext([{leftElement: leftElRef, rightElement: rightElRef}], wrapperRef)
+      const keyboardContext = useGridKeyboardNavigationContext(
+        [{ leftElement: leftElRef, rightElement: rightElRef }],
+        wrapperRef
+      );
       return (
         <GridKeyboardNavigationContext.Provider value={keyboardContext}>
           <Flex ref={wrapperRef}>
-            <DummyNavigableGrid ref={leftElRef} itemsCount={15} numberOfItemsInLine={4} itemPrefix="L "/>
-            <DummyNavigableGrid ref={rightElRef} itemsCount={7} numberOfItemsInLine={2} itemPrefix="R "/>
+            <DummyNavigableGrid ref={leftElRef} itemsCount={15} numberOfItemsInLine={4} itemPrefix="L " />
+            <DummyNavigableGrid ref={rightElRef} itemsCount={7} numberOfItemsInLine={2} itemPrefix="R " />
           </Flex>
         </GridKeyboardNavigationContext.Provider>
-      )
-  }}</Story>
+      );
+    }}
+  </Story>
 </Canvas>
 
 ## Usage
@@ -58,19 +62,24 @@ A hook used to specify a connection between multiple navigable components, which
   ]}
 />
 
-
 ## Arguments
+
 <FunctionArguments>
-  <FunctionArgument 
-    name="positions" 
+  <FunctionArgument
+    name="positions"
     type="Array[ { topElement: React.MutableRefObject, bottomElement: React.MutableRefObject } | { leftElement: React.MutableRefObject, rightElement: React.MutableRefObject } ]"
     description="An array of relations between pairs of elements"
     required
   />
-  <FunctionArgument 
-    name="wrapperRef" 
+  <FunctionArgument
+    name="wrapperRef"
     type="React.MutableRefObject"
-    description={<>A React ref for an element which contains all the elements which are listed on the <code>positions</code> argument.</>}
+    description={
+      <>
+        A React ref for an element which contains all the elements which are listed on the <code>positions</code>{" "}
+        argument.
+      </>
+    }
     required
   />
 </FunctionArguments>
@@ -78,32 +87,78 @@ A hook used to specify a connection between multiple navigable components, which
 ## Returns
 
 <FunctionArguments>
-  <FunctionArgument 
-    name="result" 
+  <FunctionArgument
+    name="result"
     type="Object"
-    description={<>A <code>GridKeyboardNavigationContext</code> which should be provided to wrap all the elements from <code>positions</code></>}
+    description={
+      <>
+        A <code>GridKeyboardNavigationContext</code> which should be provided to wrap all the elements from{" "}
+        <code>positions</code>
+      </>
+    }
   />
 </FunctionArguments>
 
 ## Variants
 
+### Disabled Elements
+
+Disabled components can be skipped by adding a `disabled` (or `data-disabled`) to the referenced element.
+
+<Canvas>
+  <Story name="Disabled Elements">
+    {() => {
+      const wrapperRef = useRef(null);
+      const topElRef = useRef(null);
+      const middleElRef = useRef(null);
+      const bottomElRef = useRef(null);
+      const keyboardContext = useGridKeyboardNavigationContext(
+        [
+          { topElement: topElRef, bottomElement: middleElRef },
+          { topElement: middleElRef, bottomElement: bottomElRef }
+        ],
+        wrapperRef
+      );
+      return (
+        <GridKeyboardNavigationContext.Provider value={keyboardContext}>
+          <Flex
+            ref={wrapperRef}
+            direction={Flex.directions.COLUMN}
+            justify={Flex.align.CENTER}
+            className="use-grid-keyboard-dummy-grid-wrapper"
+          >
+            <DummyNavigableGrid ref={topElRef} itemsCount={3} numberOfItemsInLine={3} itemPrefix="T " />
+            <DummyNavigableGrid ref={middleElRef} itemsCount={3} numberOfItemsInLine={3} itemPrefix="M " disabled />
+            <DummyNavigableGrid ref={bottomElRef} itemsCount={3} numberOfItemsInLine={3} itemPrefix="B " />
+          </Flex>
+        </GridKeyboardNavigationContext.Provider>
+      );
+    }}
+  </Story>
+</Canvas>
+
 ### Multiple Depths
+
 The hook can be used inside multiple depths, in more complex layout requirements.
 
 <Canvas>
-  <Story name="Multiple Depths">{
-    () => {
+  <Story name="Multiple Depths">
+    {() => {
       const wrapperRef = useRef(null);
       const topElRef = useRef(null);
       const bottomElRef = useRef(null);
-      const keyboardContext = useGridKeyboardNavigationContext([{topElement: topElRef, bottomElement: bottomElRef}], wrapperRef)
+      const keyboardContext = useGridKeyboardNavigationContext(
+        [{ topElement: topElRef, bottomElement: bottomElRef }],
+        wrapperRef
+      );
       return (
-      <GridKeyboardNavigationContext.Provider value={keyboardContext}>
-        <Flex ref={wrapperRef} direction={Flex.directions.COLUMN}>
-          <LayoutWithInnerKeyboardNavigation ref={topElRef} />
-          <LayoutWithInnerKeyboardNavigation ref={bottomElRef} />
-        </Flex>
-      </GridKeyboardNavigationContext.Provider>
+        <GridKeyboardNavigationContext.Provider value={keyboardContext}>
+          <Flex ref={wrapperRef} direction={Flex.directions.COLUMN}>
+            <LayoutWithInnerKeyboardNavigation ref={topElRef} />
+            <LayoutWithInnerKeyboardNavigation ref={bottomElRef} />
+          </Flex>
+        </GridKeyboardNavigationContext.Provider>
       );
-  }}</Story>
+    }}
+  </Story>
 </Canvas>
diff --git a/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.scss b/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.scss
index 663ffd8c45..93f8a8cee7 100644
--- a/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.scss
+++ b/src/components/GridKeyboardNavigationContext/__stories__/useGridKeyboardNavigationContext.stories.scss
@@ -3,19 +3,19 @@
 @import "../../../styles/global-css-settings.scss";
 
 .use-grid-keyboard-dummy-grid-wrapper {
-    padding: 12px;
-    display: flex;
-    flex-wrap: wrap;
-    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;
+  padding: 12px;
+  display: flex;
+  flex-wrap: wrap;
+  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;
 }
 
 .use-grid-keyboard-dummy-grid-item {
-    width: 60px;
-    margin: $spacing-xs-small;
+  width: 60px;
+  margin: $spacing-xs-small;
 
-    &.active-item {
-        @include focus-style-css();
-    }
+  &.active-item {
+    @include focus-style-css();
+  }
 }
diff --git a/src/components/GridKeyboardNavigationContext/__tests__/helper.jest.js b/src/components/GridKeyboardNavigationContext/__tests__/helper.jest.js
index 8697bf6c7e..a5172dfc62 100644
--- a/src/components/GridKeyboardNavigationContext/__tests__/helper.jest.js
+++ b/src/components/GridKeyboardNavigationContext/__tests__/helper.jest.js
@@ -3,7 +3,8 @@ import {
   getOppositeDirection,
   getDirectionMaps,
   focusElementWithDirection,
-  getOutmostElementInDirection
+  getOutmostElementInDirection,
+  getNextElementToFocusInDirection
 } from "../helper";
 
 describe("GridKeyboardNavigationContext.helper", () => {
@@ -14,6 +15,9 @@ describe("GridKeyboardNavigationContext.helper", () => {
   const ELEMENT5 = { current: "e5" };
   const UNMOUNTED_ELEMENT_1 = { current: null };
   const UNMOUNTED_ELEMENT_2 = { current: null };
+  const DISABLED_ELEMENT = { current: { disabled: true } };
+  const DISABLED_DATASET_ELEMENT = { current: { dataset: { disabled: "true" } } };
+  const DATASET_NOT_DISABLED_ELEMENT = { current: { dataset: { disabled: "false" } } };
 
   describe("getDirectionMaps", () => {
     it("should return empty direction maps when no positions are supplied", () => {
@@ -283,4 +287,67 @@ describe("GridKeyboardNavigationContext.helper", () => {
       expect(result).toEqual(expected);
     });
   });
+
+  describe("getNextElementToFocusInDirection", () => {
+    it("should return null if the referenced element isn't positioned on that direction", () => {
+      const directionMaps = getDirectionMaps([
+        { leftElement: ELEMENT2, rightElement: ELEMENT4 },
+        { leftElement: ELEMENT4, rightElement: ELEMENT5 }
+      ]);
+      const directionMap = directionMaps[NAV_DIRECTIONS.RIGHT];
+
+      const result = getNextElementToFocusInDirection(directionMap, ELEMENT1); // ELEMENT1 isn't mapped
+
+      expect(result).toBeNull();
+    });
+
+    it("return null if there's only one next ref, and it is currently null", () => {
+      const directionMaps = getDirectionMaps([{ leftElement: ELEMENT1, rightElement: UNMOUNTED_ELEMENT_1 }]);
+      const directionMap = directionMaps[NAV_DIRECTIONS.RIGHT];
+
+      const result = getNextElementToFocusInDirection(directionMap, ELEMENT1);
+
+      expect(result).toBeNull();
+    });
+
+    it("return null if there's only one next ref, and it is disabled", () => {
+      const directionMaps = getDirectionMaps([{ leftElement: ELEMENT1, rightElement: DISABLED_ELEMENT }]);
+      const directionMap = directionMaps[NAV_DIRECTIONS.RIGHT];
+
+      const result = getNextElementToFocusInDirection(directionMap, ELEMENT1);
+
+      expect(result).toBeNull();
+    });
+
+    it("return null if there's only one next ref, and it is disabled with data-disabled='true'", () => {
+      const directionMaps = getDirectionMaps([{ leftElement: ELEMENT1, rightElement: DISABLED_DATASET_ELEMENT }]);
+      const directionMap = directionMaps[NAV_DIRECTIONS.RIGHT];
+
+      const result = getNextElementToFocusInDirection(directionMap, ELEMENT1);
+
+      expect(result).toBeNull();
+    });
+
+    it("return the next element ref even if it has data-disabled='false'", () => {
+      const directionMaps = getDirectionMaps([{ leftElement: ELEMENT1, rightElement: DATASET_NOT_DISABLED_ELEMENT }]);
+      const directionMap = directionMaps[NAV_DIRECTIONS.RIGHT];
+
+      const result = getNextElementToFocusInDirection(directionMap, ELEMENT1);
+
+      expect(result).toBe(DATASET_NOT_DISABLED_ELEMENT);
+    });
+
+    it("return the next element ref in the given direction which is focusable, while skipping disabled or unmounted elements", () => {
+      const directionMaps = getDirectionMaps([
+        { leftElement: ELEMENT1, rightElement: UNMOUNTED_ELEMENT_1 },
+        { leftElement: UNMOUNTED_ELEMENT_1, rightElement: DISABLED_ELEMENT },
+        { leftElement: DISABLED_ELEMENT, rightElement: ELEMENT2 }
+      ]);
+      const directionMap = directionMaps[NAV_DIRECTIONS.RIGHT];
+
+      const result = getNextElementToFocusInDirection(directionMap, ELEMENT1);
+
+      expect(result).toBe(ELEMENT2);
+    });
+  });
 });
diff --git a/src/components/GridKeyboardNavigationContext/helper.js b/src/components/GridKeyboardNavigationContext/helper.js
index 925a5f3029..d7549582e0 100644
--- a/src/components/GridKeyboardNavigationContext/helper.js
+++ b/src/components/GridKeyboardNavigationContext/helper.js
@@ -83,13 +83,26 @@ export const getOutmostElementInDirection = (directionMaps, direction) => {
   return getLastFocusableElementFromElementInDirection(directionMap, firstRef);
 };
 
+export const getNextElementToFocusInDirection = (directionMap, elementRef) => {
+  const next = directionMap.get(elementRef);
+  if (!next) {
+    // this is the last element on the direction map - there' nothing next
+    return null;
+  }
+  if (!next.current || next.current.disabled || next.current.dataset?.disabled === "true") {
+    // the next element is not mounted or disabled - try the next one
+    return getNextElementToFocusInDirection(directionMap, next);
+  }
+  return next;
+};
+
 function getLastFocusableElementFromElementInDirection(directionMap, initialRef) {
   let done = false;
   let currentRef = initialRef;
 
   while (!done) {
     // as long as there's a mounted element which in that direction, take it.
-    const nextEligible = getNextEligibleRef(currentRef);
+    const nextEligible = getNextElementToFocusInDirection(directionMap, currentRef);
     if (!nextEligible) {
       done = true;
     } else {
@@ -97,18 +110,5 @@ function getLastFocusableElementFromElementInDirection(directionMap, initialRef)
     }
   }
 
-  function getNextEligibleRef(_currentRef) {
-    const next = directionMap.get(_currentRef);
-    if (!next) {
-      // this is the last element on the direction map - there' nothing next
-      return null;
-    }
-    if (!next.current) {
-      // the next element is not mounted, try the next one
-      return getNextEligibleRef(next);
-    }
-    return next;
-  }
-
   return currentRef;
 }