From 5c3a8742ef06f58e312c2b8d3452baea6ed4c4f4 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Tue, 16 Mar 2021 14:54:53 -0700
Subject: [PATCH] Introduce context menu

Summary:
Introduced a context menu for DataTable with some default options. Opted to put it under a visible hovered dropdown instead of on right-click, since this better alings with Ant's design guides (we don't have context clicks anywhere else I think), but if it isn't convincing we can still change it.

Included some default actions, to set up quick filters, and to copy values. For copying rows, implemented it to by default take the JSON from a row, rather than space separated values like in ManagedTable, as many existing plugins customize the onCopy handler to just do that, so it seemed like a better default since it is a richer format. If there are good use cases for the previous behavior, we'll probably find out after the old release :)

Introduced utility to copy text to clipboard in FlipperLib, but decoupled it from Electron.

Didn't include multi select yet, that will be done in a next diff.

Reviewed By: nikoant

Differential Revision: D26513161

fbshipit-source-id: b2b1b20b0a6f4ada9de2566bf6b02171f722c4aa
---
 desktop/app/src/Client.tsx                    | 11 ++-
 desktop/app/src/devices/BaseDevice.tsx        |  4 +-
 .../src/utils/flipperLibImplementation.tsx    | 25 ++----
 desktop/flipper-plugin/src/index.ts           |  6 +-
 .../flipper-plugin/src/plugin/FlipperLib.tsx  | 19 +++++
 .../src/test-utils/test-utils.tsx             |  2 +
 .../src/ui/datatable/DataTable.tsx            | 40 ++++++---
 .../src/ui/datatable/TableContextMenu.tsx     | 83 +++++++++++++++++++
 .../src/ui/datatable/TableRow.tsx             | 71 +++++++++++++---
 .../ui/datatable/__tests__/DataTable.node.tsx | 20 +++--
 .../src/ui/datatable/useDataTableManager.tsx  | 42 +++++++---
 11 files changed, 257 insertions(+), 66 deletions(-)
 create mode 100644 desktop/flipper-plugin/src/ui/datatable/TableContextMenu.tsx

diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx
index 53c5bb3aa..2b5e06a8a 100644
--- a/desktop/app/src/Client.tsx
+++ b/desktop/app/src/Client.tsx
@@ -35,9 +35,12 @@ import {processMessagesLater} from './utils/messageQueue';
 import {emitBytesReceived} from './dispatcher/tracking';
 import {debounce} from 'lodash';
 import {batch} from 'react-redux';
-import {createState, _SandyPluginInstance} from 'flipper-plugin';
+import {
+  createState,
+  _SandyPluginInstance,
+  _getFlipperLibImplementation,
+} from 'flipper-plugin';
 import {flipperMessagesClientPlugin} from './utils/self-inspection/plugins/FlipperMessagesClientPlugin';
-import {getFlipperLibImplementation} from './utils/flipperLibImplementation';
 import {freeze} from 'immer';
 import GK from './fb-stubs/GK';
 import {message} from 'antd';
@@ -254,7 +257,7 @@ export default class Client extends EventEmitter {
         this.sandyPluginStates.set(
           plugin.id,
           new _SandyPluginInstance(
-            getFlipperLibImplementation(),
+            _getFlipperLibImplementation(),
             plugin,
             this,
             initialStates[pluginId],
@@ -303,7 +306,7 @@ export default class Client extends EventEmitter {
       // TODO: needs to be wrapped in error tracking T68955280
       this.sandyPluginStates.set(
         plugin.id,
-        new _SandyPluginInstance(getFlipperLibImplementation(), plugin, this),
+        new _SandyPluginInstance(_getFlipperLibImplementation(), plugin, this),
       );
     }
   }
diff --git a/desktop/app/src/devices/BaseDevice.tsx b/desktop/app/src/devices/BaseDevice.tsx
index 391a64da9..0e99f6bbc 100644
--- a/desktop/app/src/devices/BaseDevice.tsx
+++ b/desktop/app/src/devices/BaseDevice.tsx
@@ -16,13 +16,13 @@ import {
   DeviceLogListener,
   Idler,
   createState,
+  _getFlipperLibImplementation,
 } from 'flipper-plugin';
 import {
   DevicePluginDefinition,
   DevicePluginMap,
   FlipperDevicePlugin,
 } from '../plugin';
-import {getFlipperLibImplementation} from '../utils/flipperLibImplementation';
 import {DeviceSpec, OS as PluginOS, PluginDetails} from 'flipper-plugin-lib';
 
 export type DeviceShell = {
@@ -238,7 +238,7 @@ export default class BaseDevice {
       this.sandyPluginStates.set(
         plugin.id,
         new _SandyDevicePluginInstance(
-          getFlipperLibImplementation(),
+          _getFlipperLibImplementation(),
           plugin,
           this,
           initialState,
diff --git a/desktop/app/src/utils/flipperLibImplementation.tsx b/desktop/app/src/utils/flipperLibImplementation.tsx
index f3ccc2de3..909535eaf 100644
--- a/desktop/app/src/utils/flipperLibImplementation.tsx
+++ b/desktop/app/src/utils/flipperLibImplementation.tsx
@@ -7,14 +7,14 @@
  * @format
  */
 
-import type {FlipperLib} from 'flipper-plugin';
+import {_setFlipperLibImplementation} from 'flipper-plugin';
 import type {Logger} from '../fb-interfaces/Logger';
 import type {Store} from '../reducers';
 import createPaste from '../fb-stubs/createPaste';
 import GK from '../fb-stubs/GK';
 import type BaseDevice from '../devices/BaseDevice';
-
-let flipperLibInstance: FlipperLib | undefined;
+import {clipboard} from 'electron';
+import constants from '../fb-stubs/constants';
 
 export function initializeFlipperLibImplementation(
   store: Store,
@@ -22,7 +22,8 @@ export function initializeFlipperLibImplementation(
 ) {
   // late require to avoid cyclic dependency
   const {addSandyPluginEntries} = require('../MenuBar');
-  flipperLibInstance = {
+  _setFlipperLibImplementation({
+    isFB: !constants.IS_PUBLIC_BUILD,
     logger,
     enableMenuEntries(entries) {
       addSandyPluginEntries(entries);
@@ -67,16 +68,8 @@ export function initializeFlipperLibImplementation(
         },
       });
     },
-  };
-}
-
-export function getFlipperLibImplementation(): FlipperLib {
-  if (!flipperLibInstance) {
-    throw new Error('Flipper lib not instantiated');
-  }
-  return flipperLibInstance;
-}
-
-export function setFlipperLibImplementation(impl: FlipperLib) {
-  flipperLibInstance = impl;
+    writeTextToClipboard(text: string) {
+      clipboard.writeText(text);
+    },
+  });
 }
diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts
index 6a7bb1957..9c363d844 100644
--- a/desktop/flipper-plugin/src/index.ts
+++ b/desktop/flipper-plugin/src/index.ts
@@ -35,7 +35,11 @@ export {
 } from './plugin/PluginContext';
 export {createState, useValue, Atom} from './state/atom';
 export {batch} from './state/batch';
-export {FlipperLib} from './plugin/FlipperLib';
+export {
+  FlipperLib,
+  getFlipperLibImplementation as _getFlipperLibImplementation,
+  setFlipperLibImplementation as _setFlipperLibImplementation,
+} from './plugin/FlipperLib';
 export {
   MenuEntry,
   NormalizedMenuEntry,
diff --git a/desktop/flipper-plugin/src/plugin/FlipperLib.tsx b/desktop/flipper-plugin/src/plugin/FlipperLib.tsx
index a7b787b44..e7e18556b 100644
--- a/desktop/flipper-plugin/src/plugin/FlipperLib.tsx
+++ b/desktop/flipper-plugin/src/plugin/FlipperLib.tsx
@@ -16,6 +16,7 @@ import {RealFlipperClient} from './Plugin';
  * This interface exposes all global methods for which an implementation will be provided by Flipper itself
  */
 export interface FlipperLib {
+  isFB: boolean;
   logger: Logger;
   enableMenuEntries(menuEntries: NormalizedMenuEntry[]): void;
   createPaste(input: string): Promise<string | undefined>;
@@ -31,4 +32,22 @@ export interface FlipperLib {
     pluginId: string,
     deeplink: unknown,
   ): void;
+  writeTextToClipboard(text: string): void;
+}
+
+let flipperLibInstance: FlipperLib | undefined;
+
+export function tryGetFlipperLibImplementation(): FlipperLib | undefined {
+  return flipperLibInstance;
+}
+
+export function getFlipperLibImplementation(): FlipperLib {
+  if (!flipperLibInstance) {
+    throw new Error('Flipper lib not instantiated');
+  }
+  return flipperLibInstance;
+}
+
+export function setFlipperLibImplementation(impl: FlipperLib) {
+  flipperLibInstance = impl;
 }
diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx
index ffd8f6af8..da2ecaa1a 100644
--- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx
+++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx
@@ -359,6 +359,7 @@ export function renderDevicePlugin<Module extends FlipperDevicePluginModule>(
 
 export function createMockFlipperLib(options?: StartPluginOptions): FlipperLib {
   return {
+    isFB: false,
     logger: stubLogger,
     enableMenuEntries: jest.fn(),
     createPaste: jest.fn(),
@@ -367,6 +368,7 @@ export function createMockFlipperLib(options?: StartPluginOptions): FlipperLib {
     },
     selectPlugin: jest.fn(),
     isPluginAvailable: jest.fn().mockImplementation(() => false),
+    writeTextToClipboard: jest.fn(),
   };
 }
 
diff --git a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx
index 507c527ff..6d13176b3 100644
--- a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx
@@ -26,6 +26,11 @@ import {useDataTableManager, TableManager} from './useDataTableManager';
 import {TableSearch} from './TableSearch';
 import styled from '@emotion/styled';
 import {theme} from '../theme';
+import {
+  tableContextMenuFactory,
+  TableContextMenuContext,
+} from './TableContextMenu';
+import {useMemoize} from '../../utils/useMemoize';
 
 interface DataTableProps<T = any> {
   columns: DataTableColumn<T>[];
@@ -187,6 +192,15 @@ export function DataTable<T extends object>(props: DataTableProps<T>) {
     [],
   );
 
+  /** Context menu */
+  const contexMenu = !props._testHeight // don't render context menu in tests
+    ? // eslint-disable-next-line
+      useMemoize(tableContextMenuFactory, [
+        visibleColumns,
+        tableManager.addColumnFilter,
+      ])
+    : undefined;
+
   return (
     <Layout.Container grow>
       <Layout.Top>
@@ -208,18 +222,20 @@ export function DataTable<T extends object>(props: DataTableProps<T>) {
             onToggleColumnFilter={tableManager.toggleColumnFilter}
           />
         </Layout.Container>
-        <DataSourceRenderer<T, RenderContext<T>>
-          dataSource={dataSource}
-          autoScroll={props.autoScroll}
-          useFixedRowHeight={!usesWrapping}
-          defaultRowHeight={DEFAULT_ROW_HEIGHT}
-          context={renderingConfig}
-          itemRenderer={itemRenderer}
-          onKeyDown={onKeyDown}
-          virtualizerRef={virtualizerRef}
-          onRangeChange={onRangeChange}
-          _testHeight={props._testHeight}
-        />
+        <TableContextMenuContext.Provider value={contexMenu}>
+          <DataSourceRenderer<T, RenderContext<T>>
+            dataSource={dataSource}
+            autoScroll={props.autoScroll}
+            useFixedRowHeight={!usesWrapping}
+            defaultRowHeight={DEFAULT_ROW_HEIGHT}
+            context={renderingConfig}
+            itemRenderer={itemRenderer}
+            onKeyDown={onKeyDown}
+            virtualizerRef={virtualizerRef}
+            onRangeChange={onRangeChange}
+            _testHeight={props._testHeight}
+          />
+        </TableContextMenuContext.Provider>
       </Layout.Top>
       {range && <RangeFinder>{range}</RangeFinder>}
     </Layout.Container>
diff --git a/desktop/flipper-plugin/src/ui/datatable/TableContextMenu.tsx b/desktop/flipper-plugin/src/ui/datatable/TableContextMenu.tsx
new file mode 100644
index 000000000..d040d7c87
--- /dev/null
+++ b/desktop/flipper-plugin/src/ui/datatable/TableContextMenu.tsx
@@ -0,0 +1,83 @@
+/**
+ * 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 {CopyOutlined, FilterOutlined} from '@ant-design/icons';
+import {Menu} from 'antd';
+import {DataTableColumn} from './DataTable';
+import {TableManager} from './useDataTableManager';
+import React from 'react';
+import {createContext} from 'react';
+import {normalizeCellValue} from './TableRow';
+import {tryGetFlipperLibImplementation} from '../../plugin/FlipperLib';
+
+const {Item, SubMenu} = Menu;
+
+export const TableContextMenuContext = createContext<
+  undefined | ((item: any) => React.ReactElement)
+>(undefined);
+
+export function tableContextMenuFactory<T>(
+  visibleColumns: DataTableColumn<T>[],
+  addColumnFilter: TableManager['addColumnFilter'],
+) {
+  return function (item: any) {
+    const lib = tryGetFlipperLibImplementation();
+    if (!lib) {
+      return (
+        <Menu>
+          <Item>Menu not ready</Item>
+        </Menu>
+      );
+    }
+    return (
+      <Menu>
+        <SubMenu title="Filter on" icon={<FilterOutlined />}>
+          {visibleColumns.map((column) => (
+            <Item
+              key={column.key}
+              onClick={() => {
+                addColumnFilter(
+                  column.key,
+                  normalizeCellValue(item[column.key]),
+                  true,
+                );
+              }}>
+              {column.title || column.key}
+            </Item>
+          ))}
+        </SubMenu>
+        <SubMenu title="Copy cell" icon={<CopyOutlined />}>
+          {visibleColumns.map((column) => (
+            <Item
+              key={column.key}
+              onClick={() => {
+                lib.writeTextToClipboard(normalizeCellValue(item[column.key]));
+              }}>
+              {column.title || column.key}
+            </Item>
+          ))}
+        </SubMenu>
+        <Item
+          onClick={() => {
+            lib.writeTextToClipboard(JSON.stringify(item, null, 2));
+          }}>
+          Copy row
+        </Item>
+        {lib.isFB && (
+          <Item
+            onClick={() => {
+              lib.createPaste(JSON.stringify(item, null, 2));
+            }}>
+            Create paste
+          </Item>
+        )}
+      </Menu>
+    );
+  };
+}
diff --git a/desktop/flipper-plugin/src/ui/datatable/TableRow.tsx b/desktop/flipper-plugin/src/ui/datatable/TableRow.tsx
index a93b0eed7..6b282891d 100644
--- a/desktop/flipper-plugin/src/ui/datatable/TableRow.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/TableRow.tsx
@@ -7,12 +7,15 @@
  * @format
  */
 
-import React, {memo} from 'react';
+import React, {memo, useContext} from 'react';
 import styled from '@emotion/styled';
 import {theme} from 'flipper-plugin';
 import type {RenderContext} from './DataTable';
 import {Width} from '../../utils/widthUtils';
 import {pad} from 'lodash';
+import {DownCircleFilled} from '@ant-design/icons';
+import {Dropdown} from 'antd';
+import {TableContextMenuContext} from './TableContextMenu';
 
 // heuristic for row estimation, should match any future styling updates
 export const DEFAULT_ROW_HEIGHT = 24;
@@ -28,6 +31,18 @@ const backgroundColor = (props: TableBodyRowContainerProps) => {
   return undefined;
 };
 
+const CircleMargin = 4;
+const RowContextMenu = styled(DownCircleFilled)({
+  position: 'absolute',
+  top: CircleMargin,
+  right: CircleMargin,
+  fontSize: DEFAULT_ROW_HEIGHT - CircleMargin * 2,
+  borderRadius: (DEFAULT_ROW_HEIGHT - CircleMargin * 2) * 0.5,
+  color: theme.primaryColor,
+  cursor: 'pointer',
+  visibility: 'hidden',
+});
+
 const TableBodyRowContainer = styled.div<TableBodyRowContainerProps>(
   (props) => ({
     display: 'flex',
@@ -46,6 +61,10 @@ const TableBodyRowContainer = styled.div<TableBodyRowContainerProps>(
     overflow: 'hidden',
     width: '100%',
     flexShrink: 0,
+    [`&:hover ${RowContextMenu}`]: {
+      visibility: 'visible',
+      color: props.highlighted ? theme.white : undefined,
+    },
   }),
 );
 TableBodyRowContainer.displayName = 'TableRow:TableBodyRowContainer';
@@ -78,6 +97,8 @@ type Props = {
 
 export const TableRow = memo(function TableRow(props: Props) {
   const {config, highlighted, value: row} = props;
+  const menu = useContext(TableContextMenuContext);
+
   return (
     <TableBodyRowContainer
       highlighted={highlighted}
@@ -89,19 +110,9 @@ export const TableRow = memo(function TableRow(props: Props) {
       {config.columns
         .filter((col) => col.visible)
         .map((col) => {
-          let value = (col as any).onRender
+          const value = (col as any).onRender
             ? (col as any).onRender(row)
-            : (row as any)[col.key] ?? '';
-          if (typeof value === 'boolean') {
-            value = value ? 'true' : 'false';
-          }
-
-          if (value instanceof Date) {
-            value =
-              value.toTimeString().split(' ')[0] +
-              '.' +
-              pad('' + value.getMilliseconds(), 3);
-          }
+            : normalizeCellValue((row as any)[col.key]);
 
           return (
             <TableBodyColumnContainer
@@ -114,6 +125,40 @@ export const TableRow = memo(function TableRow(props: Props) {
             </TableBodyColumnContainer>
           );
         })}
+      {menu && (
+        <Dropdown
+          overlay={menu(row)}
+          placement="bottomRight"
+          trigger={['click', 'contextMenu']}>
+          <RowContextMenu />
+        </Dropdown>
+      )}
     </TableBodyRowContainer>
   );
 });
+
+export function normalizeCellValue(value: any): string {
+  switch (typeof value) {
+    case 'boolean':
+      return value ? 'true' : 'false';
+    case 'number':
+      return '' + value;
+    case 'undefined':
+      return '';
+    case 'string':
+      return value;
+    case 'object': {
+      if (value === null) return '';
+      if (value instanceof Date) {
+        return (
+          value.toTimeString().split(' ')[0] +
+          '.' +
+          pad('' + value.getMilliseconds(), 3)
+        );
+      }
+      return JSON.stringify(value, null, 2);
+    }
+    default:
+      return '<unrenderable value>';
+  }
+}
diff --git a/desktop/flipper-plugin/src/ui/datatable/__tests__/DataTable.node.tsx b/desktop/flipper-plugin/src/ui/datatable/__tests__/DataTable.node.tsx
index 112dc560e..7fa8032be 100644
--- a/desktop/flipper-plugin/src/ui/datatable/__tests__/DataTable.node.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/__tests__/DataTable.node.tsx
@@ -43,14 +43,19 @@ test('update and append', async () => {
   const ds = createTestDataSource();
   const ref = createRef<TableManager>();
   const rendering = render(
-    <DataTable dataSource={ds} columns={columns} tableManagerRef={ref} />,
+    <DataTable
+      dataSource={ds}
+      columns={columns}
+      tableManagerRef={ref}
+      _testHeight={400}
+    />,
   );
   {
     const elem = await rendering.findAllByText('test DataTable');
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-4f2ebr-TableBodyRowContainer efe0za01"
+        class="css-1rnoidw-TableBodyRowContainer efe0za01"
       >
         <div
           class="ant-table-cell css-1g4z4wd-TableBodyColumnContainer efe0za00"
@@ -95,14 +100,19 @@ test('column visibility', async () => {
   const ds = createTestDataSource();
   const ref = createRef<TableManager>();
   const rendering = render(
-    <DataTable dataSource={ds} columns={columns} tableManagerRef={ref} />,
+    <DataTable
+      dataSource={ds}
+      columns={columns}
+      tableManagerRef={ref}
+      _testHeight={400}
+    />,
   );
   {
     const elem = await rendering.findAllByText('test DataTable');
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-4f2ebr-TableBodyRowContainer efe0za01"
+        class="css-1rnoidw-TableBodyRowContainer efe0za01"
       >
         <div
           class="ant-table-cell css-1g4z4wd-TableBodyColumnContainer efe0za00"
@@ -127,7 +137,7 @@ test('column visibility', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-4f2ebr-TableBodyRowContainer efe0za01"
+        class="css-1rnoidw-TableBodyRowContainer efe0za01"
       >
         <div
           class="ant-table-cell css-1g4z4wd-TableBodyColumnContainer efe0za00"
diff --git a/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx b/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx
index 1b017fd33..ad5662b3a 100644
--- a/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx
+++ b/desktop/flipper-plugin/src/ui/datatable/useDataTableManager.tsx
@@ -43,19 +43,35 @@ export function useDataTableManager<T extends object>(
     [columns],
   );
 
-  const addColumnFilter = useCallback((columnId: string, value: string) => {
-    // TODO: fix typings
-    setEffectiveColumns(
-      produce((draft: DataTableColumn<any>[]) => {
-        const column = draft.find((c) => c.key === columnId)!;
-        column.filters!.push({
-          label: value,
-          value: value.toLowerCase(),
-          enabled: true,
-        });
-      }),
-    );
-  }, []);
+  const addColumnFilter = useCallback(
+    (columnId: string, value: string, disableOthers = false) => {
+      // TODO: fix typings
+      setEffectiveColumns(
+        produce((draft: DataTableColumn<any>[]) => {
+          const column = draft.find((c) => c.key === columnId)!;
+          const filterValue = value.toLowerCase();
+          const existing = column.filters!.find((c) => c.value === filterValue);
+          if (existing) {
+            existing.enabled = true;
+          } else {
+            column.filters!.push({
+              label: value,
+              value: filterValue,
+              enabled: true,
+            });
+          }
+          if (disableOthers) {
+            column.filters!.forEach((c) => {
+              if (c.value !== filterValue) {
+                c.enabled = false;
+              }
+            });
+          }
+        }),
+      );
+    },
+    [],
+  );
 
   const removeColumnFilter = useCallback((columnId: string, index: number) => {
     // TODO: fix typings