Use DataList

Summary:
With new abstraction, `DataList` matches what the plugin trying to render.

Should fix:
https://fb.workplace.com/groups/flippersupport/permalink/1145431339270856/

Changelog: [MobileConfig] Fix issues with scrolling not working and several other improvements

Reviewed By: cekkaewnumchai

Differential Revision: D28314408

fbshipit-source-id: 4d8fbe3d8e868f737750203cd568d94bae8b4108

desktop/flipper-plugin/src/ui/DataList.tsx

@@ -7,14 +7,7 @@
  * @format
  */
 
-import React, {
-  useCallback,
-  memo,
-  createRef,
-  useEffect,
-  useMemo,
-  useState,
-} from 'react';
+import React, {memo, createRef, useMemo, useEffect, useCallback} from 'react';
 import {DataFormatter} from './DataFormatter';
 import {Layout} from './Layout';
 import {Typography} from 'antd';
@@ -28,19 +21,13 @@ import {RightOutlined} from '@ant-design/icons';
 import {theme} from './theme';
 import styled from '@emotion/styled';
 import {DataTableManager} from './data-table/DataTableManager';
-import {Atom, createState} from '../state/atom';
 import {useAssertStableRef} from '../utils/useAssertStableRef';
 import {DataSource} from '../data-source';
+import {useMakeStableCallback} from '../utils/useMakeStableCallback';
 
 const {Text} = Typography;
 
-interface Item {
-  id: string;
-  title: string;
-  description?: string;
-}
-
-interface DataListBaseProps<T extends Item> {
+type DataListBaseProps<Item> = {
   /**
    * Defines the styling of the component. By default shows a list, but alternatively the items can be displayed in a drop down
    */
@@ -50,14 +37,11 @@ interface DataListBaseProps<T extends Item> {
    * By setting `scrollable={false}` the list will only take its natural size
    */
   scrollable?: boolean;
-  /**
-   * The current selection
-   */
-  selection: Atom<string | undefined>;
   /**
    * Handler that is fired if selection is changed
    */
-  onSelect?(id: string | undefined, value: T | undefined): void;
+  selection?: string | undefined;
+  onSelect?(id: string | undefined, value: Item | undefined): void;
   className?: string;
   style?: React.CSSProperties;
   /**
@@ -67,85 +51,102 @@ interface DataListBaseProps<T extends Item> {
   /**
    * Custom render function. By default the component will render the `title` in bold and description (if any) below it
    */
-  onRenderItem?: ItemRenderer<T>;
+  onRenderItem?: ItemRenderer<Item>;
+  /**
+   * The attributes that will be picked as id/title/description for the default rendering.
+   * Defaults to id/title/description, but can be customized
+   */
+
+  titleAttribute?: keyof Item & string;
+  descriptionAttribute?: keyof Item & string;
   /**
    * Show a right arrow by default
    */
   enableArrow?: boolean;
+} & (Item extends {id: string}
+  ? {
+      idAttribute?: keyof Item & string; // optional if id field is present
     }
+  : {
+      idAttribute: keyof Item & string;
+    });
 
-export type DataListProps<T extends Item> = DataListBaseProps<T> &
+export type DataListProps<Item> = DataListBaseProps<Item> &
   // Some table props are set by DataList instead, so override them
-  Omit<DataTableProps<T>, 'records' | 'dataSource' | 'columns' | 'onSelect'>;
+  Omit<DataTableProps<Item>, 'records' | 'dataSource' | 'columns' | 'onSelect'>;
 
-export const DataList: React.FC<DataListProps<any>> = function DataList<
-  T extends Item,
->({
-  selection: baseSelection,
-  onSelect,
+export const DataList: (<T>(props: DataListProps<T>) => React.ReactElement) & {
+  Item: React.FC<DataListItemProps>;
+} = Object.assign(
+  function <T>({
+    onSelect: baseOnSelect,
+    selection,
     className,
     style,
     items,
     onRenderItem,
     enableArrow,
+    idAttribute,
+    titleAttribute,
+    descriptionAttribute,
     ...tableProps
   }: DataListProps<T>) {
     // if a tableManagerRef is provided, we piggy back on that same ref
     // eslint-disable-next-line
     const tableManagerRef = tableProps.tableManagerRef ?? createRef<undefined | DataTableManager<T>>();
 
-  useAssertStableRef(baseSelection, 'selection');
     useAssertStableRef(onRenderItem, 'onRenderItem');
     useAssertStableRef(enableArrow, 'enableArrow');
-
-  // create local selection atom if none provided
-  // eslint-disable-next-line
-  const selection = baseSelection ?? useState(() => createState<string|undefined>())[0];
+    const onSelect = useMakeStableCallback(baseOnSelect);
 
     const handleSelect = useCallback(
       (item: T | undefined) => {
-      selection.set(item?.id);
+        if (!item) {
+          onSelect?.(undefined, undefined);
+        } else {
+          const id = '' + item[idAttribute!];
+          if (id == null) {
+            throw new Error(`No valid identifier for attribute ${idAttribute}`);
+          }
+          onSelect?.(id, item);
+        }
       },
-    [selection],
+      [onSelect, idAttribute],
     );
 
+    useEffect(() => {
+      if (selection) {
+        tableManagerRef.current?.selectItemById(selection);
+      } else {
+        tableManagerRef.current?.clearSelection();
+      }
+    }, [selection, tableManagerRef]);
+
     const dataListColumns: DataTableColumn<T>[] = useMemo(
       () => [
         {
-        key: 'id' as const,
+          key: idAttribute!,
           wrap: true,
           onRender(item: T, selected: boolean, index: number) {
             return onRenderItem ? (
               onRenderItem(item, selected, index)
             ) : (
-            <DataListItem
-              title={item.title}
-              description={item.description}
+              <DataList.Item
+                title={item[titleAttribute!] as any}
+                description={item[descriptionAttribute!] as any}
                 enableArrow={enableArrow}
               />
             );
           },
         },
       ],
-    [onRenderItem, enableArrow],
-  );
-
-  useEffect(
-    function updateSelection() {
-      return selection.subscribe((valueFromAtom) => {
-        const m = tableManagerRef.current;
-        if (!m) {
-          return;
-        }
-        if (!valueFromAtom && m.getSelectedItem()) {
-          m.clearSelection();
-        } else if (valueFromAtom && m.getSelectedItem()?.id !== valueFromAtom) {
-          // find valueFromAtom in the selection
-          m.selectItemById(valueFromAtom);
-        }
-      });
-    },
-    [selection, tableManagerRef],
+      [
+        onRenderItem,
+        enableArrow,
+        titleAttribute,
+        descriptionAttribute,
+        idAttribute,
+      ],
     );
 
     return (
@@ -153,47 +154,33 @@ export const DataList: React.FC<DataListProps<any>> = function DataList<
         <DataTable<any>
           {...tableProps}
           tableManagerRef={tableManagerRef}
-        records={Array.isArray(items) ? items : undefined}
-        dataSource={Array.isArray(items) ? undefined : (items as any)}
-        recordsKey="id"
+          records={Array.isArray(items) ? items : undefined!}
+          dataSource={Array.isArray(items) ? undefined! : (items as any)}
+          recordsKey={idAttribute}
           columns={dataListColumns}
           onSelect={handleSelect}
         />
       </Layout.Container>
     );
-};
-
-DataList.defaultProps = {
-  type: 'default',
-  scrollable: true,
-  enableSearchbar: false,
-  enableColumnHeaders: false,
-  enableArrow: true,
-  enableContextMenu: false,
-  enableMultiSelect: false,
-};
-
-const DataListItem = memo(
-  ({
-    title,
-    description,
-    enableArrow,
-  }: {
-    // TODO: add icon support
-    title: string;
-    description?: string;
-    enableArrow?: boolean;
-  }) => {
+  },
+  {
+    Item: memo(({title, description, enableArrow}: DataListItemProps) => {
       return (
         <Layout.Horizontal center grow shrink padv>
           <Layout.Container grow shrink>
+            {typeof title === 'string' ? (
               <Text strong ellipsis>
                 {DataFormatter.format(title)}
               </Text>
-          {description != null && (
+            ) : (
+              title
+            )}
+            {typeof description === 'string' ? (
               <Text type="secondary" ellipsis>
                 {DataFormatter.format(description)}
               </Text>
+            ) : (
+              description
             )}
           </Layout.Container>
           {enableArrow && (
@@ -203,9 +190,33 @@ const DataListItem = memo(
           )}
         </Layout.Horizontal>
       );
+    }),
   },
 );
 
+(DataList as React.FC<DataListProps<any>>).defaultProps = {
+  type: 'default',
+  scrollable: true,
+  enableSearchbar: false,
+  enableColumnHeaders: false,
+  enableArrow: true,
+  enableContextMenu: false,
+  enableMultiSelect: false,
+  idAttribute: 'id',
+  titleAttribute: 'title',
+  descriptionAttribute: 'description',
+};
+(DataList.Item as React.FC<DataListItemProps>).defaultProps = {
+  enableArrow: true,
+};
+
+interface DataListItemProps {
+  // TODO: add icon support
+  title?: string | React.ReactElement;
+  description?: string | React.ReactElement;
+  enableArrow?: boolean;
+}
+
 const ArrowWrapper = styled.div({
   flex: 0,
   paddingLeft: theme.space.small,

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

@@ -60,6 +60,7 @@ interface DataTableBaseProps<T = any> {
   enableColumnHeaders?: boolean;
   enableMultiSelect?: boolean;
   enableContextMenu?: boolean;
+  enablePersistSettings?: boolean;
   // if set (the default) will grow and become scrollable. Otherwise will use natural size
   scrollable?: boolean;
   extraActions?: React.ReactElement;
@@ -153,6 +154,7 @@ export function DataTable<T extends object>(
         scope,
         virtualizerRef,
         autoScroll: props.enableAutoScroll,
+        enablePersistSettings: props.enablePersistSettings,
       }),
   );
 
@@ -177,15 +179,20 @@ export function DataTable<T extends object>(
 
   const renderingConfig = useMemo<TableRowRenderContext<T>>(() => {
     let startIndex = 0;
+
     return {
       columns: visibleColumns,
       onMouseEnter(e, _item, index) {
-        if (dragging.current && e.buttons === 1) {
+        if (dragging.current && e.buttons === 1 && props.enableMultiSelect) {
           // by computing range we make sure no intermediate items are missed when scrolling fast
           tableManager.addRangeToSelection(startIndex, index);
         }
       },
       onMouseDown(e, _item, index) {
+        if (!props.enableMultiSelect && e.buttons > 1) {
+          tableManager.selectItem(index, false, true);
+          return;
+        }
         if (!dragging.current) {
           if (e.buttons > 1) {
             // for right click we only want to add if needed, not deselect
@@ -218,7 +225,13 @@ export function DataTable<T extends object>(
           }
         : undefined,
     };
-  }, [visibleColumns, tableManager, onRowStyle, props.enableContextMenu]);
+  }, [
+    visibleColumns,
+    tableManager,
+    onRowStyle,
+    props.enableContextMenu,
+    props.enableMultiSelect,
+  ]);
 
   const itemRenderer = useCallback(
     function itemRenderer(
@@ -454,7 +467,7 @@ export function DataTable<T extends object>(
           searchValue={searchValue}
           useRegex={tableState.useRegex}
           dispatch={dispatch as any}
-          contextMenu={contexMenu}
+          contextMenu={props.enableContextMenu ? contexMenu : undefined}
           extraActions={props.extraActions}
         />
       )}

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

@@ -98,6 +98,7 @@ type DataManagerConfig<T> = {
   onSelect: undefined | ((item: T | undefined, items: T[]) => void);
   virtualizerRef: MutableRefObject<DataSourceVirtualizer | undefined>;
   autoScroll?: boolean;
+  enablePersistSettings?: boolean;
 };
 
 type DataManagerState<T> = {
@@ -272,6 +273,7 @@ export type DataTableManager<T> = {
   toggleColumnVisibility(column: keyof T): void;
   sortColumn(column: keyof T, direction?: SortDirection): void;
   setSearchValue(value: string): void;
+  dataSource: DataSource<T>;
 };
 
 export function createDataTableManager<T>(
@@ -315,6 +317,7 @@ export function createDataTableManager<T>(
     setSearchValue(value) {
       dispatch({type: 'setSearchValue', value});
     },
+    dataSource,
   };
 }
 
@@ -324,7 +327,9 @@ export function createInitialState<T>(
   const storageKey = `${config.scope}:DataTable:${config.defaultColumns
     .map((c) => c.key)
     .join(',')}`;
-  const prefs = loadStateFromStorage(storageKey);
+  const prefs = config.enablePersistSettings
+    ? loadStateFromStorage(storageKey)
+    : undefined;
   let initialColumns = computeInitialColumns(config.defaultColumns);
   if (prefs) {
     // merge prefs with the default column config
@@ -411,7 +416,7 @@ export function savePreferences(
   state: DataManagerState<any>,
   scrollOffset: number,
 ) {
-  if (!state.config.scope) {
+  if (!state.config.scope || !state.config.enablePersistSettings) {
     return;
   }
   const prefs: PersistedState = {

desktop/flipper-plugin/src/utils/useLatestRef.tsx

@@ -0,0 +1,24 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+import {useEffect, useRef} from 'react';
+
+/**
+ * Creates a ref object that is always synced from the value passed in.
+ */
+export function useLatestRef<T>(latest: T): {readonly current: T} {
+  const latestRef = useRef(latest);
+
+  // TODO: sync eagerly (in render) or late? Introduce a `syncEarly` flag as second arg
+  useEffect(() => {
+    latestRef.current = latest;
+  }, [latest]);
+
+  return latestRef;
+}

desktop/flipper-plugin/src/utils/useMakeStableCallback.tsx

@@ -0,0 +1,34 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+import {useCallback} from 'react';
+import {useLatestRef} from './useLatestRef';
+
+/**
+ * This hook can be used to avoid forcing consumers of a component to wrap their callbacks
+ * in useCallback, by creating wrapper callback that redirects to the lastest prop passed in.
+ *
+ * Use this hook if you would like to avoid that passing a new callback to this component,
+ * will cause child components to rerender when the callback is passed further down.
+ *
+ * Use it like: `const onSelect = useMakeStableCallback(props.onSelect)`.
+ * @param fn
+ */
+export function useMakeStableCallback<
+  T extends undefined | ((...args: any[]) => any),
+>(fn: T): T {
+  const latestFn = useLatestRef(fn);
+
+  return useCallback(
+    (...args: any[]) => {
+      return latestFn.current?.apply(null, args);
+    },
+    [latestFn],
+  ) as any;
+}

desktop/plugins/public/crash_reporter/Crashes.tsx

@@ -37,7 +37,7 @@ export function Crashes() {
           title: crash.reason ?? crash.name,
           description: `${crash.date.toLocaleString()} - ${crash.name}`,
         }))}
-        selection={plugin.selectedCrash}
+        selection={selectedCrashId}
         onRenderEmpty={null}
       />
       {selectedCrash ? (

desktop/plugins/public/network/request-mocking/ManageMockResponsePanel.tsx

@@ -159,7 +159,7 @@ export function ManageMockResponsePanel(props: Props) {
         </Toolbar>
         <DataList
           items={items}
-          selection={selectedIdAtom}
+          selection={selectedId}
           onRenderItem={handleRender}
           scrollable
         />