From 29d16feed596e51200431717b4cb14a59f0ceb20 Mon Sep 17 00:00:00 2001 From: bizzguy Date: Fri, 26 Feb 2021 07:17:00 -0800 Subject: [PATCH] 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 --- .../network/ManageMockResponsePanel.tsx | 105 +++++++++++++----- desktop/plugins/network/index.tsx | 11 +- 2 files changed, 86 insertions(+), 30 deletions(-) diff --git a/desktop/plugins/network/ManageMockResponsePanel.tsx b/desktop/plugins/network/ManageMockResponsePanel.tsx index 564173588..68b58a214 100644 --- a/desktop/plugins/network/ManageMockResponsePanel.tsx +++ b/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 ( - - - + + + + + + + {props.showWarning && ( + + )} + {props.text.length === 0 ? ( + + untitled + + ) : ( + {props.text} + )} + - - {props.showWarning && ( - - )} - {props.text.length === 0 ? ( - - untitled - - ) : ( - {props.text} - )} - - + ); } @@ -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 { + const newSet = new Set(); + 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 ( @@ -167,7 +210,8 @@ export function ManageMockResponsePanel(props: Props) { @@ -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()} /> diff --git a/desktop/plugins/network/index.tsx b/desktop/plugins/network/index.tsx index a762e9c09..fa8c0e949 100644 --- a/desktop/plugins/network/index.tsx +++ b/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): 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) {}, removeRoute(_id: string) {}, copyHighlightedCalls( @@ -330,14 +332,14 @@ export function plugin(client: PluginClient) { 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) { }; }); nextRouteId.set(newNextRouteId + 1); + return String(newNextRouteId); }, modifyRoute(id: string, routeChange: Partial) { if (!routes.get().hasOwnProperty(id)) {