From 23c0781127ea59aec8652d4ed311f32c282a1c7f Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Tue, 8 Jun 2021 06:43:47 -0700
Subject: [PATCH] Minor keyboard navigation around fix

Summary: Fixed minor keyboard navigation annoyance: pressing arrow down on the last entry would remove selection, then jump to first row. Pressing up on first row would deselect then select first again. After this change the first/last item is kept selected in those cases

Reviewed By: passy

Differential Revision: D28958705

fbshipit-source-id: 01dbce3971ed965eae3b74e6706fef96aa86df66
---
 .../src/ui/data-table/DataTable.tsx           |  4 +--
 .../src/ui/data-table/DataTableManager.tsx    | 28 +++++++++++++++----
 .../__tests__/DataTableManager.node.tsx       | 16 +++++++++++
 3 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx
index cfe740f65..eaf641cf4 100644
--- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx
@@ -188,9 +188,9 @@ export function DataTable<T extends object>(
           if (e.ctrlKey || e.metaKey) {
             tableManager.addRangeToSelection(index, index, true);
           } else if (e.shiftKey) {
-            tableManager.selectItem(index, true);
+            tableManager.selectItem(index, true, true);
           } else {
-            tableManager.selectItem(index);
+            tableManager.selectItem(index, false, true);
           }
 
           dragging.current = true;
diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx
index 8db6be00c..f619952f2 100644
--- a/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx
@@ -59,6 +59,7 @@ type DataManagerActions<T> =
       {
         nextIndex: number | ((currentIndex: number) => number);
         addToSelection?: boolean;
+        allowUnselect?: boolean;
       }
     >
   | Action<
@@ -161,9 +162,14 @@ export const dataTableManagerReducer = produce<
       break;
     }
     case 'selectItem': {
-      const {nextIndex, addToSelection} = action;
+      const {nextIndex, addToSelection, allowUnselect} = action;
       draft.selection = castDraft(
-        computeSetSelection(draft.selection, nextIndex, addToSelection),
+        computeSetSelection(
+          draft.selection,
+          nextIndex,
+          addToSelection,
+          allowUnselect,
+        ),
       );
       break;
     }
@@ -252,6 +258,7 @@ export type DataTableManager<T> = {
   selectItem(
     index: number | ((currentSelection: number) => number),
     addToSelection?: boolean,
+    allowUnselect?: boolean,
   ): void;
   addRangeToSelection(
     start: number,
@@ -276,8 +283,13 @@ export function createDataTableManager<T>(
     reset() {
       dispatch({type: 'reset'});
     },
-    selectItem(index: number, addToSelection = false) {
-      dispatch({type: 'selectItem', nextIndex: index, addToSelection});
+    selectItem(index: number, addToSelection = false, allowUnselect = false) {
+      dispatch({
+        type: 'selectItem',
+        nextIndex: index,
+        addToSelection,
+        allowUnselect,
+      });
     },
     selectItemById(id, addToSelection = false) {
       dispatch({type: 'selectItemById', id, addToSelection});
@@ -517,11 +529,17 @@ export function computeSetSelection(
   base: Selection,
   nextIndex: number | ((currentIndex: number) => number),
   addToSelection?: boolean,
+  allowUnselect?: boolean,
 ): Selection {
   const newIndex =
     typeof nextIndex === 'number' ? nextIndex : nextIndex(base.current);
   // special case: toggle existing selection off
-  if (!addToSelection && base.items.size === 1 && base.current === newIndex) {
+  if (
+    !addToSelection &&
+    allowUnselect &&
+    base.items.size === 1 &&
+    base.current === newIndex
+  ) {
     return emptySelection;
   }
   if (newIndex < 0) {
diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableManager.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableManager.node.tsx
index afe7f1247..c6a6b361c 100644
--- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableManager.node.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableManager.node.tsx
@@ -61,6 +61,20 @@ test('computeSetSelection', () => {
     items: new Set([5]),
   });
 
+  // single item existing selection, no selection toggle
+  expect(
+    computeSetSelection(
+      {
+        current: 4,
+        items: new Set([4]),
+      },
+      4,
+    ),
+  ).toEqual({
+    current: 4,
+    items: new Set([4]),
+  });
+
   // single item existing selection, toggle item off
   expect(
     computeSetSelection(
@@ -69,6 +83,8 @@ test('computeSetSelection', () => {
         items: new Set([4]),
       },
       4,
+      false,
+      true,
     ),
   ).toEqual({
     current: -1,