update control filter toggling implementation

Summary:
The `Control` button press, that hides the current search filter, had some specific logic to make sure that the current selection remained visible. Since this is now generically supported, this is no longer needed.

Also updated the `Control` button behavior to also toggle back the search filter if needed, which was never finished in the original implementation.

Changelog: DataTable: pressing the control key can be used to temporarily turn the current search filter on and off.

Reviewed By: aigoncharov

Differential Revision: D36736494

fbshipit-source-id: 2c4949efa0d6935735f61ee43f9268b7e27d1fcf

desktop/flipper-plugin/src/ui/data-table/DataTable.tsx

@@ -311,7 +311,7 @@ export function DataTable<T extends object>(
           tableManager.clearSelection();
           break;
         case 'Control':
-          tableManager.setSelectedSearchRecord();
+          tableManager.toggleSearchValue();
           break;
         default:
           handled = false;
@@ -505,30 +505,6 @@ export function DataTable<T extends object>(
     // eslint-disable-next-line
   }, []);
 
-  useEffect(
-    function findMappedIndex() {
-      // Hardcoded delay to give dataSource.view time to update, otherwise
-      // the entries we loop over here won't be the list of unfiltered records
-      // the user sees, so there won't be a match found
-      const delay = 300;
-
-      if (tableState.selectedSearchRecord) {
-        const timer = setTimeout(() => {
-          for (let i = 0; i < dataSource.view.size; i++) {
-            if (dataSource.view.get(i) === tableState.selectedSearchRecord) {
-              tableManager.clearSelectedSearchRecord();
-              tableManager.selectItem(i, false, true);
-              break;
-            }
-          }
-        }, delay);
-
-        return () => clearTimeout(timer);
-      }
-    },
-    [dataSource, selection, tableManager, tableState.selectedSearchRecord],
-  );
-
   const header = (
     <Layout.Container>
       {props.enableSearchbar && (

desktop/flipper-plugin/src/ui/data-table/DataTableManager.tsx

@@ -43,7 +43,6 @@ type PersistedState = {
   >[];
   scrollOffset: number;
   autoScroll: boolean;
-  selectedSearchRecord: any;
 };
 
 type Action<Name extends string, Args = {}> = {type: Name} & Args;
@@ -97,8 +96,7 @@ type DataManagerActions<T> =
   | Action<'toggleUseRegex'>
   | Action<'toggleAutoScroll'>
   | Action<'setAutoScroll', {autoScroll: boolean}>
-  | Action<'clearSelectedSearchRecord'>
+  | Action<'toggleSearchValue'>;
-  | Action<'setSelectedSearchRecord'>;
 
 type DataManagerConfig<T> = {
   dataSource: DataSource<T, T[keyof T]>;
@@ -122,7 +120,7 @@ export type DataManagerState<T> = {
   useRegex: boolean;
   autoScroll: boolean;
   /** Used to remember the record entry to lookup when user presses ctrl */
-  selectedSearchRecord: any;
+  previousSearchValue: string;
 };
 
 export type DataTableReducer<T> = Reducer<
@@ -174,6 +172,17 @@ export const dataTableManagerReducer = produce<
     }
     case 'setSearchValue': {
       draft.searchValue = action.value;
+      draft.previousSearchValue = '';
+      break;
+    }
+    case 'toggleSearchValue': {
+      if (draft.searchValue) {
+        draft.previousSearchValue = draft.searchValue;
+        draft.searchValue = '';
+      } else {
+        draft.searchValue = draft.previousSearchValue;
+        draft.previousSearchValue = '';
+      }
       break;
     }
     case 'toggleUseRegex': {
@@ -268,24 +277,6 @@ export const dataTableManagerReducer = produce<
       draft.autoScroll = action.autoScroll;
       break;
     }
-    case 'setSelectedSearchRecord': {
-      if (draft.searchValue !== '') {
-        // Clear search to get unfiltered list of records
-        draft.searchValue = '';
-
-        // Save the record to map back in DataTable
-        draft.selectedSearchRecord = config.dataSource.view.get(
-          draft.selection.current,
-        );
-
-        draft.selection = castDraft(emptySelection);
-      }
-      break;
-    }
-    case 'clearSelectedSearchRecord': {
-      draft.selectedSearchRecord = null;
-      break;
-    }
     default: {
       throw new Error('Unknown action ' + (action as any).type);
     }
@@ -316,8 +307,7 @@ export type DataTableManager<T> = {
   sortColumn(column: keyof T, direction?: SortDirection): void;
   setSearchValue(value: string): void;
   dataSource: DataSource<T, T[keyof T]>;
-  setSelectedSearchRecord(): void;
+  toggleSearchValue(): void;
-  clearSelectedSearchRecord(): void;
 };
 
 export function createDataTableManager<T>(
@@ -364,11 +354,8 @@ export function createDataTableManager<T>(
     setSearchValue(value) {
       dispatch({type: 'setSearchValue', value});
     },
-    setSelectedSearchRecord() {
+    toggleSearchValue() {
-      dispatch({type: 'setSelectedSearchRecord'});
+      dispatch({type: 'toggleSearchValue'});
-    },
-    clearSelectedSearchRecord() {
-      dispatch({type: 'clearSelectedSearchRecord'});
     },
     dataSource,
   };
@@ -411,9 +398,9 @@ export function createInitialState<T>(
         }
       : emptySelection,
     searchValue: prefs?.search ?? '',
+    previousSearchValue: '',
     useRegex: prefs?.useRegex ?? false,
     autoScroll: prefs?.autoScroll ?? config.autoScroll ?? false,
-    selectedSearchRecord: null,
   };
   // @ts-ignore
   res.config[immerable] = false; // optimization: never proxy anything in config
@@ -491,7 +478,6 @@ export function savePreferences(
     })),
     scrollOffset,
     autoScroll: state.autoScroll,
-    selectedSearchRecord: state.selectedSearchRecord,
   };
   localStorage.setItem(state.storageKey, JSON.stringify(prefs));
 }