Disabling activeIndex after a non-keyboard interaction (#512)

* disabling activeIndex after a non-keyboard interaction

* Zemah CR - removed redundant optional chaining on ref

src/hooks/useGridKeyboardNavigation/__tests__/useGridKeyboardNavigation.jest.js

@@ -12,16 +12,6 @@ describe("useGridKeyboardNavigation", () => {
     cleanup();
   });
 
-  it("should set the active index to 0 when focusing for the first time", () => {
-    const { result } = renderHookForTest({});
-
-    act(() => {
-      element.dispatchEvent(new Event("focus"));
-    });
-
-    expect(result.current.activeIndex).toBe(0);
-  });
-
   it("should consider the navigation direction when focusing the element with a custom event", () => {
     const items = itemsArray(9);
     const { result } = renderHookForTest({ items, numberOfItemsInLine: 3 });
@@ -50,14 +40,44 @@ describe("useGridKeyboardNavigation", () => {
     expect(result.current.activeIndex).toBe(5);
   });
 
-  it("should return a callback wrapper that sets the activeIndex to the clicked element", () => {
+  it("should return a callback wrapper that sets the activeIndex to the keyboard selected element, ", () => {
     const { result } = renderHookForTest({});
 
-    act(() => result.current.onSelectionAction(3));
+    act(() => result.current.onSelectionAction(3, true));
 
     expect(result.current.activeIndex).toBe(3);
   });
 
+  it("should select the currently active item when navigating and selecting using the keyboard", () => {
+    const onItemClicked = jest.fn();
+    const items = ["a", "b", "c", "d"];
+    renderHookForTest({ items, focusOnMount: true, focusItemIndexOnMount: 0, onItemClicked });
+
+    act(() => {
+      fireEvent.keyDown(element, { key: "ArrowRight" }); // activeIndex should be set to 1
+    });
+    act(() => {
+      fireEvent.keyDown(element, { key: " " }); // perform selection
+    });
+
+    expect(onItemClicked).toHaveBeenCalledTimes(1);
+    expect(onItemClicked).toHaveBeenCalledWith("b", 1);
+  });
+
+  it("should ignore keyboard selections which are performed after selecting with the mouse", () => {
+    const onItemClicked = jest.fn();
+    const items = ["a", "b", "c", "d"];
+    const { result } = renderHookForTest({ items, focusOnMount: true, focusItemIndexOnMount: 0, onItemClicked });
+    act(() => result.current.onSelectionAction(1)); // select without the keyboard
+    expect(onItemClicked).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      fireEvent.keyDown(element, { key: " " }); // perform selection - which should be ignored
+    });
+
+    expect(onItemClicked).toHaveBeenCalledTimes(1); // no new calls
+  });
+
   it("should return a callback wrapper that calls onItemClicked with the item and the index", () => {
     const onItemClicked = jest.fn();
     const items = ["a", "b", "c", "d"];
@@ -111,6 +131,15 @@ describe("useGridKeyboardNavigation", () => {
     expect(result.current.activeIndex).toBe(2);
   });
 
+  it("should ignore a keyboard selection action if the user is currently not using the keyboard", () => {
+    const { result } = renderHookForTest({ focusItemIndexOnMount: 2, focusOnMount: true });
+
+    act(() => result.current.onSelectionAction(3, true)); // set the activeIndex to 3
+    act(() => result.current.onSelectionAction(2)); // perform a non-keyboard action
+
+    expect(result.current.isInitialActiveState).toBe(false);
+  });
+
   describe("focusItemIndexOnMount", () => {
     it("should set the active index according to focusItemIndexOnMount on mount, when focusOnMount is true", () => {
       const items = ["a", "b", "c", "d"];

src/hooks/useGridKeyboardNavigation/useGridKeyboardNavigation.js

@@ -9,13 +9,6 @@ import useEventListener from "../useEventListener";
 
 const NO_ACTIVE_INDEX = -1;
 
-/**
- * @typedef useGridKeyboardNavigationResult
- * @property {number} activeIndex - the currently active index
- * @property {boolean} isInitialActiveState - if true, the currently active element was due to an initial mounting index option. See "options.focusItemIndexOnMount".
- * @property {function} onSelectionAction - the callback which should be used to select an item. It should be called with the selected item's index. Use this callback for onClick handlers, for example.
- */
-
 /**
  * A hook which is used for accessible keyboard navigation. Useful for components rendering a list of items that can be navigated and selected with a keyboard.
  * @param {Object} options
@@ -28,6 +21,13 @@ const NO_ACTIVE_INDEX = -1;
  * @param {number=} options.focusItemIndexOnMount - optional item index to focus when mounted. Only works with "options.focusOnMount".
  * @param {number[]=} options.disabledIndexes - optional array of disabled indices, which will be skipped while navigating.
  * @returns {useGridKeyboardNavigationResult}
+ *
+ * @typedef useGridKeyboardNavigationResult
+ * @property {number} activeIndex - the currently active index
+ * @property {boolean} isInitialActiveState - if true, the currently active element was due to an initial mounting index option. See "options.focusItemIndexOnMount".
+ * @property {(index: number, isKeyboardAction?: boolean) => void} onSelectionAction - the callback which should be used to select an item.
+ * It should be called with the selected item's index. Use this callback for onClick handlers, for example.
+ * The "isKeyboardAction" can be used to indicate a keyboard selection, which will affect the currently active index.
  */
 export default function useGridKeyboardNavigation({
   ref,
@@ -44,10 +44,12 @@ export default function useGridKeyboardNavigation({
   );
   const skippedInitialActiveIndexChange = useRef(false);
   const [activeIndex, setActiveIndex] = useState(isInitialActiveState ? focusItemIndexOnMount : NO_ACTIVE_INDEX);
+  const [isUsingKeyboardNav, setIsUsingKeyboardNav] = useState(focusOnMount);
 
   const keyboardContext = useContext(GridKeyboardNavigationContext);
 
   const onArrowNavigation = direction => {
+    setIsUsingKeyboardNav(true);
     if (activeIndex === NO_ACTIVE_INDEX) {
       setActiveIndex(0);
       return;
@@ -67,12 +69,6 @@ export default function useGridKeyboardNavigation({
     }
   };
 
-  useEffect(() => {
-    if (activeIndex > -1) {
-      ref?.current?.focus();
-    }
-  }, [activeIndex, ref]);
-
   useEffect(() => {
     if (!skippedInitialActiveIndexChange.current) {
       skippedInitialActiveIndexChange.current = true;
@@ -92,6 +88,7 @@ export default function useGridKeyboardNavigation({
       }
       const direction = e.detail?.keyboardDirection;
       if (direction) {
+        setIsUsingKeyboardNav(true);
         const newIndex = getActiveIndexFromInboundNavigation({ direction, numberOfItemsInLine, itemsCount });
         setActiveIndex(newIndex);
         return;
@@ -108,15 +105,27 @@ export default function useGridKeyboardNavigation({
   useEventListener({ eventName: "focus", callback: onFocus, ref });
   useEventListener({ eventName: "blur", callback: onBlur, ref });
 
+  useEffect(() => {
+    if (activeIndex > -1) {
+      ref.current?.focus();
+    }
+  }, [activeIndex, ref]);
+
   const onSelectionAction = useCallback(
-    index => {
+    (index, isKeyboardAction = false) => {
+      setIsUsingKeyboardNav(isKeyboardAction);
       setActiveIndex(index);
       onItemClicked(getItemByIndex(index), index);
     },
     [setActiveIndex, onItemClicked, getItemByIndex]
   );
 
-  const onKeyboardSelection = useCallback(() => onSelectionAction(activeIndex), [onSelectionAction, activeIndex]);
+  const onKeyboardSelection = useCallback(() => {
+    if (!isUsingKeyboardNav) {
+      return;
+    }
+    return onSelectionAction(activeIndex, true);
+  }, [isUsingKeyboardNav, onSelectionAction, activeIndex]);
 
   useFullKeyboardListeners({
     ref,
@@ -126,8 +135,10 @@ export default function useGridKeyboardNavigation({
     focusOnMount
   });
 
+  // if the user is not using keyboard nav, the consumers should not treat the index as active
+  const externalActiveIndex = isUsingKeyboardNav ? activeIndex : NO_ACTIVE_INDEX;
   return {
-    activeIndex,
+    activeIndex: externalActiveIndex,
     onSelectionAction,
     isInitialActiveState
   };