Merge pull request #747 from acelaya-forks/feature/modal-async-delays

Feature/modal async delays

CHANGELOG.md

@@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * [#590](https://github.com/shlinkio/shlink-web-client/issues/590) Fixed position of the datepicker triangle.
 * [#729](https://github.com/shlinkio/shlink-web-client/issues/729) Fixed wrong stats displayed in tags after renaming.
 * [#737](https://github.com/shlinkio/shlink-web-client/issues/737) Fixed incorrect contrast in warning messages when using dark theme.
+* [#726](https://github.com/shlinkio/shlink-web-client/issues/726) Fixed delete server and delete short URL modals getting removed from the DOM before finishing close transition.
 
 
 ## [3.7.3] - 2022-09-13

src/servers/DeleteServerModal.tsx

@@ -1,4 +1,4 @@
-import { FC } from 'react';
+import { FC, useRef } from 'react';
 import { Button, Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap';
 import { useNavigate } from 'react-router-dom';
 import { ServerWithId } from './data';
@@ -18,14 +18,22 @@ export const DeleteServerModal: FC<DeleteServerModalConnectProps> = (
   { server, toggle, isOpen, deleteServer, redirectHome = true },
 ) => {
   const navigate = useNavigate();
-  const closeModal = () => {
-    deleteServer(server);
+  const doDelete = useRef<boolean>(false);
+  const toggleAndDelete = () => {
+    doDelete.current = true;
     toggle();
+  };
+  const onClosed = () => {
+    if (!doDelete.current) {
+      return;
+    }
+
+    deleteServer(server);
     redirectHome && navigate('/');
   };
 
   return (
-    <Modal isOpen={isOpen} toggle={toggle} centered>
+    <Modal isOpen={isOpen} toggle={toggle} centered onClosed={onClosed}>
       <ModalHeader toggle={toggle} className="text-danger">Remove server</ModalHeader>
       <ModalBody>
         <p>Are you sure you want to remove <b>{server ? server.name : ''}</b>?</p>
@@ -38,7 +46,7 @@ export const DeleteServerModal: FC<DeleteServerModalConnectProps> = (
       </ModalBody>
       <ModalFooter>
         <Button color="link" onClick={toggle}>Cancel</Button>
-        <Button color="danger" onClick={() => closeModal()}>Delete</Button>
+        <Button color="danger" onClick={toggleAndDelete}>Delete</Button>
       </ModalFooter>
     </Modal>
   );

src/short-urls/helpers/DeleteShortUrlModal.tsx

@@ -10,23 +10,30 @@ import { ShlinkApiError } from '../../api/ShlinkApiError';
 
 interface DeleteShortUrlModalConnectProps extends ShortUrlModalProps {
   shortUrlDeletion: ShortUrlDeletion;
-  deleteShortUrl: (shortUrl: ShortUrlIdentifier) => void;
+  deleteShortUrl: (shortUrl: ShortUrlIdentifier) => Promise<void>;
+  shortUrlDeleted: (shortUrl: ShortUrlIdentifier) => void;
   resetDeleteShortUrl: () => void;
 }
 
-export const DeleteShortUrlModal = (
-  { shortUrl, toggle, isOpen, shortUrlDeletion, resetDeleteShortUrl, deleteShortUrl }: DeleteShortUrlModalConnectProps,
-) => {
+export const DeleteShortUrlModal = ({
+  shortUrl,
+  toggle,
+  isOpen,
+  shortUrlDeletion,
+  resetDeleteShortUrl,
+  deleteShortUrl,
+  shortUrlDeleted,
+}: DeleteShortUrlModalConnectProps) => {
   const [inputValue, setInputValue] = useState('');
 
   useEffect(() => resetDeleteShortUrl, []);
 
-  const { loading, error, errorData } = shortUrlDeletion;
+  const { loading, error, deleted, errorData } = shortUrlDeletion;
   const close = pipe(resetDeleteShortUrl, toggle);
-  const handleDeleteUrl = handleEventPreventingDefault(() => deleteShortUrl(shortUrl));
+  const handleDeleteUrl = handleEventPreventingDefault(() => deleteShortUrl(shortUrl).then(toggle));
 
   return (
-    <Modal isOpen={isOpen} toggle={close} centered>
+    <Modal isOpen={isOpen} toggle={close} centered onClosed={() => deleted && shortUrlDeleted(shortUrl)}>
       <form onSubmit={handleDeleteUrl}>
         <ModalHeader toggle={close}>
           <span className="text-danger">Delete short URL</span>

src/short-urls/reducers/shortUrlDeletion.ts

@@ -1,9 +1,9 @@
-import { createSlice } from '@reduxjs/toolkit';
+import { createAction, createSlice } from '@reduxjs/toolkit';
 import { createAsyncThunk } from '../../utils/helpers/redux';
 import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder';
 import { parseApiError } from '../../api/utils';
 import { ProblemDetailsError } from '../../api/types/errors';
-import { ShortUrlIdentifier } from '../data';
+import { ShortUrl, ShortUrlIdentifier } from '../data';
 
 const REDUCER_PREFIX = 'shlink/shortUrlDeletion';
 
@@ -31,6 +31,8 @@ export const deleteShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) =>
   },
 );
 
+export const shortUrlDeleted = createAction<ShortUrl>(`${REDUCER_PREFIX}/shortUrlDeleted`);
+
 export const shortUrlDeletionReducerCreator = (deleteShortUrlThunk: ReturnType<typeof deleteShortUrl>) => {
   const { actions, reducer } = createSlice({
     name: REDUCER_PREFIX,

src/short-urls/reducers/shortUrlsList.ts

@@ -5,7 +5,7 @@ import { createNewVisits } from '../../visits/reducers/visitCreation';
 import { createAsyncThunk } from '../../utils/helpers/redux';
 import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder';
 import { ShlinkShortUrlsListParams, ShlinkShortUrlsResponse } from '../../api/types';
-import { deleteShortUrl } from './shortUrlDeletion';
+import { shortUrlDeleted } from './shortUrlDeletion';
 import { createShortUrl } from './shortUrlCreation';
 import { editShortUrl } from './shortUrlEdition';
 import { ShortUrl } from '../data';
@@ -36,7 +36,6 @@ export const shortUrlsListReducerCreator = (
   listShortUrlsThunk: ReturnType<typeof listShortUrls>,
   editShortUrlThunk: ReturnType<typeof editShortUrl>,
   createShortUrlThunk: ReturnType<typeof createShortUrl>,
-  deleteShortUrlThunk: ReturnType<typeof deleteShortUrl>,
 ) => createSlice({
   name: REDUCER_PREFIX,
   initialState,
@@ -81,7 +80,7 @@ export const shortUrlsListReducerCreator = (
     );
 
     builder.addCase(
-      deleteShortUrlThunk.fulfilled,
+      shortUrlDeleted,
       pipe(
         (state, { payload }) => (!state.shortUrls ? state : assocPath(
           ['shortUrls', 'data'],

src/short-urls/services/provideServices.ts

@@ -9,7 +9,7 @@ import { DeleteShortUrlModal } from '../helpers/DeleteShortUrlModal';
 import { CreateShortUrlResult } from '../helpers/CreateShortUrlResult';
 import { listShortUrls, shortUrlsListReducerCreator } from '../reducers/shortUrlsList';
 import { shortUrlCreationReducerCreator, createShortUrl } from '../reducers/shortUrlCreation';
-import { shortUrlDeletionReducerCreator, deleteShortUrl } from '../reducers/shortUrlDeletion';
+import { shortUrlDeletionReducerCreator, deleteShortUrl, shortUrlDeleted } from '../reducers/shortUrlDeletion';
 import { editShortUrl, shortUrlEditionReducerCreator } from '../reducers/shortUrlEdition';
 import { shortUrlDetailReducerCreator } from '../reducers/shortUrlDetail';
 import { ConnectDecorator } from '../../container/types';
@@ -46,7 +46,10 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => {
   ));
 
   bottle.serviceFactory('DeleteShortUrlModal', () => DeleteShortUrlModal);
-  bottle.decorator('DeleteShortUrlModal', connect(['shortUrlDeletion'], ['deleteShortUrl', 'resetDeleteShortUrl']));
+  bottle.decorator('DeleteShortUrlModal', connect(
+    ['shortUrlDeletion'],
+    ['deleteShortUrl', 'shortUrlDeleted', 'resetDeleteShortUrl'],
+  ));
 
   bottle.serviceFactory('QrCodeModal', QrCodeModal, 'ImageDownloader');
   bottle.decorator('QrCodeModal', connect(['selectedServer']));
@@ -63,7 +66,6 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => {
     'listShortUrls',
     'editShortUrl',
     'createShortUrl',
-    'deleteShortUrl',
   );
   bottle.serviceFactory('shortUrlsListReducer', prop('reducer'), 'shortUrlsListReducerCreator');
 
@@ -87,6 +89,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => {
 
   bottle.serviceFactory('deleteShortUrl', deleteShortUrl, 'buildShlinkApiClient');
   bottle.serviceFactory('resetDeleteShortUrl', prop('resetDeleteShortUrl'), 'shortUrlDeletionReducerCreator');
+  bottle.serviceFactory('shortUrlDeleted', () => shortUrlDeleted);
 
   bottle.serviceFactory('getShortUrlDetail', prop('getShortUrlDetail'), 'shortUrlDetailReducerCreator');
 

test/__helpers__/TestModalWrapper.tsx

@@ -0,0 +1,14 @@
+import { FC, ReactElement } from 'react';
+import { useToggle } from '../../src/utils/helpers/hooks';
+
+interface RenderModalArgs {
+  isOpen: boolean;
+  toggle: () => void;
+}
+
+export const TestModalWrapper: FC<{ renderModal: (args: RenderModalArgs) => ReactElement }> = (
+  { renderModal },
+) => {
+  const [isOpen, toggle] = useToggle(true);
+  return renderModal({ isOpen, toggle });
+};

test/servers/DeleteServerModal.test.tsx

@@ -1,26 +1,29 @@
-import { screen } from '@testing-library/react';
+import { screen, waitFor } from '@testing-library/react';
 import { Mock } from 'ts-mockery';
 import { useNavigate } from 'react-router-dom';
 import { DeleteServerModal } from '../../src/servers/DeleteServerModal';
 import { ServerWithId } from '../../src/servers/data';
 import { renderWithEvents } from '../__helpers__/setUpTest';
+import { TestModalWrapper } from '../__helpers__/TestModalWrapper';
 
 jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), useNavigate: jest.fn() }));
 
 describe('<DeleteServerModal />', () => {
   const deleteServerMock = jest.fn();
   const navigate = jest.fn();
-  const toggleMock = jest.fn();
   const serverName = 'the_server_name';
   const setUp = () => {
     (useNavigate as any).mockReturnValue(navigate);
 
     return renderWithEvents(
-      <DeleteServerModal
-        server={Mock.of<ServerWithId>({ name: serverName })}
-        toggle={toggleMock}
-        isOpen
-        deleteServer={deleteServerMock}
+      <TestModalWrapper
+        renderModal={(args) => (
+          <DeleteServerModal
+            {...args}
+            server={Mock.of<ServerWithId>({ name: serverName })}
+            deleteServer={deleteServerMock}
+          />
+        )}
       />,
     );
   };
@@ -47,10 +50,8 @@ describe('<DeleteServerModal />', () => {
   ])('toggles when clicking cancel button', async (getButton) => {
     const { user } = setUp();
 
-    expect(toggleMock).not.toHaveBeenCalled();
     await user.click(getButton());
 
-    expect(toggleMock).toHaveBeenCalledTimes(1);
     expect(deleteServerMock).not.toHaveBeenCalled();
     expect(navigate).not.toHaveBeenCalled();
   });
@@ -58,13 +59,11 @@ describe('<DeleteServerModal />', () => {
   it('deletes server when clicking accept button', async () => {
     const { user } = setUp();
 
-    expect(toggleMock).not.toHaveBeenCalled();
     expect(deleteServerMock).not.toHaveBeenCalled();
     expect(navigate).not.toHaveBeenCalled();
     await user.click(screen.getByRole('button', { name: 'Delete' }));
 
-    expect(toggleMock).toHaveBeenCalledTimes(1);
-    expect(deleteServerMock).toHaveBeenCalledTimes(1);
-    expect(navigate).toHaveBeenCalledTimes(1);
+    await waitFor(() => expect(deleteServerMock).toHaveBeenCalledTimes(1));
+    await waitFor(() => expect(navigate).toHaveBeenCalledTimes(1));
   });
 });

test/short-urls/helpers/DeleteShortUrlModal.test.tsx

@@ -1,10 +1,11 @@
-import { screen } from '@testing-library/react';
+import { screen, waitFor } from '@testing-library/react';
 import { Mock } from 'ts-mockery';
 import { DeleteShortUrlModal } from '../../../src/short-urls/helpers/DeleteShortUrlModal';
 import { ShortUrl } from '../../../src/short-urls/data';
 import { ShortUrlDeletion } from '../../../src/short-urls/reducers/shortUrlDeletion';
 import { renderWithEvents } from '../../__helpers__/setUpTest';
 import { ErrorTypeV2, ErrorTypeV3, InvalidShortUrlDeletion, ProblemDetailsError } from '../../../src/api/types/errors';
+import { TestModalWrapper } from '../../__helpers__/TestModalWrapper';
 
 describe('<DeleteShortUrlModal />', () => {
   const shortUrl = Mock.of<ShortUrl>({
@@ -12,15 +13,20 @@ describe('<DeleteShortUrlModal />', () => {
     shortCode: 'abc123',
     longUrl: 'https://long-domain.com/foo/bar',
   });
-  const deleteShortUrl = jest.fn();
+  const deleteShortUrl = jest.fn().mockResolvedValue(undefined);
+  const shortUrlDeleted = jest.fn();
   const setUp = (shortUrlDeletion: Partial<ShortUrlDeletion>) => renderWithEvents(
-    <DeleteShortUrlModal
-      isOpen
-      shortUrl={shortUrl}
-      shortUrlDeletion={Mock.of<ShortUrlDeletion>(shortUrlDeletion)}
-      deleteShortUrl={deleteShortUrl}
-      toggle={() => {}}
-      resetDeleteShortUrl={() => {}}
+    <TestModalWrapper
+      renderModal={(args) => (
+        <DeleteShortUrlModal
+          {...args}
+          shortUrl={shortUrl}
+          shortUrlDeletion={Mock.of<ShortUrlDeletion>(shortUrlDeletion)}
+          deleteShortUrl={deleteShortUrl}
+          shortUrlDeleted={shortUrlDeleted}
+          resetDeleteShortUrl={jest.fn()}
+        />
+      )}
     />,
   );
 
@@ -74,6 +80,7 @@ describe('<DeleteShortUrlModal />', () => {
     const { user } = setUp({
       loading: false,
       error: false,
+      deleted: true,
       shortCode,
     });
 
@@ -81,5 +88,6 @@ describe('<DeleteShortUrlModal />', () => {
     await user.type(screen.getByPlaceholderText(/^Insert the short code/), shortCode);
     await user.click(screen.getByRole('button', { name: 'Delete' }));
     expect(deleteShortUrl).toHaveBeenCalledTimes(1);
+    await waitFor(() => expect(shortUrlDeleted).toHaveBeenCalledTimes(1));
   });
 });

test/short-urls/reducers/shortUrlsList.test.ts

@@ -3,7 +3,7 @@ import {
   listShortUrls as listShortUrlsCreator,
   shortUrlsListReducerCreator,
 } from '../../../src/short-urls/reducers/shortUrlsList';
-import { deleteShortUrl as deleteShortUrlCreator } from '../../../src/short-urls/reducers/shortUrlDeletion';
+import { shortUrlDeleted } from '../../../src/short-urls/reducers/shortUrlDeletion';
 import { ShlinkPaginator, ShlinkShortUrlsResponse } from '../../../src/api/types';
 import { createShortUrl as createShortUrlCreator } from '../../../src/short-urls/reducers/shortUrlCreation';
 import { editShortUrl as editShortUrlCreator } from '../../../src/short-urls/reducers/shortUrlEdition';
@@ -18,8 +18,7 @@ describe('shortUrlsListReducer', () => {
   const listShortUrls = listShortUrlsCreator(buildShlinkApiClient);
   const editShortUrl = editShortUrlCreator(buildShlinkApiClient);
   const createShortUrl = createShortUrlCreator(buildShlinkApiClient);
-  const deleteShortUrl = deleteShortUrlCreator(buildShlinkApiClient);
-  const { reducer } = shortUrlsListReducerCreator(listShortUrls, editShortUrl, createShortUrl, deleteShortUrl);
+  const { reducer } = shortUrlsListReducerCreator(listShortUrls, editShortUrl, createShortUrl);
 
   afterEach(jest.clearAllMocks);
 
@@ -59,7 +58,7 @@ describe('shortUrlsListReducer', () => {
         error: false,
       };
 
-      expect(reducer(state, { type: deleteShortUrl.fulfilled.toString(), payload: { shortCode } })).toEqual({
+      expect(reducer(state, { type: shortUrlDeleted.toString(), payload: { shortCode } })).toEqual({
         shortUrls: {
           data: [{ shortCode, domain: 'example.com' }, { shortCode: 'foo' }],
           pagination: { totalItems: 9 },