From 7e4df00138c3fd92488a8059d5439d34c571d6c9 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Thu, 10 Jun 2021 12:55:56 -0700
Subject: [PATCH] Implement context menu

Summary:
Re-introduced context menu to DataTable, due to popular demand.

Originally it wasn't there to better align with ant design principles, but in an app like Flipper it makes just too much sense to have it

See e.g. https://fb.workplace.com/groups/flippersupport/permalink/1138285579985432/

changelog: Restored context menu in data tables

Reviewed By: passy

Differential Revision: D28996137

fbshipit-source-id: 16ef4c90997c9313efa62da7576fd453a7853761
---
 desktop/flipper-plugin/src/ui/DataList.tsx    |  2 ++
 .../ui/data-inspector/DataInspectorNode.tsx   |  2 +-
 .../src/ui/data-table/DataTable.tsx           | 22 ++++++++++++++++---
 .../src/ui/data-table/TableContextMenu.tsx    | 19 +++++++++-------
 .../src/ui/data-table/TableRow.tsx            | 13 ++++++++++-
 .../data-table/__tests__/DataTable.node.tsx   |  6 ++---
 .../__tests__/DataTableRecords.node.tsx       |  2 +-
 .../__tests__/flipper_messages.node.tsx       |  2 +-
 8 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/desktop/flipper-plugin/src/ui/DataList.tsx b/desktop/flipper-plugin/src/ui/DataList.tsx
index 8db683eba..b8322eb28 100644
--- a/desktop/flipper-plugin/src/ui/DataList.tsx
+++ b/desktop/flipper-plugin/src/ui/DataList.tsx
@@ -169,6 +169,8 @@ DataList.defaultProps = {
   enableSearchbar: false,
   enableColumnHeaders: false,
   enableArrow: true,
+  enableContextMenu: false,
+  enableMultiSelect: false,
 };
 
 const DataListItem = memo(
diff --git a/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx b/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx
index f2671a70b..cc361faed 100644
--- a/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx
+++ b/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx
@@ -32,7 +32,7 @@ export {DataValueExtractor} from './DataPreview';
 
 export const RootDataContext = createContext<() => any>(() => ({}));
 
-const contextMenuTrigger = ['contextMenu' as const];
+export const contextMenuTrigger = ['contextMenu' as const];
 
 const BaseContainer = styled.div<{depth?: number; disabled?: boolean}>(
   (props) => ({
diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx
index eaf641cf4..f801866da 100644
--- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx
@@ -59,6 +59,7 @@ interface DataTableBaseProps<T = any> {
   enableAutoScroll?: boolean;
   enableColumnHeaders?: boolean;
   enableMultiSelect?: boolean;
+  enableContextMenu?: boolean;
   // if set (the default) will grow and become scrollable. Otherwise will use natural size
   scrollable?: boolean;
   extraActions?: React.ReactElement;
@@ -119,6 +120,7 @@ export interface TableRowRenderContext<T = any> {
     itemId: number,
   ): void;
   onRowStyle?(item: T): React.CSSProperties | undefined;
+  onContextMenu?(): React.ReactElement;
 }
 
 export type DataTableProps<T> = DataTableInput<T> & DataTableBaseProps<T>;
@@ -185,7 +187,10 @@ export function DataTable<T extends object>(
       },
       onMouseDown(e, _item, index) {
         if (!dragging.current) {
-          if (e.ctrlKey || e.metaKey) {
+          if (e.buttons > 1) {
+            // for right click we only want to add if needed, not deselect
+            tableManager.addRangeToSelection(index, index, false);
+          } else if (e.ctrlKey || e.metaKey) {
             tableManager.addRangeToSelection(index, index, true);
           } else if (e.shiftKey) {
             tableManager.selectItem(index, true, true);
@@ -205,8 +210,15 @@ export function DataTable<T extends object>(
         }
       },
       onRowStyle,
+      onContextMenu: props.enableContextMenu
+        ? () => {
+            // using a ref keeps the config stable, so that a new context menu doesn't need
+            // all rows to be rerendered, but rather shows it conditionally
+            return contextMenuRef.current?.()!;
+          }
+        : undefined,
     };
-  }, [visibleColumns, tableManager, onRowStyle]);
+  }, [visibleColumns, tableManager, onRowStyle, props.enableContextMenu]);
 
   const itemRenderer = useCallback(
     function itemRenderer(
@@ -415,6 +427,9 @@ export function DataTable<T extends object>(
         ],
       );
 
+  const contextMenuRef = useRef(contexMenu);
+  contextMenuRef.current = contexMenu;
+
   useEffect(function initialSetup() {
     return function cleanup() {
       // write current prefs to local storage
@@ -519,7 +534,8 @@ DataTable.defaultProps = {
   enableSearchbar: true,
   enableAutoScroll: false,
   enableColumnHeaders: true,
-  eanbleMultiSelect: true,
+  enableMultiSelect: true,
+  enableContextMenu: true,
   onRenderEmpty: emptyRenderer,
 } as Partial<DataTableProps<any>>;
 
diff --git a/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx b/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx
index 22eac5211..5a030c9e2 100644
--- a/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/TableContextMenu.tsx
@@ -45,19 +45,19 @@ export function tableContextMenuFactory<T>(
     );
   }
   const hasSelection = selection.items.size > 0 ?? false;
-
   return (
     <Menu>
       {onContextMenu
         ? onContextMenu(getSelectedItem(datasource, selection))
         : null}
       <SubMenu
+        key="filter same"
         title="Filter on same"
         icon={<FilterOutlined />}
         disabled={!hasSelection}>
-        {visibleColumns.map((column) => (
+        {visibleColumns.map((column, idx) => (
           <Item
-            key={column.key}
+            key={column.key ?? idx}
             onClick={() => {
               dispatch({
                 type: 'setColumnFilterFromSelection',
@@ -69,12 +69,13 @@ export function tableContextMenuFactory<T>(
         ))}
       </SubMenu>
       <SubMenu
+        key="copy cells"
         title="Copy cell(s)"
         icon={<CopyOutlined />}
         disabled={!hasSelection}>
-        {visibleColumns.map((column) => (
+        {visibleColumns.map((column, idx) => (
           <Item
-            key={column.key}
+            key={column.key ?? idx}
             onClick={() => {
               const items = getSelectedItems(datasource, selection);
               if (items.length) {
@@ -88,6 +89,7 @@ export function tableContextMenuFactory<T>(
         ))}
       </SubMenu>
       <Item
+        key="copyToClipboard"
         disabled={!hasSelection}
         onClick={() => {
           const items = getSelectedItems(datasource, selection);
@@ -99,6 +101,7 @@ export function tableContextMenuFactory<T>(
       </Item>
       {lib.isFB && (
         <Item
+          key="createPaste"
           disabled={!hasSelection}
           onClick={() => {
             const items = getSelectedItems(datasource, selection);
@@ -110,9 +113,9 @@ export function tableContextMenuFactory<T>(
         </Item>
       )}
       <Menu.Divider />
-      <SubMenu title="Visible columns">
-        {columns.map((column) => (
-          <Menu.Item key={column.key}>
+      <SubMenu title="Visible columns" key="visible columns">
+        {columns.map((column, idx) => (
+          <Menu.Item key={column.key ?? idx}>
             <Checkbox
               checked={column.visible}
               onClick={(e) => {
diff --git a/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx b/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx
index 59d43958f..00e804409 100644
--- a/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/TableRow.tsx
@@ -13,6 +13,8 @@ import {theme} from '../theme';
 import type {TableRowRenderContext} from './DataTable';
 import {Width} from '../../utils/widthUtils';
 import {DataFormatter} from '../DataFormatter';
+import {Dropdown} from 'antd';
+import {contextMenuTrigger} from '../data-inspector/DataInspectorNode';
 
 // heuristic for row estimation, should match any future styling updates
 export const DEFAULT_ROW_HEIGHT = 24;
@@ -106,7 +108,7 @@ export const TableRow = memo(function TableRow<T>({
   highlighted,
   config,
 }: TableRowProps<T>) {
-  return (
+  const row = (
     <TableBodyRowContainer
       highlighted={highlighted}
       onMouseDown={(e) => {
@@ -135,4 +137,13 @@ export const TableRow = memo(function TableRow<T>({
         })}
     </TableBodyRowContainer>
   );
+  if (config.onContextMenu) {
+    return (
+      <Dropdown overlay={config.onContextMenu} trigger={contextMenuTrigger}>
+        {row}
+      </Dropdown>
+    );
+  } else {
+    return row;
+  }
 });
diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx
index 98feeaa6b..5ad3712c7 100644
--- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx
@@ -50,7 +50,7 @@ test('update and append', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
+        class="ant-dropdown-trigger css-1k3kr6b-TableBodyRowContainer e1luu51r1"
       >
         <div
           class="css-9bipfg-TableBodyColumnContainer e1luu51r0"
@@ -104,7 +104,7 @@ test('column visibility', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
+        class="ant-dropdown-trigger css-1k3kr6b-TableBodyRowContainer e1luu51r1"
       >
         <div
           class="css-9bipfg-TableBodyColumnContainer e1luu51r0"
@@ -131,7 +131,7 @@ test('column visibility', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
+        class="ant-dropdown-trigger css-1k3kr6b-TableBodyRowContainer e1luu51r1"
       >
         <div
           class="css-9bipfg-TableBodyColumnContainer e1luu51r0"
diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx
index e3b12c857..184bcff0a 100644
--- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx
+++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx
@@ -44,7 +44,7 @@ test('update and append', async () => {
     expect(elem.length).toBe(1);
     expect(elem[0].parentElement).toMatchInlineSnapshot(`
       <div
-        class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
+        class="ant-dropdown-trigger css-1k3kr6b-TableBodyRowContainer e1luu51r1"
       >
         <div
           class="css-9bipfg-TableBodyColumnContainer e1luu51r0"
diff --git a/desktop/plugins/public/flipper-messages/__tests__/flipper_messages.node.tsx b/desktop/plugins/public/flipper-messages/__tests__/flipper_messages.node.tsx
index cb8e725d5..9c89f3301 100644
--- a/desktop/plugins/public/flipper-messages/__tests__/flipper_messages.node.tsx
+++ b/desktop/plugins/public/flipper-messages/__tests__/flipper_messages.node.tsx
@@ -116,7 +116,7 @@ test('It can render rows', async () => {
   expect((await renderer.findByText('unique-string')).parentElement)
     .toMatchInlineSnapshot(`
     <div
-      class="css-1k3kr6b-TableBodyRowContainer e1luu51r1"
+      class="ant-dropdown-trigger css-1k3kr6b-TableBodyRowContainer e1luu51r1"
     >
       <div
         class="css-1vr131n-TableBodyColumnContainer e1luu51r0"