Merge branch 'ps-fix-attach-a-file' into 'master'

Fix "Attach a file" not working after entering text

See merge request gitlab-org/gitlab!44216

app/assets/javascripts/gl_form.js

@@ -6,7 +6,14 @@ import { addMarkdownListeners, removeMarkdownListeners } from './lib/utils/text_
 import { disableButtonIfEmptyField } from '~/lib/utils/common_utils';
 
 export default class GLForm {
-  constructor(form, enableGFM = {}) {
+  /**
+   * Create a GLForm
+   *
+   * @param {jQuery} form Root element of the GLForm
+   * @param {Object} enableGFM Which autocomplete features should be enabled?
+   * @param {Boolean} forceNew If true, treat the element as a **new** form even if `gfm-form` class already exists.
+   */
+  constructor(form, enableGFM = {}, forceNew = false) {
     this.form = form;
     this.textarea = this.form.find('textarea.js-gfm-input');
     this.enableGFM = { ...defaultAutocompleteConfig, ...enableGFM };
@@ -22,7 +29,7 @@ export default class GLForm {
     // Before we start, we should clean up any previous data for this form
     this.destroy();
     // Set up the form
-    this.setupForm();
+    this.setupForm(forceNew);
     this.form.data('glForm', this);
   }
 
@@ -39,8 +46,8 @@ export default class GLForm {
     this.form.data('glForm', null);
   }
 
-  setupForm() {
-    const isNewForm = this.form.is(':not(.gfm-form)');
+  setupForm(forceNew = false) {
+    const isNewForm = this.form.is(':not(.gfm-form)') || forceNew;
     this.form.removeClass('js-new-note-form');
     if (isNewForm) {
       this.form.find('.div-dropzone').remove();

app/assets/javascripts/vue_shared/components/markdown/field.vue

@@ -167,7 +167,9 @@ export default {
   },
   mounted() {
     // GLForm class handles all the toolbar buttons
-    return new GLForm($(this.$refs['gl-form']), {
+    return new GLForm(
+      $(this.$refs['gl-form']),
+      {
         emojis: this.enableAutocomplete,
         members: this.enableAutocomplete && !this.glFeatures.tributeAutocomplete,
         issues: this.enableAutocomplete && !this.glFeatures.tributeAutocomplete,
@@ -177,7 +179,9 @@ export default {
         labels: this.enableAutocomplete && !this.glFeatures.tributeAutocomplete,
         snippets: this.enableAutocomplete,
         vulnerabilities: this.enableAutocomplete,
-    });
+      },
+      true,
+    );
   },
   beforeDestroy() {
     const glForm = $(this.$refs['gl-form']).data('glForm');
@@ -230,7 +234,7 @@ export default {
   <div
     ref="gl-form"
     :class="{ 'gl-mt-3 gl-mb-3': addSpacingClasses }"
-    class="js-vue-markdown-field md-area position-relative"
+    class="js-vue-markdown-field md-area position-relative gfm-form"
   >
     <markdown-header
       :preview-markdown="previewMarkdown"

changelogs/unreleased/ps-fix-attach-a-file.yml

+---
+title: Fix attach file button not working in description fields
+merge_request: 44216
+author:
+type: fixed

spec/frontend/snippets/components/__snapshots__/snippet_description_edit_spec.js.snap

@@ -24,7 +24,7 @@ exports[`Snippet Description Edit component rendering matches the snapshot 1`] =
     </div>
      
     <div
-      class="js-vue-markdown-field md-area position-relative js-expanded gfm-form"
+      class="js-vue-markdown-field md-area position-relative gfm-form js-expanded"
     >
       <markdown-header-stub
         linecontent=""

spec/frontend/vue_shared/components/markdown/field_spec.js

@@ -2,12 +2,13 @@ import { mount } from '@vue/test-utils';
 import { TEST_HOST, FIXTURES_PATH } from 'spec/test_constants';
 import AxiosMockAdapter from 'axios-mock-adapter';
 import $ from 'jquery';
-import fieldComponent from '~/vue_shared/components/markdown/field.vue';
+import MarkdownField from '~/vue_shared/components/markdown/field.vue';
 import axios from '~/lib/utils/axios_utils';
 
 const markdownPreviewPath = `${TEST_HOST}/preview`;
 const markdownDocsPath = `${TEST_HOST}/docs`;
 const textareaValue = 'testing\n123';
+const uploadsPath = 'test/uploads';
 
 function assertMarkdownTabs(isWrite, writeLink, previewLink, wrapper) {
   expect(writeLink.element.parentNode.classList.contains('active')).toBe(isWrite);
@@ -15,54 +16,81 @@ function assertMarkdownTabs(isWrite, writeLink, previewLink, wrapper) {
   expect(wrapper.find('.md-preview-holder').element.style.display).toBe(isWrite ? 'none' : '');
 }
 
-function createComponent() {
-  const wrapper = mount(fieldComponent, {
-    propsData: {
-      markdownDocsPath,
-      markdownPreviewPath,
-      isSubmitting: false,
-      textareaValue,
-    },
-    slots: {
-      textarea: `<textarea>${textareaValue}</textarea>`,
-    },
-  });
-  return wrapper;
-}
-
-const getPreviewLink = wrapper => wrapper.find('.nav-links .js-preview-link');
-const getWriteLink = wrapper => wrapper.find('.nav-links .js-write-link');
-const getMarkdownButton = wrapper => wrapper.find('.js-md');
-const getAllMarkdownButtons = wrapper => wrapper.findAll('.js-md');
-const getVideo = wrapper => wrapper.find('video');
-
 describe('Markdown field component', () => {
   let axiosMock;
+  let subject;
 
   beforeEach(() => {
     axiosMock = new AxiosMockAdapter(axios);
+    // window.uploads_path is needed for dropzone to initialize
+    window.uploads_path = uploadsPath;
   });
 
   afterEach(() => {
+    subject.destroy();
+    subject = null;
     axiosMock.restore();
   });
 
+  function createSubject() {
+    // We actually mount a wrapper component so that we can force Vue to rerender classes in order to test a regression
+    // caused by mixing Vanilla JS and Vue.
+    subject = mount(
+      {
+        components: {
+          MarkdownField,
+        },
+        props: {
+          wrapperClasses: {
+            type: String,
+            required: false,
+            default: '',
+          },
+        },
+        template: `
+<markdown-field :class="wrapperClasses" v-bind="$attrs">
+  <template #textarea>
+    <textarea class="js-gfm-input" :value="$attrs.textareaValue"></textarea>
+  </template>
+</markdown-field>`,
+      },
+      {
+        propsData: {
+          markdownDocsPath,
+          markdownPreviewPath,
+          isSubmitting: false,
+          textareaValue,
+        },
+      },
+    );
+  }
+
+  const getPreviewLink = () => subject.find('.nav-links .js-preview-link');
+  const getWriteLink = () => subject.find('.nav-links .js-write-link');
+  const getMarkdownButton = () => subject.find('.js-md');
+  const getAllMarkdownButtons = () => subject.findAll('.js-md');
+  const getVideo = () => subject.find('video');
+  const getAttachButton = () => subject.find('.button-attach-file');
+  const clickAttachButton = () => getAttachButton().trigger('click');
+  const findDropzone = () => subject.find('.div-dropzone');
+
   describe('mounted', () => {
-    let wrapper;
     const previewHTML = `
     <p>markdown preview</p>
     <video src="${FIXTURES_PATH}/static/mock-video.mp4" muted="muted"></video>
   `;
     let previewLink;
     let writeLink;
+    let dropzoneSpy;
 
-    afterEach(() => {
-      wrapper.destroy();
+    beforeEach(() => {
+      dropzoneSpy = jest.fn();
+      createSubject();
+      findDropzone().element.addEventListener('click', dropzoneSpy);
     });
 
     it('renders textarea inside backdrop', () => {
-      wrapper = createComponent();
-      expect(wrapper.find('.zen-backdrop textarea').element).not.toBeNull();
+      expect(subject.find('.zen-backdrop textarea').element).not.toBeNull();
     });
 
     describe('markdown preview', () => {
@@ -71,44 +99,40 @@ describe('Markdown field component', () => {
       });
 
       it('sets preview link as active', () => {
-        wrapper = createComponent();
-        previewLink = getPreviewLink(wrapper);
+        previewLink = getPreviewLink();
         previewLink.trigger('click');
 
-        return wrapper.vm.$nextTick().then(() => {
+        return subject.vm.$nextTick().then(() => {
           expect(previewLink.element.parentNode.classList.contains('active')).toBeTruthy();
         });
       });
 
       it('shows preview loading text', () => {
-        wrapper = createComponent();
-        previewLink = getPreviewLink(wrapper);
+        previewLink = getPreviewLink();
         previewLink.trigger('click');
 
-        return wrapper.vm.$nextTick(() => {
-          expect(wrapper.find('.md-preview-holder').element.textContent.trim()).toContain(
+        return subject.vm.$nextTick(() => {
+          expect(subject.find('.md-preview-holder').element.textContent.trim()).toContain(
             'Loading…',
           );
         });
       });
 
       it('renders markdown preview and GFM', () => {
-        wrapper = createComponent();
         const renderGFMSpy = jest.spyOn($.fn, 'renderGFM');
 
-        previewLink = getPreviewLink(wrapper);
+        previewLink = getPreviewLink();
 
         previewLink.trigger('click');
 
         return axios.waitFor(markdownPreviewPath).then(() => {
-          expect(wrapper.find('.md-preview-holder').element.innerHTML).toContain(previewHTML);
+          expect(subject.find('.md-preview-holder').element.innerHTML).toContain(previewHTML);
           expect(renderGFMSpy).toHaveBeenCalled();
         });
       });
 
       it('calls video.pause() on comment input when isSubmitting is changed to true', () => {
-        wrapper = createComponent();
-        previewLink = getPreviewLink(wrapper);
+        previewLink = getPreviewLink();
         previewLink.trigger('click');
 
         let callPause;
@@ -116,79 +140,107 @@ describe('Markdown field component', () => {
         return axios
           .waitFor(markdownPreviewPath)
           .then(() => {
-            const video = getVideo(wrapper);
+            const video = getVideo();
             callPause = jest.spyOn(video.element, 'pause').mockImplementation(() => true);
 
-            wrapper.setProps({
-              isSubmitting: true,
-              markdownPreviewPath,
-              markdownDocsPath,
-            });
+            subject.setProps({ isSubmitting: true });
 
-            return wrapper.vm.$nextTick();
+            return subject.vm.$nextTick();
           })
           .then(() => {
             expect(callPause).toHaveBeenCalled();
           });
       });
 
-      it('clicking already active write or preview link does nothing', () => {
-        wrapper = createComponent();
-        writeLink = getWriteLink(wrapper);
-        previewLink = getPreviewLink(wrapper);
+      it('clicking already active write or preview link does nothing', async () => {
+        writeLink = getWriteLink();
+        previewLink = getPreviewLink();
 
         writeLink.trigger('click');
-        return wrapper.vm
-          .$nextTick()
-          .then(() => assertMarkdownTabs(true, writeLink, previewLink, wrapper))
-          .then(() => writeLink.trigger('click'))
-          .then(() => wrapper.vm.$nextTick())
-          .then(() => assertMarkdownTabs(true, writeLink, previewLink, wrapper))
-          .then(() => previewLink.trigger('click'))
-          .then(() => wrapper.vm.$nextTick())
-          .then(() => assertMarkdownTabs(false, writeLink, previewLink, wrapper))
-          .then(() => previewLink.trigger('click'))
-          .then(() => wrapper.vm.$nextTick())
-          .then(() => assertMarkdownTabs(false, writeLink, previewLink, wrapper));
+        await subject.vm.$nextTick();
+
+        assertMarkdownTabs(true, writeLink, previewLink, subject);
+        writeLink.trigger('click');
+        await subject.vm.$nextTick();
+
+        assertMarkdownTabs(true, writeLink, previewLink, subject);
+        previewLink.trigger('click');
+        await subject.vm.$nextTick();
+
+        assertMarkdownTabs(false, writeLink, previewLink, subject);
+        previewLink.trigger('click');
+        await subject.vm.$nextTick();
+
+        assertMarkdownTabs(false, writeLink, previewLink, subject);
       });
     });
 
     describe('markdown buttons', () => {
       it('converts single words', () => {
-        wrapper = createComponent();
-        const textarea = wrapper.find('textarea').element;
+        const textarea = subject.find('textarea').element;
         textarea.setSelectionRange(0, 7);
-        const markdownButton = getMarkdownButton(wrapper);
+        const markdownButton = getMarkdownButton();
         markdownButton.trigger('click');
 
-        return wrapper.vm.$nextTick(() => {
+        return subject.vm.$nextTick(() => {
           expect(textarea.value).toContain('**testing**');
         });
       });
 
       it('converts a line', () => {
-        wrapper = createComponent();
-        const textarea = wrapper.find('textarea').element;
+        const textarea = subject.find('textarea').element;
         textarea.setSelectionRange(0, 0);
-        const markdownButton = getAllMarkdownButtons(wrapper).wrappers[5];
+        const markdownButton = getAllMarkdownButtons().wrappers[5];
         markdownButton.trigger('click');
 
-        return wrapper.vm.$nextTick(() => {
+        return subject.vm.$nextTick(() => {
           expect(textarea.value).toContain('- testing');
         });
       });
 
       it('converts multiple lines', () => {
-        wrapper = createComponent();
-        const textarea = wrapper.find('textarea').element;
+        const textarea = subject.find('textarea').element;
         textarea.setSelectionRange(0, 50);
-        const markdownButton = getAllMarkdownButtons(wrapper).wrappers[5];
+        const markdownButton = getAllMarkdownButtons().wrappers[5];
         markdownButton.trigger('click');
 
-        return wrapper.vm.$nextTick(() => {
+        return subject.vm.$nextTick(() => {
           expect(textarea.value).toContain('- testing\n- 123');
         });
       });
     });
+
+    it('should render attach a file button', () => {
+      expect(getAttachButton().text()).toBe('Attach a file');
+    });
+
+    it('should trigger dropzone when attach button is clicked', () => {
+      expect(dropzoneSpy).not.toHaveBeenCalled();
+
+      clickAttachButton();
+
+      expect(dropzoneSpy).toHaveBeenCalled();
+    });
+
+    describe('when textarea has changed', () => {
+      beforeEach(async () => {
+        // Do something to trigger rerendering the class
+        subject.setProps({ wrapperClasses: 'foo' });
+
+        await subject.vm.$nextTick();
+      });
+
+      it('should have rerendered classes and kept gfm-form', () => {
+        expect(subject.classes()).toEqual(expect.arrayContaining(['gfm-form', 'foo']));
+      });
+
+      it('should trigger dropzone when attach button is clicked', () => {
+        expect(dropzoneSpy).not.toHaveBeenCalled();
+
+        clickAttachButton();
+
+        expect(dropzoneSpy).toHaveBeenCalled();
+      });
+    });
   });
 });