Merge branch '220987-rich-content-editor-v-model' into 'master'

Remove rich_content_editor v-model

Closes #220987

See merge request gitlab-org/gitlab!34956

app/assets/javascripts/static_site_editor/components/edit_area.vue

@@ -37,41 +37,28 @@ export default {
       saveable: false,
       parsedSource: parseSourceFile(this.content),
       editorMode: EDITOR_TYPES.wysiwyg,
+      isModified: false,
     };
   },
   computed: {
     editableContent() {
-      return this.parsedSource.editable;
-    },
-    editableKey() {
-      return this.isWysiwygMode ? 'body' : 'raw';
+      return this.parsedSource.content(this.isWysiwygMode);
     },
     isWysiwygMode() {
       return this.editorMode === EDITOR_TYPES.wysiwyg;
     },
-    modified() {
-      return this.isWysiwygMode
-        ? this.parsedSource.isModifiedBody()
-        : this.parsedSource.isModifiedRaw();
-    },
   },
   methods: {
-    syncSource() {
-      if (this.isWysiwygMode) {
-        this.parsedSource.syncBodyToRaw();
-        return;
-      }
-
-      this.parsedSource.syncRawToBody();
+    onInputChange(newVal) {
+      this.parsedSource.sync(newVal, this.isWysiwygMode);
+      this.isModified = this.parsedSource.isModified();
     },
     onModeChange(mode) {
-      // Sequentially sync then switch modes (rich-content-editor's v-model computed source content update)
-      this.syncSource();
       this.editorMode = mode;
+      this.$refs.editor.resetInitialValue(this.editableContent);
     },
     onSubmit() {
-      this.syncSource();
-      this.$emit('submit', { content: this.editableContent.raw });
+      this.$emit('submit', { content: this.parsedSource.content() });
     },
   },
 };
@@ -80,16 +67,18 @@ export default {
   <div class="d-flex flex-grow-1 flex-column h-100">
     <edit-header class="py-2" :title="title" />
     <rich-content-editor
-      v-model="editableContent[editableKey]"
+      ref="editor"
+      :content="editableContent"
       :initial-edit-type="editorMode"
       class="mb-9 h-100"
       @modeChange="onModeChange"
+      @input="onInputChange"
     />
-    <unsaved-changes-confirm-dialog :modified="modified" />
+    <unsaved-changes-confirm-dialog :modified="isModified" />
     <publish-toolbar
       class="gl-fixed gl-left-0 gl-bottom-0 gl-w-full"
       :return-url="returnUrl"
-      :saveable="modified"
+      :saveable="isModified"
       :saving-changes="savingChanges"
       @submit="onSubmit"
     />

app/assets/javascripts/static_site_editor/services/parse_source_file.js

@@ -22,33 +22,43 @@ const parseSourceFile = raw => {
     return buildPayload(source, '', '', source);
   };
 
-  const computedRaw = () => `${editable.header}${editable.spacing}${editable.body}`;
-
-  const syncRawToBody = () => {
+  const syncEditable = () => {
     /*
     We re-parse as markdown editing could have added non-body changes (preFrontMatter, frontMatter, or spacing).
-    Re-parsing additionally gets us the desired body that was extracted from the mutated editable.raw
-    Additionally we intentionally mutate the existing editable's key values as opposed to reassigning the object itself so consumers of the potentially reactive property stay in sync.
+    Re-parsing additionally gets us the desired body that was extracted from the potentially mutated editable.raw
     */
-    Object.assign(editable, parse(editable.raw));
+    editable = parse(editable.raw);
   };
 
   const syncBodyToRaw = () => {
-    editable.raw = computedRaw();
+    editable.raw = `${editable.header}${editable.spacing}${editable.body}`;
+  };
+
+  const sync = (newVal, isBodyToRaw) => {
+    const editableKey = isBodyToRaw ? 'body' : 'raw';
+    editable[editableKey] = newVal;
+
+    if (isBodyToRaw) {
+      syncBodyToRaw();
+    }
+
+    syncEditable();
+  };
+
+  const content = (isBody = false) => {
+    const editableKey = isBody ? 'body' : 'raw';
+    return editable[editableKey];
   };
 
-  const isModifiedRaw = () => initial.raw !== editable.raw;
-  const isModifiedBody = () => initial.raw !== computedRaw();
+  const isModified = () => initial.raw !== editable.raw;
 
   initial = parse(raw);
   editable = parse(raw);
 
   return {
-    editable,
-    isModifiedRaw,
-    isModifiedBody,
-    syncBodyToRaw,
-    syncRawToBody,
+    content,
+    isModified,
+    sync,
   };
 };
 

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

@@ -27,7 +27,7 @@ export default {
     AddImageModal,
   },
   props: {
-    value: {
+    content: {
       type: String,
       required: true,
     },
@@ -66,23 +66,6 @@ export default {
       return this.$refs.editor;
     },
   },
-  watch: {
-    value(newVal) {
-      const isSameMode = this.previousMode === this.editorApi.currentMode;
-      if (!isSameMode) {
-        /*
-        The ToastUI Editor consumes its content via the `initial-value` prop and then internally
-        manages changes. If we desire the `v-model` to work as expected, we need to manually call
-        `setMarkdown`. However, if we do this in each v-model change we'll continually prevent
-        the editor from internally managing changes. Thus we use the `previousMode` flag as
-        confirmation to actually update its internals. This is initially designed so that front
-        matter is excluded from editing in wysiwyg mode, but included in markdown mode.
-        */
-        this.editorInstance.invoke('setMarkdown', newVal);
-        this.previousMode = this.editorApi.currentMode;
-      }
-    },
-  },
   beforeDestroy() {
     removeCustomEventListener(
       this.editorApi,
@@ -93,6 +76,9 @@ export default {
     this.editorApi.eventManager.removeEventHandler('changeMode', this.onChangeMode);
   },
   methods: {
+    resetInitialValue(newVal) {
+      this.editorInstance.invoke('setMarkdown', newVal);
+    },
     onContentChanged() {
       this.$emit('input', getMarkdown(this.editorInstance));
     },
@@ -123,7 +109,7 @@ export default {
   <div>
     <toast-editor
       ref="editor"
-      :initial-value="value"
+      :initial-value="content"
       :options="editorOptions"
       :preview-style="previewStyle"
       :initial-edit-type="initialEditType"

spec/frontend/static_site_editor/components/edit_area_spec.js

@@ -52,7 +52,7 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
 
   it('renders rich content editor', () => {
     expect(findRichContentEditor().exists()).toBe(true);
-    expect(findRichContentEditor().props('value')).toBe(body);
+    expect(findRichContentEditor().props('content')).toBe(body);
   });
 
   it('renders publish toolbar', () => {
@@ -76,6 +76,15 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
       return wrapper.vm.$nextTick();
     });
 
+    it('updates parsedSource with new content', () => {
+      const newContent = 'New content';
+      const spySyncParsedSource = jest.spyOn(wrapper.vm.parsedSource, 'sync');
+
+      findRichContentEditor().vm.$emit('input', newContent);
+
+      expect(spySyncParsedSource).toHaveBeenCalledWith(newContent, true);
+    });
+
     it('sets publish toolbar as saveable', () => {
       expect(findPublishToolbar().props('saveable')).toBe(true);
     });
@@ -103,35 +112,21 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
     });
 
     it.each`
-      initialMode              | targetMode
-      ${EDITOR_TYPES.wysiwyg}  | ${EDITOR_TYPES.markdown}
-      ${EDITOR_TYPES.markdown} | ${EDITOR_TYPES.wysiwyg}
-    `('sets editorMode from $initialMode to $targetMode', ({ initialMode, targetMode }) => {
-      setInitialMode(initialMode);
-      findRichContentEditor().vm.$emit('modeChange', targetMode);
-
-      expect(wrapper.vm.editorMode).toBe(targetMode);
-    });
-
-    it.each`
-      syncFnName         | initialMode              | targetMode
-      ${'syncBodyToRaw'} | ${EDITOR_TYPES.wysiwyg}  | ${EDITOR_TYPES.markdown}
-      ${'syncRawToBody'} | ${EDITOR_TYPES.markdown} | ${EDITOR_TYPES.wysiwyg}
+      initialMode              | targetMode               | resetValue
+      ${EDITOR_TYPES.wysiwyg}  | ${EDITOR_TYPES.markdown} | ${content}
+      ${EDITOR_TYPES.markdown} | ${EDITOR_TYPES.wysiwyg}  | ${body}
     `(
-      'calls $syncFnName source before switching from $initialMode to $targetMode',
-      ({ syncFnName, initialMode, targetMode }) => {
+      'sets editorMode from $initialMode to $targetMode',
+      ({ initialMode, targetMode, resetValue }) => {
         setInitialMode(initialMode);
 
-        const spySyncSource = jest.spyOn(wrapper.vm, 'syncSource');
-        const spySyncParsedSource = jest.spyOn(wrapper.vm.parsedSource, syncFnName);
+        const resetInitialValue = jest.fn();
 
+        findRichContentEditor().setMethods({ resetInitialValue });
         findRichContentEditor().vm.$emit('modeChange', targetMode);
 
-        expect(spySyncSource).toHaveBeenCalled();
-        expect(spySyncParsedSource).toHaveBeenCalled();
-
-        spySyncSource.mockReset();
-        spySyncParsedSource.mockReset();
+        expect(resetInitialValue).toHaveBeenCalledWith(resetValue);
+        expect(wrapper.vm.editorMode).toBe(targetMode);
       },
     );
   });

spec/frontend/static_site_editor/services/parse_source_file_spec.js

-import {
-  sourceContent as content,
-  sourceContentHeader as header,
-  sourceContentSpacing as spacing,
-  sourceContentBody as body,
-} from '../mock_data';
+import { sourceContent as content, sourceContentBody as body } from '../mock_data';
 
 import parseSourceFile from '~/static_site_editor/services/parse_source_file';
 
 describe('parseSourceFile', () => {
-  const contentSimple = content;
   const contentComplex = [content, content, content].join('');
+  const complexBody = [body, content, content].join('');
+  const edit = 'and more';
+  const newContent = `${content} ${edit}`;
+  const newContentComplex = `${contentComplex} ${edit}`;
 
-  describe('the editable shape and its expected values', () => {
+  describe('unmodified content', () => {
     it.each`
-      sourceContent     | sourceHeader | sourceSpacing | sourceBody                           | desc
-      ${contentSimple}  | ${header}    | ${spacing}    | ${body}                              | ${'extracts header'}
-      ${contentComplex} | ${header}    | ${spacing}    | ${[body, content, content].join('')} | ${'extracts body'}
-    `('$desc', ({ sourceContent, sourceHeader, sourceSpacing, sourceBody }) => {
-      const { editable } = parseSourceFile(sourceContent);
-
-      expect(editable).toMatchObject({
-        raw: sourceContent,
-        header: sourceHeader,
-        spacing: sourceSpacing,
-        body: sourceBody,
-      });
-    });
-
-    it('returns the same front matter regardless of front matter duplication', () => {
-      const parsedSourceSimple = parseSourceFile(contentSimple);
-      const parsedSourceComplex = parseSourceFile(contentComplex);
-
-      expect(parsedSourceSimple.editable.header).toBe(parsedSourceComplex.editable.header);
-    });
+      parsedSource
+      ${parseSourceFile(content)}
+      ${parseSourceFile(contentComplex)}
+    `('returns false by default', ({ parsedSource }) => {
+      expect(parsedSource.isModified()).toBe(false);
     });
 
-  describe('editable body to raw content default and changes', () => {
     it.each`
-      sourceContent     | desc
-      ${contentSimple}  | ${'returns false by default for both raw and body'}
-      ${contentComplex} | ${'returns false by default for both raw and body'}
-    `('$desc', ({ sourceContent }) => {
-      const parsedSource = parseSourceFile(sourceContent);
-
-      expect(parsedSource.isModifiedRaw()).toBe(false);
-      expect(parsedSource.isModifiedBody()).toBe(false);
+      parsedSource                       | isBody       | target
+      ${parseSourceFile(content)}        | ${undefined} | ${content}
+      ${parseSourceFile(content)}        | ${false}     | ${content}
+      ${parseSourceFile(content)}        | ${true}      | ${body}
+      ${parseSourceFile(contentComplex)} | ${undefined} | ${contentComplex}
+      ${parseSourceFile(contentComplex)} | ${false}     | ${contentComplex}
+      ${parseSourceFile(contentComplex)} | ${true}      | ${complexBody}
+    `(
+      'returns only the $target content when the `isBody` parameter argument is $isBody',
+      ({ parsedSource, isBody, target }) => {
+        expect(parsedSource.content(isBody)).toBe(target);
+      },
+    );
   });
 
-    it.each`
-      sourceContent     | editableKey | syncKey            | isModifiedKey       | desc
-      ${contentSimple}  | ${'body'}   | ${'syncBodyToRaw'} | ${'isModifiedRaw'}  | ${'returns true after modification and sync'}
-      ${contentSimple}  | ${'raw'}    | ${'syncRawToBody'} | ${'isModifiedBody'} | ${'returns true after modification and sync'}
-      ${contentComplex} | ${'body'}   | ${'syncBodyToRaw'} | ${'isModifiedRaw'}  | ${'returns true after modification and sync'}
-      ${contentComplex} | ${'raw'}    | ${'syncRawToBody'} | ${'isModifiedBody'} | ${'returns true after modification and sync'}
-    `('$desc', ({ sourceContent, editableKey, syncKey, isModifiedKey }) => {
-      const parsedSource = parseSourceFile(sourceContent);
-      parsedSource.editable[editableKey] += 'Added content';
-      parsedSource[syncKey]();
+  describe('modified content', () => {
+    const newBody = `${body} ${edit}`;
+    const newComplexBody = `${complexBody} ${edit}`;
 
-      expect(parsedSource[isModifiedKey]()).toBe(true);
-    });
+    it.each`
+      parsedSource                       | isModified | targetRaw            | targetBody
+      ${parseSourceFile(content)}        | ${false}   | ${content}           | ${body}
+      ${parseSourceFile(content)}        | ${true}    | ${newContent}        | ${newBody}
+      ${parseSourceFile(contentComplex)} | ${false}   | ${contentComplex}    | ${complexBody}
+      ${parseSourceFile(contentComplex)} | ${true}    | ${newContentComplex} | ${newComplexBody}
+    `(
+      'returns $isModified after a $targetRaw sync',
+      ({ parsedSource, isModified, targetRaw, targetBody }) => {
+        parsedSource.sync(targetRaw);
+
+        expect(parsedSource.isModified()).toBe(isModified);
+        expect(parsedSource.content()).toBe(targetRaw);
+        expect(parsedSource.content(true)).toBe(targetBody);
+      },
+    );
   });
 });

spec/frontend/vue_shared/components/rich_content_editor/rich_content_editor_spec.js

@@ -25,13 +25,13 @@ jest.mock('~/vue_shared/components/rich_content_editor/services/editor_service',
 describe('Rich Content Editor', () => {
   let wrapper;
 
-  const value = '## Some Markdown';
+  const content = '## Some Markdown';
   const findEditor = () => wrapper.find({ ref: 'editor' });
   const findAddImageModal = () => wrapper.find(AddImageModal);
 
   beforeEach(() => {
     wrapper = shallowMount(RichContentEditor, {
-      propsData: { value },
+      propsData: { content },
     });
   });
 
@@ -41,7 +41,7 @@ describe('Rich Content Editor', () => {
     });
 
     it('renders the correct content', () => {
-      expect(findEditor().props().initialValue).toBe(value);
+      expect(findEditor().props().initialValue).toBe(content);
     });
 
     it('provides the correct editor options', () => {
@@ -73,6 +73,18 @@ describe('Rich Content Editor', () => {
     });
   });
 
+  describe('when content is reset', () => {
+    it('should reset the content via setMarkdown', () => {
+      const newContent = 'Just the body content excluding the front matter for example';
+      const mockInstance = { invoke: jest.fn() };
+      wrapper.vm.$refs.editor = mockInstance;
+
+      wrapper.vm.resetInitialValue(newContent);
+
+      expect(mockInstance.invoke).toHaveBeenCalledWith('setMarkdown', newContent);
+    });
+  });
+
   describe('when editor is loaded', () => {
     it('adds the CUSTOM_EVENTS.openAddImageModal custom event listener', () => {
       const mockEditorApi = { eventManager: { addEventType: jest.fn(), listen: jest.fn() } };