Merge branch 'add-list-component'

6 files changed, 310 insertions, 51 deletions

diff --git a/gui/src/renderer/components/Accordion.tsx b/gui/src/renderer/components/Accordion.tsx
index a9b07a7493..87dfdede24 100644
--- a/gui/src/renderer/components/Accordion.tsx
+++ b/gui/src/renderer/components/Accordion.tsx
@@ -104,11 +104,13 @@ export default class Accordion extends React.Component<IProps, IState> {
     }
   }
 
-  private onTransitionEnd = () => {
-    this.props.onTransitionEnd?.();
-    if (this.props.expanded) {
-      // Height auto enables the container to grow if the content changes size
-      this.setState({ containerHeight: 'auto' });
+  private onTransitionEnd = (event: React.TransitionEvent<HTMLDivElement>) => {
+    if (event.target === this.containerRef.current) {
+      this.props.onTransitionEnd?.();
+      if (this.props.expanded) {
+        // Height auto enables the container to grow if the content changes size
+        this.setState({ containerHeight: 'auto' });
+      }
     }
   };
 }
diff --git a/gui/src/renderer/components/CustomDnsSettings.tsx b/gui/src/renderer/components/CustomDnsSettings.tsx
index a6f409ea5c..c0d1a66f5c 100644
--- a/gui/src/renderer/components/CustomDnsSettings.tsx
+++ b/gui/src/renderer/components/CustomDnsSettings.tsx
@@ -1,10 +1,10 @@
-import React, { useCallback, useMemo, useRef, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { sprintf } from 'sprintf-js';
 import { colors } from '../../config.json';
 import { messages } from '../../shared/gettext';
 import { useAppContext } from '../context';
 import { IpAddress } from '../lib/ip';
-import { useBoolean } from '../lib/utilityHooks';
+import { useBoolean, useMounted } from '../lib/utilityHooks';
 import { formatMarkdown } from '../markdown-formatter';
 import { useSelector } from '../redux/store';
 import Accordion from './Accordion';
@@ -29,6 +29,7 @@ import {
   StyledRemoveButton,
   StyledRemoveIcon,
 } from './CustomDnsSettingsStyles';
+import List, { stringValueAsKey } from './List';
 import { ModalAlert, ModalAlertType } from './Modal';
 
 export default function CustomDnsSettings() {
@@ -38,6 +39,7 @@ export default function CustomDnsSettings() {
   const [inputVisible, showInput, hideInput] = useBoolean(false);
   const [invalid, setInvalid, setValid] = useBoolean(false);
   const [confirmAction, setConfirmAction] = useState<() => Promise<void>>();
+  const [savingEdit, setSavingEdit] = useState(false);
   const willShowConfirmationDialog = useRef(false);
 
   const featureAvailable = useMemo(
@@ -135,6 +137,8 @@ export default function CustomDnsSettings() {
       }
 
       const edit = async () => {
+        setSavingEdit(true);
+
         const addresses = dns.customOptions.addresses.map((address) =>
           oldAddress === address ? newAddress : address,
         );
@@ -177,6 +181,8 @@ export default function CustomDnsSettings() {
     [dns, setDnsOptions],
   );
 
+  useEffect(() => setSavingEdit(false), [dns.customOptions.addresses]);
+
   return (
     <>
       <StyledCustomDnsSwitchContainer disabled={!featureAvailable}>
@@ -197,17 +203,20 @@ export default function CustomDnsSettings() {
       </StyledCustomDnsSwitchContainer>
       <Accordion expanded={featureAvailable && (dns.state === 'custom' || inputVisible)}>
         <Cell.Section role="listbox">
-          {dns.customOptions.addresses.map((item, i) => {
-            return (
+          <List
+            items={dns.customOptions.addresses}
+            getKey={stringValueAsKey}
+            skipAddTransition={true}
+            skipRemoveTransition={savingEdit}>
+            {(item) => (
               <CellListItem
-                key={i}
                 onRemove={onRemove}
                 onChange={onEdit}
                 willShowConfirmationDialog={willShowConfirmationDialog}>
                 {item}
               </CellListItem>
-            );
-          })}
+            )}
+          </List>
         </Cell.Section>
 
         {inputVisible && (
@@ -294,6 +303,7 @@ interface ICellListItemProps {
 function CellListItem(props: ICellListItemProps) {
   const [editing, startEditing, stopEditing] = useBoolean(false);
   const [invalid, setInvalid, setValid] = useBoolean(false);
+  const isMounted = useMounted();
 
   const inputContainerRef = useRef() as React.RefObject<HTMLDivElement>;
 
@@ -309,7 +319,9 @@ function CellListItem(props: ICellListItemProps) {
       } else {
         try {
           await props.onChange(props.children, value);
-          stopEditing();
+          if (isMounted()) {
+            stopEditing();
+          }
         } catch {
           setInvalid();
         }
diff --git a/gui/src/renderer/components/List.tsx b/gui/src/renderer/components/List.tsx
new file mode 100644
index 0000000000..a0d7a3ec5c
--- /dev/null
+++ b/gui/src/renderer/components/List.tsx
@@ -0,0 +1,148 @@
+import { useCallback, useEffect, useRef, useState } from 'react';
+import Accordion from './Accordion';
+
+export const stringValueAsKey = (value: string): string => value;
+
+interface ListProps<T> {
+  items: Array<T>;
+  getKey: (data: T) => string;
+  children: (data: T) => React.ReactNode;
+  skipAddTransition?: boolean;
+  skipInitialAddTransition?: boolean;
+  skipRemoveTransition?: boolean;
+}
+
+export interface RowData<T> {
+  key: string;
+  data: T;
+}
+
+export interface RowDisplayData<T> extends RowData<T> {
+  removing: boolean;
+}
+
+export default function List<T>(props: ListProps<T>) {
+  const [displayItems, setDisplayItems] = useState(() =>
+    convertToRowDisplayData(props.items, props.getKey),
+  );
+  // Skip add transition on first render when initial items are added.
+  const skipAddTransition = useRef(props.skipInitialAddTransition ?? false);
+
+  useEffect(() => {
+    setDisplayItems((prevItems) => {
+      if (props.skipRemoveTransition) {
+        return convertToRowDisplayData(props.items, props.getKey);
+      } else {
+        const nextItems = convertToRowData(props.items, props.getKey);
+        return calculateItemList(prevItems, nextItems);
+      }
+    });
+  }, [props.items, props.getKey]);
+
+  useEffect(() => {
+    // Set to animate accordion for added items after first render unless
+    // props.skipAddTransition === true.
+    skipAddTransition.current = props.skipAddTransition ?? false;
+  }, []);
+
+  const onRemoved = useCallback((key: string) => {
+    setDisplayItems((items) => items.filter((item) => item.key !== key));
+  }, []);
+
+  return (
+    <>
+      {displayItems.map((displayItem) => (
+        <ListItem
+          key={displayItem.key}
+          data={displayItem}
+          onRemoved={onRemoved}
+          render={props.children}
+          skipAddTransition={skipAddTransition.current}
+        />
+      ))}
+    </>
+  );
+}
+
+interface ListItemProps<T> {
+  data: RowDisplayData<T>;
+  onRemoved: (key: string) => void;
+  render: (data: T) => React.ReactNode;
+  skipAddTransition: boolean;
+}
+
+function ListItem<T>(props: ListItemProps<T>) {
+  // If skipAddTransition is true then the item is expanded from the beginning.
+  const [expanded, setExpanded] = useState(props.skipAddTransition);
+
+  const onTransitionEnd = useCallback(() => {
+    if (props.data.removing) {
+      props.onRemoved(props.data.key);
+    }
+  }, [props.onRemoved, props.data.key, props.data.removing]);
+
+  // Expands after initial render and collapses when item is set to being removed.
+  useEffect(() => setExpanded(!props.data.removing), [props.data.removing]);
+
+  return (
+    <Accordion expanded={expanded} onTransitionEnd={onTransitionEnd}>
+      {props.render(props.data.data)}
+    </Accordion>
+  );
+}
+
+function convertToRowData<T>(items: Array<T>, getKey: (data: T) => string): Array<RowData<T>> {
+  return items.map((item) => ({ key: getKey(item), data: item }));
+}
+
+function convertToRowDisplayData<T>(
+  items: Array<T>,
+  getKey: (data: T) => string,
+  removing = false,
+): Array<RowDisplayData<T>> {
+  return convertToRowData(items, getKey).map((item) => ({ ...item, removing }));
+}
+
+export function calculateItemList<T>(
+  prevItemsList: Array<RowDisplayData<T>>,
+  nextItemsList: Array<RowData<T>>,
+): Array<RowDisplayData<T>> {
+  const prevItems = [...prevItemsList];
+  const nextItems = [...nextItemsList];
+
+  if (
+    prevItems.length !== nextItems.length ||
+    !prevItems.every((prevItem, i) => prevItem.key === nextItems[i].key)
+  ) {
+    // If the nextItems contains changes from prevItems we want to calculate the next state.
+    const combinedItems: Array<RowDisplayData<T>> = [];
+
+    while (prevItems.length > 0 || nextItems.length > 0) {
+      const prevItem = prevItems[0];
+      const nextItem = nextItems[0];
+
+      // Either prevItem or nextItem must have a value since at least one of the lists isn't
+      // empty.
+      if (prevItem?.key === nextItem?.key) {
+        combinedItems.push({ ...prevItem, removing: false });
+        prevItems.shift();
+        nextItems.shift();
+      } else if (
+        prevItem === undefined ||
+        nextItems.find((item) => item.key === prevItem.key) !== undefined
+      ) {
+        // An item has been added if there are no more previous items or if the current prevItem
+        // exists later in nextItems.
+        combinedItems.push({ ...nextItem, removing: false });
+        nextItems.shift();
+      } else {
+        combinedItems.push({ ...prevItem, removing: true });
+        prevItems.shift();
+      }
+    }
+
+    return combinedItems;
+  } else {
+    return prevItemsList;
+  }
+}
diff --git a/gui/src/renderer/components/SplitTunnelingSettings.tsx b/gui/src/renderer/components/SplitTunnelingSettings.tsx
index 278cbd90a8..5f6f125d39 100644
--- a/gui/src/renderer/components/SplitTunnelingSettings.tsx
+++ b/gui/src/renderer/components/SplitTunnelingSettings.tsx
@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { useSelector } from 'react-redux';
 import { sprintf } from 'sprintf-js';
 import { colors } from '../../config.json';
@@ -14,6 +14,7 @@ import * as Cell from './cell';
 import { CustomScrollbarsRef } from './CustomScrollbars';
 import ImageView from './ImageView';
 import { Layout } from './Layout';
+import List from './List';
 import { ModalContainer, ModalAlert, ModalAlertType } from './Modal';
 import {
   BackBarItem,
@@ -32,8 +33,6 @@ import {
   StyledIcon,
   StyledCellLabel,
   StyledIconPlaceholder,
-  StyledApplicationListContent,
-  StyledApplicationListAnimation,
   StyledSpinnerRow,
   StyledBrowseButton,
   StyledSearchInput,
@@ -47,6 +46,7 @@ import {
   StyledDisabledWarning,
   StyledActionIcon,
   StyledCellWarningIcon,
+  StyledListContainer,
 } from './SplitTunnelingSettingsStyles';
 
 export default function SplitTunneling() {
@@ -475,34 +475,32 @@ interface IApplicationListProps<T extends IApplication> {
 }
 
 function ApplicationList<T extends IApplication>(props: IApplicationListProps<T>) {
-  const [applicationListHeight, setApplicationListHeight] = useState<number>();
-  const applicationListRef = useRef() as React.RefObject<HTMLDivElement>;
-
-  useLayoutEffect(() => {
-    const height = applicationListRef.current?.getBoundingClientRect().height;
-    setApplicationListHeight(height);
-  }, [applicationListRef, props.applications]);
-
-  return (
-    <StyledApplicationListAnimation height={applicationListHeight}>
-      <StyledApplicationListContent ref={applicationListRef}>
-        {props.applications === undefined ? (
-          <StyledSpinnerRow>
-            <ImageView source="icon-spinner" height={60} width={60} />
-          </StyledSpinnerRow>
-        ) : (
-          props.applications.map((application) => (
+  if (props.applications === undefined) {
+    return (
+      <StyledSpinnerRow>
+        <ImageView source="icon-spinner" height={60} width={60} />
+      </StyledSpinnerRow>
+    );
+  } else {
+    return (
+      <StyledListContainer>
+        <List items={props.applications} getKey={applicationGetKey}>
+          {(application) => (
             <props.rowComponent
               key={application.absolutepath}
               application={application}
               onSelect={props.onSelect}
               onRemove={props.onRemove}
             />
-          ))
-        )}
-      </StyledApplicationListContent>
-    </StyledApplicationListAnimation>
-  );
+          )}
+        </List>
+      </StyledListContainer>
+    );
+  }
+}
+
+function applicationGetKey<T extends IApplication>(application: T): string {
+  return application.absolutepath;
 }
 
 interface IApplicationRowProps<T extends IApplication> {
diff --git a/gui/src/renderer/components/SplitTunnelingSettingsStyles.tsx b/gui/src/renderer/components/SplitTunnelingSettingsStyles.tsx
index a551f55d55..239b3fe560 100644
--- a/gui/src/renderer/components/SplitTunnelingSettingsStyles.tsx
+++ b/gui/src/renderer/components/SplitTunnelingSettingsStyles.tsx
@@ -68,25 +68,21 @@ export const StyledIconPlaceholder = styled.div({
   marginRight: '12px',
 });
 
-export const StyledApplicationListContent = styled.div({
-  display: 'flex',
-  flexDirection: 'column',
-});
-
-export const StyledApplicationListAnimation = styled.div({}, (props: { height?: number }) => ({
-  overflow: 'hidden',
-  height: props.height ? `${props.height}px` : 'auto',
-  transition: 'height 500ms ease-in-out',
-  marginBottom: '20px',
-}));
-
-export const StyledSpinnerRow = styled.div({
+export const StyledSpinnerRow = styled(Cell.CellButton)({
   display: 'flex',
+  alignItems: 'center',
   justifyContent: 'center',
   padding: '8px 0',
+  marginBottom: '20px',
   background: colors.blue40,
 });
 
+export const StyledListContainer = styled.div({
+  display: 'flex',
+  flexDirection: 'column',
+  marginBottom: '20px',
+});
+
 export const StyledBrowseButton = styled(AppButton.BlueButton)({
   margin: '0 22px 22px',
 });
diff --git a/gui/test/list-diff.spec.ts b/gui/test/list-diff.spec.ts
new file mode 100644
index 0000000000..bca3e72d84
--- /dev/null
+++ b/gui/test/list-diff.spec.ts
@@ -0,0 +1,103 @@
+import { expect } from 'chai';
+import { it, describe } from 'mocha';
+import { calculateItemList, RowDisplayData } from '../src/renderer/components/List';
+
+const prevItems: Array<RowDisplayData<undefined>> = [
+  { key: 'a', data: undefined, removing: false },
+  { key: 'b', data: undefined, removing: false },
+  { key: 'c', data: undefined, removing: false },
+  { key: 'd', data: undefined, removing: false },
+];
+
+describe('List diff', () => {
+  it('Should add item to the beginning', () => {
+    const nextItems: Array<RowDisplayData<undefined>> = [
+      { key: '1', data: undefined, removing: false },
+      ...prevItems,
+    ];
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(5);
+    expect(combinedItems.slice(1)).to.deep.equal(prevItems);
+    expect(combinedItems).to.deep.equal(nextItems);
+  });
+
+  it('Should add item to the end', () => {
+    const nextItems: Array<RowDisplayData<undefined>> = [
+      ...prevItems,
+      { key: '1', data: undefined, removing: false },
+    ];
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(5);
+    expect(combinedItems.slice(0, 4)).to.deep.equal(prevItems);
+    expect(combinedItems).to.deep.equal(nextItems);
+  });
+
+  it('Should add item to the middle', () => {
+    const nextItems: Array<RowDisplayData<undefined>> = [
+      ...prevItems.slice(0, 2),
+      { key: '1', data: undefined, removing: false },
+      ...prevItems.slice(2),
+    ];
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(5);
+    expect([...combinedItems.slice(0, 2), ...combinedItems.slice(3)]).to.deep.equal(prevItems);
+    expect(combinedItems).to.deep.equal(nextItems);
+  });
+
+  it('Should remove first item', () => {
+    const nextItems = prevItems.slice(1);
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(4);
+    expect(combinedItems.slice(1, 4)).to.deep.equal(prevItems.slice(1, 4));
+    expect(combinedItems[0]).to.deep.equal({ ...prevItems[0], removing: true });
+  });
+
+  it('Should remove last item', () => {
+    const nextItems = prevItems.slice(0, -1);
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(4);
+    expect(combinedItems.slice(0, -1)).to.deep.equal(prevItems.slice(0, -1));
+    expect(combinedItems.at(-1)).to.deep.equal({ ...prevItems.at(-1), removing: true });
+  });
+
+  it('Should remove middle item', () => {
+    const nextItems = [...prevItems.slice(0, 1), ...prevItems.slice(2)];
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(4);
+    expect(combinedItems.slice(0, 1)).to.deep.equal(prevItems.slice(0, 1));
+    expect(combinedItems.slice(2)).to.deep.equal(prevItems.slice(2));
+    expect(combinedItems[1]).to.deep.equal({ ...prevItems[1], removing: true });
+  });
+
+  it('should both add and remove items', () => {
+    const nextItems = [
+      { key: '1', data: undefined, removing: false },
+      ...prevItems.slice(1, -1),
+      { key: '2', data: undefined, removing: false },
+    ];
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.have.length(6);
+    expect(combinedItems[0]).to.deep.equal({ ...prevItems[0], removing: true });
+    expect(combinedItems[1]).to.deep.equal(nextItems[0]);
+    expect(combinedItems.slice(2, -2)).to.deep.equal(prevItems.slice(1, -1));
+    expect(combinedItems.at(-2)).to.deep.equal({ ...prevItems.at(-1), removing: true });
+    expect(combinedItems.at(-1)).to.deep.equal(nextItems.at(-1));
+  });
+
+  it('should remove item being removed', () => {
+    const prevItems: Array<RowDisplayData<undefined>> = [
+      { key: '1', data: undefined, removing: true },
+    ];
+    const nextItems: Array<RowDisplayData<undefined>> = [];
+    const combinedItems = calculateItemList(prevItems, nextItems);
+
+    expect(combinedItems).to.deep.equal(prevItems);
+  });
+});