Connections: Avoid creating component during rendering & remove non-reusable hook

web/packages/teleterm/src/ui/TopBar/Connections/Connections.test.tsx

@@ -16,16 +16,20 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { render, screen, userEvent } from 'design/utils/testing';
+import { act, render, screen, userEvent } from 'design/utils/testing';
 
 import Logger, { NullService } from 'teleterm/logger';
 import { VnetContextProvider } from 'teleterm/ui/Vnet';
 import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider';
 import { MockAppContext } from 'teleterm/ui/fixtures/mocks';
-
 import { makeRootCluster } from 'teleterm/services/tshd/testHelpers';
+import {
+  DocumentTshNode,
+  Workspace,
+} from 'teleterm/ui/services/workspacesService';
 
-import { Workspace } from 'teleterm/ui/services/workspacesService';
+import { routing } from 'teleterm/ui/uri';
+import { unique } from 'teleterm/ui/utils';
 
 import { Connections } from './Connections';
 
@@ -109,7 +113,7 @@ describe('opening a connection', () => {
       serverId: 'foo',
       login: 'alice',
       title: 'alice@foo',
-      uri: '/docs/123',
+      uri: routing.getDocUri({ docId: unique() }),
     };
     appContext.workspacesService.setState(draft => {
       draft.workspaces[cluster.uri] = {
@@ -137,3 +141,138 @@ describe('opening a connection', () => {
     expect(workspace.location).toEqual(doc.uri);
   });
 });
+
+test('adding a new conn while the list is open puts the new conn at the top', async () => {
+  const user = userEvent.setup();
+  const appContext = new MockAppContext();
+  const cluster = makeRootCluster();
+  appContext.clustersService.setState(draft => {
+    draft.clusters.set(cluster.uri, cluster);
+  });
+  const doc: DocumentTshNode = {
+    kind: 'doc.terminal_tsh_node',
+    origin: 'search_bar',
+    rootClusterId: cluster.name,
+    leafClusterId: undefined,
+    serverUri: `${cluster.uri}/servers/foo`,
+    serverId: 'foo',
+    login: 'alice',
+    title: 'alice@foo',
+    uri: routing.getDocUri({ docId: unique() }),
+    status: 'connected',
+  };
+  appContext.workspacesService.setState(draft => {
+    draft.workspaces[cluster.uri] = {
+      documents: [doc],
+      location: undefined,
+    } as Workspace;
+  });
+
+  render(
+    <MockAppContextProvider appContext={appContext}>
+      <VnetContextProvider>
+        <Connections />
+      </VnetContextProvider>
+    </MockAppContextProvider>
+  );
+
+  await user.click(screen.getByTitle(/Open Connections/));
+  expect(screen.getByText(doc.title)).toBeInTheDocument();
+
+  const newDoc: DocumentTshNode = {
+    kind: 'doc.terminal_tsh_node',
+    origin: 'search_bar',
+    rootClusterId: cluster.name,
+    leafClusterId: undefined,
+    serverUri: `${cluster.uri}/servers/bar`,
+    serverId: 'bar',
+    login: 'alice',
+    title: 'alice@bar',
+    uri: routing.getDocUri({ docId: unique() }),
+    status: 'connected',
+  };
+
+  act(() => {
+    appContext.workspacesService.setState(draft => {
+      draft.workspaces[cluster.uri].documents.push(newDoc);
+    });
+  });
+
+  const oldConn = screen.getByText(doc.title);
+  const newConn = screen.getByText(newDoc.title);
+  // https://developer.mozilla.org/en-US/docs/Web/API/Node/compareDocumentPosition
+  expect(
+    newConn.compareDocumentPosition(oldConn) & Node.DOCUMENT_POSITION_FOLLOWING
+  ).toBeTruthy();
+});
+
+test('disconnecting a conn does not update its position in the list', async () => {
+  const user = userEvent.setup();
+  const appContext = new MockAppContext();
+  const cluster = makeRootCluster();
+  appContext.clustersService.setState(draft => {
+    draft.clusters.set(cluster.uri, cluster);
+  });
+  const doc: DocumentTshNode = {
+    kind: 'doc.terminal_tsh_node',
+    origin: 'search_bar',
+    rootClusterId: cluster.name,
+    leafClusterId: undefined,
+    serverUri: `${cluster.uri}/servers/foo`,
+    serverId: 'foo',
+    login: 'alice',
+    title: 'alice@foo',
+    uri: routing.getDocUri({ docId: unique() }),
+    status: 'connected',
+  };
+  const docToBeClosed: DocumentTshNode = {
+    kind: 'doc.terminal_tsh_node',
+    origin: 'search_bar',
+    rootClusterId: cluster.name,
+    leafClusterId: undefined,
+    serverUri: `${cluster.uri}/servers/bar`,
+    serverId: 'bar',
+    login: 'alice',
+    title: 'alice@bar',
+    uri: routing.getDocUri({ docId: unique() }),
+    status: 'connected',
+  };
+  appContext.workspacesService.setState(draft => {
+    draft.workspaces[cluster.uri] = {
+      documents: [docToBeClosed, doc],
+      location: undefined,
+    } as Workspace;
+  });
+
+  render(
+    <MockAppContextProvider appContext={appContext}>
+      <VnetContextProvider>
+        <Connections />
+      </VnetContextProvider>
+    </MockAppContextProvider>
+  );
+
+  await user.click(screen.getByTitle(/Open Connections/));
+  const conn = () => screen.getByText(doc.title);
+  const connToBeClosed = () => screen.getByText(docToBeClosed.title);
+
+  // https://developer.mozilla.org/en-US/docs/Web/API/Node/compareDocumentPosition
+  expect(
+    connToBeClosed().compareDocumentPosition(conn()) &
+      Node.DOCUMENT_POSITION_FOLLOWING
+  ).toBeTruthy();
+
+  const disconnectButton = screen.getByTitle(
+    `Disconnect ${docToBeClosed.title}`
+  );
+  await user.click(disconnectButton);
+
+  expect(
+    await screen.findByTitle(`Remove ${docToBeClosed.title}`)
+  ).toBeInTheDocument();
+
+  expect(
+    connToBeClosed().compareDocumentPosition(conn()) &
+      Node.DOCUMENT_POSITION_FOLLOWING
+  ).toBeTruthy();
+});

web/packages/teleterm/src/ui/TopBar/Connections/Connections.tsx

@@ -19,33 +19,27 @@
 import { useCallback, useMemo, useRef, useState } from 'react';
 import Popover from 'design/Popover';
 import { Box, StepSlider } from 'design';
-import { StepComponentProps } from 'design/StepSlider';
 
 import { useKeyboardShortcuts } from 'teleterm/ui/services/keyboardShortcuts';
-import { KeyboardArrowsNavigation } from 'teleterm/ui/components/KeyboardArrowsNavigation';
 import { VnetSliderStep, useVnetContext } from 'teleterm/ui/Vnet';
+import { useAppContext } from 'teleterm/ui/appContextProvider';
 
-import { useConnections } from './useConnections';
 import { ConnectionsIcon } from './ConnectionsIcon/ConnectionsIcon';
-import { ConnectionsFilterableList } from './ConnectionsFilterableList/ConnectionsFilterableList';
+import { ConnectionsSliderStep } from './ConnectionsSliderStep';
 
 export function Connections() {
+  const { connectionTracker } = useAppContext();
+  connectionTracker.useState();
   const iconRef = useRef();
   const [isPopoverOpened, setIsPopoverOpened] = useState(false);
-  const connections = useConnections();
   const { status: vnetStatus } = useVnetContext();
   const isAnyConnectionActive =
-    connections.isAnyConnectionActive || vnetStatus === 'running';
+    connectionTracker.getConnections().some(c => c.connected) ||
+    vnetStatus === 'running';
 
   const togglePopover = useCallback(() => {
-    setIsPopoverOpened(wasOpened => {
-      const isOpened = !wasOpened;
-      if (isOpened) {
-        connections.updateSorting();
-      }
-      return isOpened;
-    });
-  }, [setIsPopoverOpened, connections.updateSorting]);
+    setIsPopoverOpened(wasOpened => !wasOpened);
+  }, []);
 
   useKeyboardShortcuts(
     useMemo(
@@ -56,9 +50,8 @@ export function Connections() {
     )
   );
 
-  function activateItem(id: string): void {
+  function closeConnectionList(): void {
     setIsPopoverOpened(false);
-    connections.activateItem(id);
   }
 
   // TODO(ravicious): Investigate the problem with height getting temporarily reduced when switching
@@ -69,22 +62,6 @@ export function Connections() {
   //
   // We aim to replace the sliding animation with an expanding animation before the release, so it
   // might not be worth the effort.
-  const sliderSteps = [
-    (props: StepComponentProps) => (
-      <Box p={2} ref={props.refCallback}>
-        <KeyboardArrowsNavigation>
-          <ConnectionsFilterableList
-            items={connections.items}
-            onActivateItem={activateItem}
-            onRemoveItem={connections.removeItem}
-            onDisconnectItem={connections.disconnectItem}
-            slideToVnet={props.next}
-          />
-        </KeyboardArrowsNavigation>
-      </Box>
-    ),
-    VnetSliderStep,
-  ];
 
   return (
     <>
@@ -104,9 +81,16 @@ export function Connections() {
           padding (so 24px on both sides) and ConnectionsFilterableList had 300px of width.
         */}
         <Box width="324px" bg="levels.elevated">
-          <StepSlider currFlow="default" flows={{ default: sliderSteps }} />
+          <StepSlider
+            currFlow="default"
+            flows={stepSliderFlows}
+            // The rest of the props is spread to each individual step component.
+            closeConnectionList={closeConnectionList}
+          />
         </Box>
       </Popover>
     </>
   );
 }
+
+const stepSliderFlows = { default: [ConnectionsSliderStep, VnetSliderStep] };

web/packages/teleterm/src/ui/TopBar/Connections/ConnectionsFilterableList/ConnectionItem.tsx

@@ -32,25 +32,25 @@ export function ConnectionItem(props: {
   index: number;
   item: ExtendedTrackedConnection;
   showClusterName: boolean;
-  onActivate(): void;
-  onRemove(): void;
-  onDisconnect(): void;
+  activate(): void;
+  remove(): void;
+  disconnect(): void;
 }) {
   const offline = !props.item.connected;
   const { isActive, scrollIntoViewIfActive } = useKeyboardArrowsNavigation({
     index: props.index,
-    onRun: props.onActivate,
+    onRun: props.activate,
   });
 
   const actionIcons = {
     disconnect: {
-      title: 'Disconnect',
-      action: props.onDisconnect,
+      title: `Disconnect ${props.item.title}`,
+      action: props.disconnect,
       Icon: Unlink,
     },
     remove: {
-      title: 'Remove',
-      action: props.onRemove,
+      title: `Remove ${props.item.title}`,
+      action: props.remove,
       Icon: Trash,
     },
   };
@@ -64,7 +64,7 @@ export function ConnectionItem(props: {
 
   return (
     <ListItem
-      onClick={props.onActivate}
+      onClick={props.activate}
       isActive={isActive}
       ref={ref}
       $showClusterName={props.showClusterName}

web/packages/teleterm/src/ui/TopBar/Connections/ConnectionsFilterableList/ConnectionsFilterableList.tsx

@@ -28,9 +28,9 @@ import { ConnectionItem } from './ConnectionItem';
 
 export function ConnectionsFilterableList(props: {
   items: ExtendedTrackedConnection[];
-  onActivateItem(id: string): void;
-  onRemoveItem(id: string): void;
-  onDisconnectItem(id: string): void;
+  activateItem(id: string): void;
+  removeItem(id: string): void;
+  disconnectItem(id: string): void;
   slideToVnet(): void;
 }) {
   const { setActiveIndex } = useKeyboardArrowsNavigationStateUpdate();
@@ -80,9 +80,9 @@ export function ConnectionsFilterableList(props: {
             item={item}
             index={index}
             showClusterName={showClusterName}
-            onActivate={() => props.onActivateItem(item.id)}
-            onRemove={() => props.onRemoveItem(item.id)}
-            onDisconnect={() => props.onDisconnectItem(item.id)}
+            activate={() => props.activateItem(item.id)}
+            remove={() => props.removeItem(item.id)}
+            disconnect={() => props.disconnectItem(item.id)}
           />
         )
       }