Provide standardised MasterDetail

Summary:
Noticed in reviews during the convertathon there is still quite some boilerplate in things that happen on the boundary of UI and plugin state, such as setting up menu entries and providing common functionality like clear, master/detail layout, etc.

This diff introduces the `MasterDetail` component, which takes a higher level approach by merely needing to provide the state atoms and desired features, and taking care of the wiring.

Applied it to createTablePlugin, to prove that going from `createTablePlugin` to `MasterDetail` will be a much smaller step now.

Verified on the funnel logger plugin

Reviewed By: passy

Differential Revision: D28090362

fbshipit-source-id: 146f8c315fea903901ad4e3e46711642f16cf0e6

desktop/flipper-plugin/src/__tests__/api.node.tsx

@@ -38,6 +38,7 @@ test('Correct top level API exposed', () => {
       "ElementsInspector",
       "Layout",
       "MarkerTimeline",
+      "MasterDetail",
       "NUX",
       "Panel",
       "Tab",

desktop/flipper-plugin/src/index.ts

@@ -58,6 +58,7 @@ export {
 export {Sidebar as _Sidebar} from './ui/Sidebar';
 export {DetailSidebar} from './ui/DetailSidebar';
 export {Toolbar} from './ui/Toolbar';
+export {MasterDetail} from './ui/MasterDetail';
 
 export {renderReactRoot} from './utils/renderReactRoot';
 export {

desktop/flipper-plugin/src/plugin/DevicePlugin.tsx

@@ -11,7 +11,7 @@ import {SandyPluginDefinition} from './SandyPluginDefinition';
 import {BasePluginInstance, BasePluginClient} from './PluginBase';
 import {FlipperLib} from './FlipperLib';
 import {DeviceType as PluginDeviceType} from 'flipper-plugin-lib';
-import {Atom} from '../state/atom';
+import {Atom, ReadOnlyAtom} from '../state/atom';
 
 export type DeviceLogListener = (entry: DeviceLogEntry) => void;
 
@@ -60,6 +60,9 @@ export interface DevicePluginClient extends BasePluginClient {
    * opens a different plugin by id, optionally providing a deeplink to bring the plugin to a certain state
    */
   selectPlugin(pluginId: string, deeplinkPayload?: unknown): void;
+
+  readonly isConnected: boolean;
+  readonly connected: ReadOnlyAtom<boolean>;
 }
 
 /**
@@ -103,6 +106,10 @@ export class SandyDevicePluginInstance extends BasePluginInstance {
           flipperLib.selectPlugin(realDevice, null, pluginId, deeplink);
         }
       },
+      get isConnected() {
+        return realDevice.connected.get();
+      },
+      connected: realDevice.connected,
     };
     this.initializePlugin(() =>
       definition.asDevicePluginModule().devicePlugin(this.client),

desktop/flipper-plugin/src/plugin/PluginBase.tsx

@@ -77,6 +77,12 @@ export interface BasePluginClient {
    */
   createPaste(input: string): Promise<string | undefined>;
 
+  /**
+   * Returns true if this is an internal Facebook build.
+   * Always returns `false` in open source
+   */
+  readonly isFB: boolean;
+
   /**
    * Returns true if the user is taking part in the given gatekeeper.
    * Always returns `false` in open source.
@@ -224,6 +230,8 @@ export abstract class BasePluginInstance {
             batched(this.importHandler)(this.initialStates);
           } catch (e) {
             const msg = `Error occurred when importing date for plugin '${this.definition.id}': '${e}`;
+            // msg is already specific
+            // eslint-disable-next-line
             console.error(msg, e);
             message.error(msg);
           }
@@ -275,20 +283,25 @@ export abstract class BasePluginInstance {
       addMenuEntry: (...entries) => {
         for (const entry of entries) {
           const normalized = normalizeMenuEntry(entry);
-          if (
-            this.menuEntries.find(
+          const idx = this.menuEntries.findIndex(
             (existing) =>
               existing.label === normalized.label ||
               existing.action === normalized.action,
-            )
-          ) {
-            throw new Error(`Duplicate menu entry: '${normalized.label}'`);
-          }
+          );
+          if (idx !== -1) {
+            this.menuEntries[idx] = normalizeMenuEntry(entry);
+          } else {
             this.menuEntries.push(normalizeMenuEntry(entry));
           }
+          if (this.activated) {
+            // entries added after initial registration
+            this.flipperLib.enableMenuEntries(this.menuEntries);
+          }
+        }
       },
       writeTextToClipboard: this.flipperLib.writeTextToClipboard,
       createPaste: this.flipperLib.createPaste,
+      isFB: this.flipperLib.isFB,
       GK: this.flipperLib.GK,
       showNotification: (notification: Notification) => {
         this.flipperLib.showNotification(this.pluginKey, notification);
@@ -301,8 +314,8 @@ export abstract class BasePluginInstance {
   activate() {
     this.assertNotDestroyed();
     if (!this.activated) {
-      this.activated = true;
       this.flipperLib.enableMenuEntries(this.menuEntries);
+      this.activated = true;
       this.events.emit('activate');
       this.flipperLib.logger.trackTimeSince(
         `activePlugin-${this.definition.id}`,

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

@@ -0,0 +1,263 @@
+/**
+ * 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 * as React from 'react';
+import {
+  createElement,
+  createRef,
+  useCallback,
+  useState,
+  useEffect,
+} from 'react';
+import {DataInspector} from './data-inspector/DataInspector';
+import {DataTable, DataTableProps} from './data-table/DataTable';
+import {DataTableManager} from './data-table/DataTableManager';
+import {DetailSidebar} from './DetailSidebar';
+import {Layout} from './Layout';
+import {Panel} from './Panel';
+
+import {
+  DeleteOutlined,
+  PauseCircleOutlined,
+  PlayCircleOutlined,
+} from '@ant-design/icons';
+import {Button} from 'antd';
+import {usePluginInstance} from '../plugin/PluginContext';
+import {Atom, createState, useValue} from '../state/atom';
+import {useAssertStableRef} from '../utils/useAssertStableRef';
+
+type MasterDetailProps<T> = {
+  /**
+   * Where to display the details of the currently selected record?
+   * 'main' (default): show the details in the standard, centrally controlled right sidebar
+   * 'right': show a resizable pane to the right
+   * 'bottom': show a resizable pane to the bottom
+   * 'none': don't show details at all
+   */
+  sidebarPosition?: 'bottom' | 'right' | 'main' | 'none';
+  /**
+   * Component that accepts a 'record' prop that is used to render details.
+   * If none is provided, a standard `DataInspector` component will be used to display the entire record.
+   */
+  sidebarComponent?: React.FC<{record: T}>;
+  /**
+   * Default size of the sidebar.
+   */
+  sidebarSize?: number;
+  /**
+   * If provided, this atom will be used to store selection in.
+   */
+  selection?: Atom<T | undefined>;
+  /**
+   * If provided, this atom will be used to store pause/resume state in, and a pause/resume toggle will be shown
+   */
+  isPaused?: Atom<boolean>;
+  /**
+   * If set, a clear button will be shown.
+   * By default this will clear the dataSource (if applicable).
+   */
+  enableClear?: boolean;
+  /**
+   * Callback to be called when clear action is used.
+   */
+  onClear?: () => void;
+  /**
+   * If provided, standard menu entries will be created for clear, goToBottom and createPaste
+   */
+  enableMenuEntries?: boolean;
+};
+
+export function MasterDetail<T extends object>({
+  dataSource,
+  records,
+  sidebarComponent,
+  sidebarPosition,
+  sidebarSize,
+  onSelect,
+  extraActions,
+  enableMenuEntries,
+  enableClear,
+  isPaused,
+  selection,
+  onClear,
+  ...tableProps
+}: MasterDetailProps<T> & DataTableProps<T>) {
+  useAssertStableRef(isPaused, 'isPaused');
+  useAssertStableRef(selection, 'selection');
+
+  const pluginInstance = usePluginInstance();
+  const {client} = pluginInstance;
+  const connected = useValue(pluginInstance.client.connected);
+
+  const selectionAtom =
+    // if no selection atom is provided, the component is uncontrolled
+    // and we maintain our own selection atom
+    // eslint-disable-next-line
+    selection ?? useState(() => createState<T | undefined>(undefined))[0];
+  const selectedRecord = useValue(selectionAtom);
+
+  // if a tableManagerRef is provided, we piggy back on that same ref
+  // eslint-disable-next-line
+  const tableManagerRef = tableProps.tableManagerRef ?? createRef<undefined | DataTableManager<T>>();
+
+  const pausedState = useValue(isPaused, false);
+
+  const sidebar =
+    sidebarPosition !== 'none' && selectedRecord && sidebarComponent
+      ? createElement(sidebarComponent, {
+          record: selectedRecord,
+        })
+      : null;
+
+  const handleSelect = useCallback(
+    (record: T | undefined, records: T[]) => {
+      selectionAtom.set(record);
+      onSelect?.(record, records);
+    },
+    [selectionAtom, onSelect],
+  );
+
+  const handleTogglePause = useCallback(() => {
+    isPaused?.set(!isPaused?.get());
+  }, [isPaused]);
+
+  const handleClear = useCallback(() => {
+    handleSelect(undefined, []);
+    if (dataSource) {
+      dataSource.clear();
+      onClear?.();
+    } else {
+      if (!onClear) {
+        throw new Error(
+          "onClear must be set when using 'enableClear' and 'records'",
+        );
+      }
+      onClear();
+    }
+  }, [dataSource, onClear, handleSelect]);
+
+  const handleCreatePaste = useCallback(() => {
+    const selection = tableManagerRef.current?.getSelectedItems();
+    switch (selection?.length) {
+      case undefined:
+      case 0:
+        return;
+      case 1:
+        client.createPaste(JSON.stringify(selection[0], null, 2));
+        break;
+      default:
+        client.createPaste(JSON.stringify(selection, null, 2));
+    }
+  }, [client, tableManagerRef]);
+
+  const handleGoToBottom = useCallback(() => {
+    const size = dataSource ? dataSource.view.size : records!.length;
+    tableManagerRef?.current?.selectItem(size - 1);
+  }, [dataSource, records, tableManagerRef]);
+
+  useEffect(
+    function setupMenuEntries() {
+      if (enableMenuEntries) {
+        if (enableClear) {
+          client.addMenuEntry({
+            action: 'clear',
+            handler: handleClear,
+          });
+        }
+        if (client.isFB) {
+          client.addMenuEntry({
+            action: 'createPaste',
+            handler: handleCreatePaste,
+          });
+        }
+        client.addMenuEntry({
+          action: 'goToBottom',
+          handler: handleGoToBottom,
+        });
+      }
+    },
+    [
+      client,
+      enableClear,
+      enableMenuEntries,
+      handleClear,
+      handleCreatePaste,
+      handleGoToBottom,
+    ],
+  );
+
+  const table = (
+    <DataTable<T>
+      autoScroll
+      {...tableProps}
+      dataSource={dataSource as any}
+      records={records!}
+      tableManagerRef={tableManagerRef}
+      onSelect={handleSelect}
+      extraActions={
+        <>
+          {connected && isPaused && (
+            <Button
+              title={`Click to ${pausedState ? 'resume' : 'pause'} the stream`}
+              danger={pausedState}
+              onClick={handleTogglePause}>
+              {pausedState ? <PlayCircleOutlined /> : <PauseCircleOutlined />}
+            </Button>
+          )}
+          {connected && enableClear && (
+            <Button title="Clear records" onClick={handleClear}>
+              <DeleteOutlined />
+            </Button>
+          )}
+          {extraActions}
+        </>
+      }
+    />
+  );
+
+  switch (sidebarPosition!) {
+    case 'main':
+      return (
+        <Layout.Container grow>
+          {table}
+          <DetailSidebar>{sidebar}</DetailSidebar>
+        </Layout.Container>
+      );
+    case 'right':
+      return (
+        <Layout.Right resizable width={sidebarSize}>
+          {table}
+          {sidebar}
+        </Layout.Right>
+      );
+    case 'bottom':
+      return (
+        <Layout.Bottom resizable width={sidebarSize}>
+          {table}
+          {sidebar}
+        </Layout.Bottom>
+      );
+    case 'none':
+      return table;
+  }
+}
+
+MasterDetail.defaultProps = {
+  sidebarPosition: 'main',
+  sidebarSize: 400,
+  sidebarComponent: DefaultRenderSidebar,
+} as Partial<MasterDetailProps<any>>;
+
+function DefaultRenderSidebar<T>({record}: {record: T}) {
+  return (
+    <Panel title="Payload" collapsible={false} pad>
+      <DataInspector data={record} expandRoot />
+    </Panel>
+  );
+}

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

@@ -49,7 +49,7 @@ import {debounce} from 'lodash';
 import {StaticDataSourceRenderer} from './StaticDataSourceRenderer';
 import {useInUnitTest} from '../../utils/useInUnitTest()';
 
-interface DataTableProps<T = any> {
+interface DataTableBaseProps<T = any> {
   columns: DataTableColumn<T>[];
 
   autoScroll?: boolean;
@@ -65,9 +65,10 @@ interface DataTableProps<T = any> {
 }
 
 type DataTableInput<T = any> =
-  | {dataSource: DataSource<T, any, any>}
+  | {dataSource: DataSource<T, any, any>; records?: undefined}
   | {
       records: T[];
+      dataSource?: undefined;
     };
 
 export type DataTableColumn<T = any> = {
@@ -102,8 +103,10 @@ export interface RenderContext<T = any> {
   ): void;
 }
 
+export type DataTableProps<T> = DataTableInput<T> & DataTableBaseProps<T>;
+
 export function DataTable<T extends object>(
-  props: DataTableInput<T> & DataTableProps<T>,
+  props: DataTableProps<T>,
 ): React.ReactElement {
   const {onRowStyle, onSelect, onCopyRows, onContextMenu} = props;
   const dataSource = normalizeDataSourceInput(props);
@@ -140,6 +143,9 @@ export function DataTable<T extends object>(
   const [tableManager] = useState(() =>
     createDataTableManager(dataSource, dispatch, stateRef),
   );
+  if (props.tableManagerRef) {
+    (props.tableManagerRef as MutableRefObject<any>).current = tableManager;
+  }
 
   const {columns, selection, searchValue, sorting} = tableState;
 
@@ -385,10 +391,6 @@ export function DataTable<T extends object>(
       );
 
   useEffect(function initialSetup() {
-    if (props.tableManagerRef) {
-      (props.tableManagerRef as MutableRefObject<any>).current = tableManager;
-    }
-
     return function cleanup() {
       // write current prefs to local storage
       savePreferences(stateRef.current, lastOffset.current);
@@ -484,10 +486,10 @@ export function DataTable<T extends object>(
 
 /* eslint-disable react-hooks/rules-of-hooks */
 function normalizeDataSourceInput<T>(props: DataTableInput<T>): DataSource<T> {
-  if ('dataSource' in props) {
+  if (props.dataSource) {
     return props.dataSource;
   }
-  if ('records' in props) {
+  if (props.records) {
     const [dataSource] = useState(() => {
       const ds = new DataSource<T>(undefined);
       syncRecordsToDataSource(ds, props.records);

desktop/flipper-plugin/src/utils/__tests__/createTablePlugin.node.tsx

@@ -29,11 +29,7 @@ test('createTablePlugin returns FlipperPlugin', () => {
     Array [
       "selection",
       "rows",
-      "clear",
-      "tableManagerRef",
-      "connected",
       "isPaused",
-      "resumePause",
     ]
   `);
 });

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

@@ -7,31 +7,14 @@
  * @format
  */
 
-import {
-  DeleteOutlined,
-  PauseCircleOutlined,
-  PlayCircleOutlined,
-} from '@ant-design/icons';
-import {Button, notification, Typography} from 'antd';
-import React, {createRef, useCallback} from 'react';
+import {notification, Typography} from 'antd';
+import React from 'react';
 import {PluginClient} from '../plugin/Plugin';
 import {usePlugin} from '../plugin/PluginContext';
-import {createState, useValue} from '../state/atom';
+import {createState} from '../state/atom';
 import {createDataSource, DataSource} from '../state/DataSource';
-import {DataInspector} from '../ui/data-inspector/DataInspector';
-import {DataTable, DataTableColumn} from '../ui/data-table/DataTable';
-import {DataTableManager} from '../ui/data-table/DataTableManager';
-import {DetailSidebar} from '../ui/DetailSidebar';
-import {Layout} from '../ui/Layout';
-import {Panel} from '../ui/Panel';
-
-function defaultRenderSidebar<T>(record: T) {
-  return (
-    <Panel title="Payload" collapsible={false} pad>
-      <DataInspector data={record} expandRoot />
-    </Panel>
-  );
-}
+import {DataTableColumn} from '../ui/data-table/DataTable';
+import {MasterDetail} from '../ui/MasterDetail';
 
 type PluginResult<Raw, Row> = {
   plugin(
@@ -95,7 +78,6 @@ export function createTablePlugin<
     });
     const selection = createState<undefined | Row>(undefined);
     const isPaused = createState(false);
-    const tableManagerRef = createRef<undefined | DataTableManager<Row>>();
 
     client.onMessage(props.method, (event) => {
       if (isPaused.get()) {
@@ -113,7 +95,7 @@ export function createTablePlugin<
 
     if (props.resetMethod) {
       client.onMessage(props.resetMethod, () => {
-        clear();
+        rows.clear();
       });
     }
 
@@ -134,94 +116,29 @@ export function createTablePlugin<
       });
     });
 
-    client.addMenuEntry(
-      {
-        action: 'clear',
-        handler: clear,
-      },
-      {
-        action: 'createPaste',
-        handler: createPaste,
-      },
-      {
-        action: 'goToBottom',
-        handler: goToBottom,
-      },
-    );
-
-    function clear() {
-      rows.clear();
-      tableManagerRef.current?.clearSelection();
-    }
-
-    function createPaste() {
-      let selection = tableManagerRef.current?.getSelectedItems();
-      if (!selection?.length) {
-        selection = rows.view.output(0, rows.view.size);
-      }
-      if (selection?.length) {
-        client.createPaste(JSON.stringify(selection, null, 2));
-      }
-    }
-
-    function goToBottom() {
-      tableManagerRef?.current?.selectItem(rows.view.size - 1);
-    }
-
     return {
       selection,
       rows,
-      clear,
-      tableManagerRef,
-      connected: client.connected,
       isPaused,
-      resumePause() {
-        isPaused.update((v) => !v);
-      },
     };
   }
 
+  function SidebarComponent({record}: {record: Row}) {
+    return props.renderSidebar!(record);
+  }
+
   function Component() {
     const instance = usePlugin(plugin);
-    const paused = useValue(instance.isPaused);
-    const selection = useValue(instance.selection);
-    const connected = useValue(instance.connected);
-
-    const handleSelect = useCallback((v) => instance.selection.set(v), [
-      instance,
-    ]);
-
     return (
-      <Layout.Container grow>
-        <DataTable<Row>
+      <MasterDetail<Row>
         columns={props.columns}
         dataSource={instance.rows}
-          tableManagerRef={instance.tableManagerRef}
-          autoScroll
-          onSelect={handleSelect}
-          extraActions={
-            connected ? (
-              <>
-                <Button
-                  title={`Click to ${paused ? 'resume' : 'pause'} the stream`}
-                  danger={paused}
-                  onClick={instance.resumePause}>
-                  {paused ? <PlayCircleOutlined /> : <PauseCircleOutlined />}
-                </Button>
-                <Button title="Clear records" onClick={instance.clear}>
-                  <DeleteOutlined />
-                </Button>
-              </>
-            ) : undefined
-          }
+        sidebarComponent={props.renderSidebar ? SidebarComponent : undefined}
+        selection={instance.selection}
+        isPaused={instance.isPaused}
+        enableMenuEntries
+        enableClear
       />
-        <DetailSidebar>
-          {selection
-            ? props.renderSidebar?.(selection) ??
-              defaultRenderSidebar(selection)
-            : null}
-        </DetailSidebar>
-      </Layout.Container>
     );
   }
 

docs/extending/flipper-plugin.mdx

@@ -824,6 +824,27 @@ See `View > Flipper Style Guide` inside the Flipper application for more details
 ### ElementsInspectorElement
 ### ElementsInspectorProps
 
+Coming soon
+
+### MasterDetail
+
+The `MasterDetail` provides a default setup for a `DataTable` in combination with a sidebar, and some super common features like keyboard shortcuts, paste integration, and clear / pause-resume buttons.
+
+The `MasterDetail` component accepts all `DataTable` props, and beyond that the following props.
+
+* The `sidebarPosition` prop controls here to display the details of the currently selected record:
+    * `'main'` (default): show the details in the standard, centrally controlled right sidebar
+    * `'right'`: show a resizable pane to the right
+    * `'bottom'`: show a resizable pane to the bottom
+    * `'none'`: don't show details at all
+* `sidebarComponent`: Component that accepts a 'record' prop that is used to render details.If none is provided, a standard `DataInspector` component will be used to display the entire record.
+* `sidebarSize`: Default size of the sidebar.
+* `selection`: If provided, this atom will be used to store selection in.
+* `isPaused`: If provided, this atom will be used to store pause/resume state in, and a pause/resume toggle will be shown
+* `enableClear: If set, a clear button will be shown.By default this will clear the dataSource (if any).
+* `onClear`: Callback to be called when clear action is used.
+* `enableMenuEntries`: If provided, standard menu entries will be created for clear, goToBottom and createPaste.
+
 ### Panel
 
 A collapsible UI region. The collapsed state of the pane will automatically be persisted so that the collapsed state is restored the next time user visits the plugin again. Note that the children of a Panel should have some size, either a fixed or a natural size. Elements that grow to their parent size will become invisible.