Convert existing `window.confirm` calls to use `confirm` prompt

Convert existing uses of `window.confirm` to use our own `confirm`
utility which uses the standard Dialog from our pattern library.

Using our own prompt avoids an immediate problem that browsers, starting
with Chrome v91, are starting to block the use of `window.confirm` in
third-party iframe. It also makes the visual design of the prompt match
the rest of the application.

src/sidebar/components/Annotation/AnnotationActionBar.js

@@ -7,6 +7,7 @@ import { isPrivate, permits } from '../../helpers/permissions';
 import { withServices } from '../../service-context';
 
 import { IconButton } from '../../../shared/components/buttons';
+import { confirm } from '../../../shared/prompts';
 
 import AnnotationShareControl from './AnnotationShareControl';
 
@@ -57,11 +58,19 @@ function AnnotationActionBar({
   const shareLink =
     sharingEnabled(settings) && annotationSharingLink(annotation);
 
-  const onDelete = () => {
-    if (window.confirm('Are you sure you want to delete this annotation?')) {
-      annotationsService.delete(annotation).catch(err => {
+  const onDelete = async () => {
+    if (
+      await confirm({
+        title: 'Delete annotation?',
+        message: 'Are you sure you want to delete this annotation?',
+        confirmAction: 'Delete',
+      })
+    ) {
+      try {
+        await annotationsService.delete(annotation);
+      } catch (err) {
         toastMessenger.error(err.message);
-      });
+      }
     }
   };
 

src/sidebar/components/Annotation/test/AnnotationActionBar-test.js

@@ -12,8 +12,8 @@ import { waitFor } from '../../../../test-util/wait';
 
 describe('AnnotationActionBar', () => {
   let fakeAnnotation;
+  let fakeConfirm;
   let fakeOnReply;
-
   let fakeUserProfile;
 
   // Fake services
@@ -87,6 +87,8 @@ describe('AnnotationActionBar', () => {
       profile: sinon.stub().returns(fakeUserProfile),
     };
 
+    fakeConfirm = sinon.stub().resolves(false);
+
     $imports.$mock(mockImportedComponents());
     $imports.$mock({
       '../../helpers/annotation-sharing': {
@@ -95,12 +97,11 @@ describe('AnnotationActionBar', () => {
       },
       '../../helpers/permissions': { permits: fakePermits },
       '../../store/use-store': { useStoreProxy: () => fakeStore },
+      '../../../shared/prompts': { confirm: fakeConfirm },
     });
-    sinon.stub(window, 'confirm').returns(false);
   });
 
   afterEach(() => {
-    window.confirm.restore();
     $imports.$restore();
   });
 
@@ -145,25 +146,25 @@ describe('AnnotationActionBar', () => {
       assert.isTrue(getButton(wrapper, 'trash').exists());
     });
 
-    it('asks for confirmation before deletion', () => {
+    it('asks for confirmation before deletion', async () => {
       allowOnly('delete');
       const button = getButton(createComponent(), 'trash');
 
-      act(() => {
-        button.props().onClick();
+      await act(async () => {
+        await button.props().onClick();
       });
 
-      assert.calledOnce(confirm);
+      assert.calledOnce(fakeConfirm);
       assert.notCalled(fakeAnnotationsService.delete);
     });
 
-    it('invokes delete on service when confirmed', () => {
+    it('invokes delete on service when confirmed', async () => {
       allowOnly('delete');
-      window.confirm.returns(true);
+      fakeConfirm.resolves(true);
       const button = getButton(createComponent(), 'trash');
 
-      act(() => {
-        button.props().onClick();
+      await act(async () => {
+        await button.props().onClick();
       });
 
       assert.calledWith(fakeAnnotationsService.delete, fakeAnnotation);
@@ -171,12 +172,12 @@ describe('AnnotationActionBar', () => {
 
     it('sets a flash message if there is an error with deletion', async () => {
       allowOnly('delete');
-      window.confirm.returns(true);
+      fakeConfirm.resolves(true);
       fakeAnnotationsService.delete.rejects();
 
       const button = getButton(createComponent(), 'trash');
-      act(() => {
-        button.props().onClick();
+      await act(async () => {
+        await button.props().onClick();
       });
 
       await waitFor(() => fakeToastMessenger.error.called);
@@ -275,8 +276,6 @@ describe('AnnotationActionBar', () => {
     });
 
     it('invokes flag on service when clicked', () => {
-      window.confirm.returns(true);
-
       const button = getButton(createComponent(), 'flag');
 
       act(() => {
@@ -287,7 +286,6 @@ describe('AnnotationActionBar', () => {
     });
 
     it('sets flash error message if flagging fails on service', async () => {
-      window.confirm.returns(true);
       fakeAnnotationsService.flag.rejects();
 
       const button = getButton(createComponent(), 'flag');

src/sidebar/components/GroupList/GroupListItem.js

@@ -2,6 +2,7 @@ import { orgName } from '../../helpers/group-list-item-common';
 import { withServices } from '../../service-context';
 import { useStoreProxy } from '../../store/use-store';
 import { copyText } from '../../util/copy-to-clipboard';
+import { confirm } from '../../../shared/prompts';
 
 import MenuItem from '../MenuItem';
 
@@ -49,9 +50,15 @@ function GroupListItem({
     groupsService.focus(group.id);
   };
 
-  const leaveGroup = () => {
+  const leaveGroup = async () => {
     const message = `Are you sure you want to leave the group "${group.name}"?`;
-    if (window.confirm(message)) {
+    if (
+      await confirm({
+        title: 'Leave group?',
+        message,
+        confirmAction: 'Leave',
+      })
+    ) {
       groupsService.leave(group.id);
     }
   };

src/sidebar/components/GroupList/test/GroupListItem-test.js

@@ -3,7 +3,12 @@ import { act } from 'preact/test-utils';
 
 import GroupListItem, { $imports } from '../GroupListItem';
 
+function delay(ms) {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
 describe('GroupListItem', () => {
+  let fakeConfirm;
   let fakeCopyText;
   let fakeToastMessenger;
   let fakeGroupsService;
@@ -57,6 +62,8 @@ describe('GroupListItem', () => {
     }
     FakeSlider.displayName = 'Slider';
 
+    fakeConfirm = sinon.stub().resolves(false);
+
     $imports.$mock({
       '../MenuItem': FakeMenuItem,
       '../../util/copy-to-clipboard': {
@@ -64,14 +71,12 @@ describe('GroupListItem', () => {
       },
       '../../helpers/group-list-item-common': fakeGroupListItemCommon,
       '../../store/use-store': { useStoreProxy: () => fakeStore },
+      '../../../shared/prompts': { confirm: fakeConfirm },
     });
-
-    sinon.stub(window, 'confirm').returns(false);
   });
 
   afterEach(() => {
     $imports.$restore();
-    window.confirm.restore();
   });
 
   const createGroupListItem = (fakeGroup, props = {}) => {
@@ -243,28 +248,30 @@ describe('GroupListItem', () => {
     assert.isTrue(submenu.exists('MenuItem[label="Leave group"]'));
   });
 
-  it('prompts to leave group if "Leave" action is clicked', () => {
+  it('prompts to leave group if "Leave" action is clicked', async () => {
     const wrapper = createGroupListItem(fakeGroup, {
       isExpanded: true,
     });
 
     const submenu = getSubmenu(wrapper);
     clickMenuItem(submenu, 'Leave group');
+    await delay(0);
 
-    assert.called(window.confirm);
+    assert.called(fakeConfirm);
     assert.notCalled(fakeGroupsService.leave);
   });
 
-  it('leaves group if "Leave" is clicked and user confirms', () => {
+  it('leaves group if "Leave" is clicked and user confirms', async () => {
     const wrapper = createGroupListItem(fakeGroup, {
       isExpanded: true,
     });
-    window.confirm.returns(true);
+    fakeConfirm.resolves(true);
 
     const submenu = getSubmenu(wrapper);
     clickMenuItem(submenu, 'Leave group');
+    await delay(0);
 
-    assert.called(window.confirm);
+    assert.called(fakeConfirm);
     assert.calledWith(fakeGroupsService.leave, fakeGroup.id);
   });
 

src/sidebar/components/HypothesisApp.js

@@ -2,6 +2,7 @@ import classnames from 'classnames';
 import { useEffect, useMemo } from 'preact/hooks';
 
 import bridgeEvents from '../../shared/bridge-events';
+import { confirm } from '../../shared/prompts';
 import serviceConfig from '../config/service-config';
 import { useStoreProxy } from '../store/use-store';
 import { parseAccountID } from '../helpers/account-id';
@@ -129,26 +130,33 @@ function HypothesisApp({
     window.open(serviceUrl('signup'));
   };
 
-  const promptToLogout = () => {
-    // TODO - Replace this with a UI which doesn't look terrible.
-    let text = '';
+  const promptToLogout = async () => {
     const drafts = store.countDrafts();
+    if (drafts === 0) {
+      return true;
+    }
+
+    let message = '';
     if (drafts === 1) {
-      text =
+      message =
         'You have an unsaved annotation.\n' +
         'Do you really want to discard this draft?';
     } else if (drafts > 1) {
-      text =
+      message =
         'You have ' +
         drafts +
         ' unsaved annotations.\n' +
         'Do you really want to discard these drafts?';
     }
-    return drafts === 0 || window.confirm(text);
+    return confirm({
+      title: 'Discard drafts?',
+      message,
+      confirmAction: 'Discard',
+    });
   };
 
-  const logout = () => {
-    if (!promptToLogout()) {
+  const logout = async () => {
+    if (!(await promptToLogout())) {
       return;
     }
     store.clearGroups();

src/sidebar/components/test/HypothesisApp-test.js

@@ -10,6 +10,7 @@ describe('HypothesisApp', () => {
   let fakeStore = null;
   let fakeAuth = null;
   let fakeBridge = null;
+  let fakeConfirm;
   let fakeServiceConfig = null;
   let fakeSession = null;
   let fakeShouldAutoDisplayTutorial = null;
@@ -77,6 +78,8 @@ describe('HypothesisApp', () => {
       notice: sinon.stub(),
     };
 
+    fakeConfirm = sinon.stub().resolves(false);
+
     $imports.$mock(mockImportedComponents());
     $imports.$mock({
       '../config/service-config': fakeServiceConfig,
@@ -85,6 +88,7 @@ describe('HypothesisApp', () => {
         shouldAutoDisplayTutorial: fakeShouldAutoDisplayTutorial,
       },
       '../helpers/theme': { applyTheme: fakeApplyTheme },
+      '../../shared/prompts': { confirm: fakeConfirm },
     });
   });
 
@@ -305,43 +309,41 @@ describe('HypothesisApp', () => {
   });
 
   describe('"Log out" action', () => {
-    const clickLogOut = wrapper => wrapper.find('TopBar').props().onLogout();
-
     beforeEach(() => {
-      sinon.stub(window, 'confirm');
+      fakeConfirm.resolves(true);
     });
 
-    afterEach(() => {
-      window.confirm.restore();
-    });
+    const clickLogOut = async wrapper => {
+      await wrapper.find('TopBar').props().onLogout();
+    };
 
     // Tests used by both the first and third-party account scenarios.
     function addCommonLogoutTests() {
       // nb. Slightly different messages are shown depending on the draft count.
       [1, 2].forEach(draftCount => {
-        it('prompts the user if there are drafts', () => {
+        it('prompts the user if there are drafts', async () => {
           fakeStore.countDrafts.returns(draftCount);
 
           const wrapper = createComponent();
-          clickLogOut(wrapper);
+          await clickLogOut(wrapper);
 
-          assert.equal(window.confirm.callCount, 1);
+          assert.equal(fakeConfirm.callCount, 1);
         });
       });
 
-      it('clears groups', () => {
+      it('clears groups', async () => {
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert.called(fakeStore.clearGroups);
       });
 
-      it('removes unsaved annotations', () => {
+      it('removes unsaved annotations', async () => {
         fakeStore.unsavedAnnotations = sinon
           .stub()
           .returns(['draftOne', 'draftTwo', 'draftThree']);
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert.calledWith(fakeStore.removeAnnotations, [
           'draftOne',
@@ -350,49 +352,49 @@ describe('HypothesisApp', () => {
         ]);
       });
 
-      it('discards drafts', () => {
+      it('discards drafts', async () => {
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert(fakeStore.discardAllDrafts.calledOnce);
       });
 
-      it('does not remove unsaved annotations if the user cancels the prompt', () => {
+      it('does not remove unsaved annotations if the user cancels the prompt', async () => {
         const wrapper = createComponent();
         fakeStore.countDrafts.returns(1);
-        window.confirm.returns(false);
+        fakeConfirm.resolves(false);
 
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert.notCalled(fakeStore.removeAnnotations);
       });
 
-      it('does not discard drafts if the user cancels the prompt', () => {
+      it('does not discard drafts if the user cancels the prompt', async () => {
         const wrapper = createComponent();
         fakeStore.countDrafts.returns(1);
-        window.confirm.returns(false);
+        fakeConfirm.resolves(false);
 
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert(fakeStore.discardAllDrafts.notCalled);
       });
 
-      it('does not prompt if there are no drafts', () => {
+      it('does not prompt if there are no drafts', async () => {
         const wrapper = createComponent();
         fakeStore.countDrafts.returns(0);
 
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
-        assert.notCalled(window.confirm);
+        assert.notCalled(fakeConfirm);
       });
     }
 
     context('when no third-party service is in use', () => {
       addCommonLogoutTests();
 
-      it('calls session.logout()', () => {
+      it('calls session.logout()', async () => {
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
         assert.called(fakeSession.logout);
       });
     });
@@ -404,9 +406,9 @@ describe('HypothesisApp', () => {
 
       addCommonLogoutTests();
 
-      it('sends LOGOUT_REQUESTED', () => {
+      it('sends LOGOUT_REQUESTED', async () => {
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert.calledOnce(fakeBridge.call);
         assert.calledWithExactly(
@@ -415,19 +417,19 @@ describe('HypothesisApp', () => {
         );
       });
 
-      it('does not send LOGOUT_REQUESTED if the user cancels the prompt', () => {
+      it('does not send LOGOUT_REQUESTED if the user cancels the prompt', async () => {
         fakeStore.countDrafts.returns(1);
-        window.confirm.returns(false);
+        fakeConfirm.returns(false);
 
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
 
         assert.notCalled(fakeBridge.call);
       });
 
-      it('does not call session.logout()', () => {
+      it('does not call session.logout()', async () => {
         const wrapper = createComponent();
-        clickLogOut(wrapper);
+        await clickLogOut(wrapper);
         assert.notCalled(fakeSession.logout);
       });
     });