Refactor Confirm Modal

Currently we had overly compllicated logic
between the JS hook and Vue component for
firing the modal.

This MR simplifies that logic by moving the DOM
listeners into the Vue component itself.

This way we no longer need to do prop management
from the JS hooks and instead just have all our
logic of showing, hiding, updating, and using
the modal in one place.

app/assets/javascripts/confirm_modal.js

@@ -3,40 +3,9 @@ import ConfirmModal from '~/vue_shared/components/confirm_modal.vue';
 
 const mountConfirmModal = () => {
   return new Vue({
-    data() {
-      return {
-        path: '',
-        method: '',
-        modalAttributes: null,
-        showModal: false,
-      };
-    },
-    mounted() {
-      document.querySelectorAll('.js-confirm-modal-button').forEach(button => {
-        button.addEventListener('click', e => {
-          e.preventDefault();
-
-          this.path = button.dataset.path;
-          this.method = button.dataset.method;
-          this.modalAttributes = JSON.parse(button.dataset.modalAttributes);
-          this.showModal = true;
-        });
-      });
-    },
-    methods: {
-      dismiss() {
-        this.showModal = false;
-      },
-    },
     render(h) {
       return h(ConfirmModal, {
-        props: {
-          path: this.path,
-          method: this.method,
-          modalAttributes: this.modalAttributes,
-          showModal: this.showModal,
-        },
-        on: { dismiss: this.dismiss },
+        props: { selector: '.js-confirm-modal-button' },
       });
     },
   }).$mount();

app/assets/javascripts/vue_shared/components/confirm_modal.vue

 <script>
 import { GlModal } from '@gitlab/ui';
 import csrf from '~/lib/utils/csrf';
+import { uniqueId } from 'lodash';
 
 export default {
   components: {
     GlModal,
   },
   props: {
-    modalAttributes: {
-      type: Object,
-      required: false,
-      default: () => {
-        return {};
-      },
-    },
-    path: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    method: {
+    selector: {
       type: String,
-      required: false,
-      default: '',
-    },
-    showModal: {
-      type: Boolean,
-      required: false,
-      default: false,
+      required: true,
     },
   },
-  watch: {
-    showModal(val) {
-      if (val) {
-        // Wait for v-if to render
-        this.$nextTick(() => {
-          this.openModal();
-        });
-      }
-    },
+  data() {
+    return {
+      modalId: uniqueId('confirm-modal-'),
+      path: '',
+      method: '',
+      modalAttributes: {},
+    };
+  },
+  mounted() {
+    document.querySelectorAll(this.selector).forEach(button => {
+      button.addEventListener('click', e => {
+        e.preventDefault();
+
+        this.path = button.dataset.path;
+        this.method = button.dataset.method;
+        this.modalAttributes = JSON.parse(button.dataset.modalAttributes);
+        this.openModal();
+      });
+    });
   },
   methods: {
     openModal() {
       this.$refs.modal.show();
     },
+    closeModal() {
+      this.$refs.modal.hide();
+    },
     submitModal() {
       this.$refs.form.submit();
     },
@@ -54,11 +50,11 @@ export default {
 
 <template>
   <gl-modal
-    v-if="showModal"
     ref="modal"
+    :modal-id="modalId"
     v-bind="modalAttributes"
     @primary="submitModal"
-    @canceled="$emit('dismiss')"
+    @cancel="closeModal"
   >
     <form ref="form" :action="path" method="post">
       <!-- Rails workaround for <form method="delete" />

ee/app/helpers/ee/geo_helper.rb

@@ -159,7 +159,6 @@ module EE
         path: path,
         method: 'delete',
         modal_attributes: {
-          modalId: 'geo-entry-removal-modal',
           title: s_('Geo|Remove tracking database entry'),
           message: s_('Geo|Tracking database entry will be removed. Are you sure?'),
           okVariant: 'danger',

spec/frontend/confirm_modal_spec.js

@@ -8,7 +8,6 @@ describe('ConfirmModal', () => {
       path: `${TEST_HOST}/1`,
       method: 'delete',
       modalAttributes: {
-        modalId: 'geo-entry-removal-modal',
         title: 'Remove tracking database entry',
         message: 'Tracking database entry will be removed. Are you sure?',
         okVariant: 'danger',
@@ -19,7 +18,6 @@ describe('ConfirmModal', () => {
       path: `${TEST_HOST}/1`,
       method: 'post',
       modalAttributes: {
-        modalId: 'geo-entry-removal-modal',
         title: 'Update tracking database entry',
         message: 'Tracking database entry will be updated. Are you sure?',
         okVariant: 'success',
@@ -53,6 +51,7 @@ describe('ConfirmModal', () => {
   const findModalOkButton = (modal, variant) =>
     modal.querySelector(`.modal-footer .btn-${variant}`);
   const findModalCancelButton = modal => modal.querySelector('.modal-footer .btn-secondary');
+  const modalIsHidden = () => findModal().getAttribute('aria-hidden') === 'true';
 
   const serializeModal = (modal, buttonIndex) => {
     const { modalAttributes } = buttons[buttonIndex];
@@ -61,7 +60,6 @@ describe('ConfirmModal', () => {
       path: modal.querySelector('form').action,
       method: modal.querySelector('input[name="_method"]').value,
       modalAttributes: {
-        modalId: modal.id,
         title: modal.querySelector('.modal-title').innerHTML,
         message: modal.querySelector('.modal-body div').innerHTML,
         okVariant: [...findModalOkButton(modal, modalAttributes.okVariant).classList]
@@ -92,6 +90,7 @@ describe('ConfirmModal', () => {
     describe('GlModal', () => {
       it('is rendered', () => {
         expect(findModal()).toExist();
+        expect(modalIsHidden()).toBe(false);
       });
 
       describe('Cancel Button', () => {
@@ -102,7 +101,7 @@ describe('ConfirmModal', () => {
         });
 
         it('closes the modal', () => {
-          expect(findModal()).not.toExist();
+          expect(modalIsHidden()).toBe(true);
         });
       });
     });

spec/frontend/vue_shared/components/confirm_modal_spec.js

@@ -6,11 +6,10 @@ import ConfirmModal from '~/vue_shared/components/confirm_modal.vue';
 jest.mock('~/lib/utils/csrf', () => ({ token: 'test-csrf-token' }));
 
 describe('vue_shared/components/confirm_modal', () => {
-  const testModalProps = {
+  const MOCK_MODAL_DATA = {
     path: `${TEST_HOST}/1`,
     method: 'delete',
     modalAttributes: {
-      modalId: 'test-confirm-modal',
       title: 'Are you sure?',
       message: 'This will remove item 1',
       okVariant: 'danger',
@@ -18,8 +17,13 @@ describe('vue_shared/components/confirm_modal', () => {
     },
   };
 
+  const defaultProps = {
+    selector: '.test-button',
+  };
+
   const actionSpies = {
     openModal: jest.fn(),
+    closeModal: jest.fn(),
   };
 
   let wrapper;
@@ -27,7 +31,7 @@ describe('vue_shared/components/confirm_modal', () => {
   const createComponent = (props = {}) => {
     wrapper = shallowMount(ConfirmModal, {
       propsData: {
-        ...testModalProps,
+        ...defaultProps,
         ...props,
       },
       methods: {
@@ -48,28 +52,18 @@ describe('vue_shared/components/confirm_modal', () => {
       .wrappers.map(x => ({ name: x.attributes('name'), value: x.attributes('value') }));
 
   describe('template', () => {
-    describe('when showModal is false', () => {
+    describe('when modal data is set', () => {
       beforeEach(() => {
         createComponent();
+        wrapper.vm.modalAttributes = MOCK_MODAL_DATA.modalAttributes;
       });
 
-      it('does not render GlModal', () => {
-        expect(findModal().exists()).toBeFalsy();
-      });
-    });
-
-    describe('when showModal is true', () => {
-      beforeEach(() => {
-        createComponent({ showModal: true });
-      });
-
-      it('renders GlModal', () => {
+      it('renders GlModal wtih data', () => {
         expect(findModal().exists()).toBeTruthy();
         expect(findModal().attributes()).toEqual(
           expect.objectContaining({
-            modalid: testModalProps.modalAttributes.modalId,
-            oktitle: testModalProps.modalAttributes.okTitle,
-            okvariant: testModalProps.modalAttributes.okVariant,
+            oktitle: MOCK_MODAL_DATA.modalAttributes.okTitle,
+            okvariant: MOCK_MODAL_DATA.modalAttributes.okVariant,
           }),
         );
       });
@@ -77,25 +71,49 @@ describe('vue_shared/components/confirm_modal', () => {
   });
 
   describe('methods', () => {
-    beforeEach(() => {
-      createComponent({ showModal: true });
-    });
+    describe('submitModal', () => {
+      beforeEach(() => {
+        createComponent();
+        wrapper.vm.path = MOCK_MODAL_DATA.path;
+        wrapper.vm.method = MOCK_MODAL_DATA.method;
+      });
+
+      it('does not submit form', () => {
+        expect(findForm().element.submit).not.toHaveBeenCalled();
+      });
+
+      describe('when modal submitted', () => {
+        beforeEach(() => {
+          findModal().vm.$emit('primary');
+        });
 
-    it('does not submit form', () => {
-      expect(findForm().element.submit).not.toHaveBeenCalled();
+        it('submits form', () => {
+          expect(findFormData()).toEqual([
+            { name: '_method', value: MOCK_MODAL_DATA.method },
+            { name: 'authenticity_token', value: 'test-csrf-token' },
+          ]);
+          expect(findForm().element.submit).toHaveBeenCalled();
+        });
+      });
     });
 
-    describe('when modal submitted', () => {
+    describe('closeModal', () => {
       beforeEach(() => {
-        findModal().vm.$emit('primary');
+        createComponent();
       });
 
-      it('submits form', () => {
-        expect(findFormData()).toEqual([
-          { name: '_method', value: testModalProps.method },
-          { name: 'authenticity_token', value: 'test-csrf-token' },
-        ]);
-        expect(findForm().element.submit).toHaveBeenCalled();
+      it('does not close modal', () => {
+        expect(actionSpies.closeModal).not.toHaveBeenCalled();
+      });
+
+      describe('when modal closed', () => {
+        beforeEach(() => {
+          findModal().vm.$emit('cancel');
+        });
+
+        it('closes modal', () => {
+          expect(actionSpies.closeModal).toHaveBeenCalled();
+        });
       });
     });
   });