Network Plugin - fix highlighting issues in routes list (#1917)

Summary:
Fix problems with row highlighting on mocks route list.  When selecting a row, the highlight would sometimes be on the wrong row.  Also, when adding routes, the routes would sometimes get added in the middle of the list instead of at the end.

These problems were caused by incorrectly referencing route rows in code.  Sometimes index was used and other times the key was used.  Fixed by consistently using key.

This fixes the problem described in issue https://github.com/facebook/flipper/issues/1733

## Changelog

Network Plugin - fix problems with row highlighting on mocks route list

Pull Request resolved: https://github.com/facebook/flipper/pull/1917

Test Plan:
Start with no mocks | No rows should be shown | TRUE
Add a single mock | First (and only row should be highlighted) | TRUE
Delete mock | No rows should be shown | TRUE
Add single mockAdd another mock | 2nd row should be highlighted | TRUE
Add another mock | 3rd row should be highlighted | TRUE
Add 2 more mocks and assign number to each row (5 total) | Last row should be highlighted | TRUE
Select 1st row | 1st row should be highlighted | TRUE
Delete 1st row | 1st row should be highlighed | TRUE
Select last row | last row should be highlighted | TRUE
Delete last row | last row should be deleted and new last row should be highlighted | TRUE
Highlight a middle row (row 3) | middle row should be highlighted | TRUE
Delete middle row | middle row should be deleted and last row should be in its place and should be highlighted | TRUE

Reviewed By: passy

Differential Revision: D26604455

Pulled By: mweststrate

fbshipit-source-id: f29690244e32b364983089541718f4bc75154dd1

desktop/plugins/network/ManageMockResponsePanel.tsx

@@ -24,7 +24,7 @@ import {MockResponseDetails} from './MockResponseDetails';
 import {NetworkRouteContext} from './index';
 import {RequestId} from './types';
 
-import {message} from 'antd';
+import {message, Modal} from 'antd';
 import {NUX, Layout} from 'flipper-plugin';
 
 type Props = {
@@ -101,23 +101,25 @@ function RouteRow(props: {
   handleRemoveId: () => void;
 }) {
   return (
-    <Layout.Horizontal>
-      <Layout.Horizontal onClick={props.handleRemoveId}>
-        <Icon name="cross-circle" color={colors.red} />
+    <Layout.Container>
+      <Layout.Horizontal style={{paddingBottom: 20}}>
+        <Layout.Horizontal onClick={props.handleRemoveId}>
+          <Icon name="cross-circle" color={colors.red} />
+        </Layout.Horizontal>
+        <Layout.Horizontal>
+          {props.showWarning && (
+            <Icon name="caution-triangle" color={colors.yellow} />
+          )}
+          {props.text.length === 0 ? (
+            <TextEllipsis style={{color: colors.blackAlpha50}}>
+              untitled
+            </TextEllipsis>
+          ) : (
+            <TextEllipsis>{props.text}</TextEllipsis>
+          )}
+        </Layout.Horizontal>
       </Layout.Horizontal>
-      <Layout.Horizontal>
-        {props.showWarning && (
-          <Icon name="caution-triangle" color={colors.yellow} />
-        )}
-        {props.text.length === 0 ? (
-          <TextEllipsis style={{color: colors.blackAlpha50}}>
-            untitled
-          </TextEllipsis>
-        ) : (
-          <TextEllipsis>{props.text}</TextEllipsis>
-        )}
-      </Layout.Horizontal>
-    </Layout.Horizontal>
+    </Layout.Container>
   );
 }
 
@@ -150,16 +152,57 @@ export function ManageMockResponsePanel(props: Props) {
   useEffect(() => {
     setSelectedId((selectedId) => {
       const keys = Object.keys(props.routes);
-      return keys.length === 0
-        ? null
-        : selectedId === null || !keys.includes(selectedId)
-        ? keys[keys.length - 1]
-        : selectedId;
+      let returnValue: string | null = null;
+      // selectId is null when there are no rows or it is the first time rows are shown
+      if (selectedId === null) {
+        if (keys.length === 0) {
+          // there are no rows
+          returnValue = null;
+        } else {
+          // first time rows are shown
+          returnValue = keys[0];
+        }
+      } else {
+        if (keys.includes(selectedId)) {
+          returnValue = selectedId;
+        } else {
+          // selectedId row value not in routes so default to first line
+          returnValue = keys[0];
+        }
+      }
+      return returnValue;
     });
   }, [props.routes]);
   const duplicatedIds = useMemo(() => _duplicateIds(props.routes), [
     props.routes,
   ]);
+
+  function getSelectedIds(): Set<string> {
+    const newSet = new Set<string>();
+    newSet.add(selectedId ?? '');
+    return newSet;
+  }
+
+  function getPreviousId(id: string): string | null {
+    const keys = Object.keys(props.routes);
+    const currentIndex = keys.indexOf(id);
+    if (currentIndex == 0) {
+      return null;
+    } else {
+      return keys[currentIndex - 1];
+    }
+  }
+
+  function getNextId(id: string): string | null {
+    const keys = Object.keys(props.routes);
+    const currentIndex = keys.indexOf(id);
+    if (currentIndex >= keys.length - 1) {
+      return getPreviousId(id);
+    } else {
+      return keys[currentIndex + 1];
+    }
+  }
+
   return (
     <Layout.Container style={{height: 550}}>
       <Layout.Left>
@@ -167,7 +210,8 @@ export function ManageMockResponsePanel(props: Props) {
           <Layout.Horizontal gap>
             <Button
               onClick={() => {
-                networkRouteManager.addRoute();
+                const newId = networkRouteManager.addRoute();
+                setSelectedId(newId);
               }}>
               Add Route
             </Button>
@@ -204,9 +248,18 @@ export function ManageMockResponsePanel(props: Props) {
               multiline={false}
               columnSizes={ColumnSizes}
               columns={Columns}
+              rowLineHeight={26}
               rows={_buildRows(props.routes, duplicatedIds, (id) => {
-                networkRouteManager.removeRoute(id);
-                setSelectedId(null);
+                Modal.confirm({
+                  title: 'Are you sure you want to delete this item?',
+                  icon: '',
+                  onOk() {
+                    const nextId = getNextId(id);
+                    networkRouteManager.removeRoute(id);
+                    setSelectedId(nextId);
+                  },
+                  onCancel() {},
+                });
               })}
               stickyBottom={true}
               autoHeight={false}
@@ -217,7 +270,7 @@ export function ManageMockResponsePanel(props: Props) {
                   selectedIds.length === 1 ? selectedIds[0] : null;
                 setSelectedId(newSelectedId);
               }}
-              highlightedRows={new Set(selectedId)}
+              highlightedRows={getSelectedIds()}
             />
           </Panel>
         </Layout.Container>

desktop/plugins/network/index.tsx

@@ -124,7 +124,7 @@ const TextEllipsis = styled(Text)({
 
 // State management
 export interface NetworkRouteManager {
-  addRoute(): void;
+  addRoute(): string | null;
   modifyRoute(id: string, routeChange: Partial<Route>): void;
   removeRoute(id: string): void;
   copyHighlightedCalls(
@@ -137,7 +137,9 @@ export interface NetworkRouteManager {
   clearRoutes(): void;
 }
 const nullNetworkRouteManager: NetworkRouteManager = {
-  addRoute() {},
+  addRoute(): string | null {
+    return '';
+  },
   modifyRoute(_id: string, _routeChange: Partial<Route>) {},
   removeRoute(_id: string) {},
   copyHighlightedCalls(
@@ -330,14 +332,14 @@ export function plugin(client: PluginClient<Events, Methods>) {
       routes.set(newRoutes);
       isMockResponseSupported.set(result);
       showMockResponseDialog.set(false);
-      nextRouteId.set(Object.keys(routes).length);
+      nextRouteId.set(Object.keys(routes.get()).length);
 
       informClientMockChange(routes.get());
     });
 
     // declare new variable to be called inside the interface
     networkRouteManager.set({
-      addRoute() {
+      addRoute(): string | null {
         const newNextRouteId = nextRouteId.get();
         routes.update((draft) => {
           draft[newNextRouteId.toString()] = {
@@ -349,6 +351,7 @@ export function plugin(client: PluginClient<Events, Methods>) {
           };
         });
         nextRouteId.set(newNextRouteId + 1);
+        return String(newNextRouteId);
       },
       modifyRoute(id: string, routeChange: Partial<Route>) {
         if (!routes.get().hasOwnProperty(id)) {