Commit 6030369a authored by derek-knox's avatar derek-knox

Remove rich_content_editor v-model

The previous v-model approach broke the contract
expectation in that under the hood, rich_content_editor
uses the toast-editor component that only takes an
initial value and internally manages its state.
I also cleaned up edit_area by properly encapuslating
parse_source_file's concerns in it where previously
they crossed over
parent 1b91dd8e
...@@ -37,41 +37,28 @@ export default { ...@@ -37,41 +37,28 @@ export default {
saveable: false, saveable: false,
parsedSource: parseSourceFile(this.content), parsedSource: parseSourceFile(this.content),
editorMode: EDITOR_TYPES.wysiwyg, editorMode: EDITOR_TYPES.wysiwyg,
isModified: false,
}; };
}, },
computed: { computed: {
editableContent() { editableContent() {
return this.parsedSource.editable; return this.parsedSource.content(this.isWysiwygMode);
},
editableKey() {
return this.isWysiwygMode ? 'body' : 'raw';
}, },
isWysiwygMode() { isWysiwygMode() {
return this.editorMode === EDITOR_TYPES.wysiwyg; return this.editorMode === EDITOR_TYPES.wysiwyg;
}, },
modified() {
return this.isWysiwygMode
? this.parsedSource.isModifiedBody()
: this.parsedSource.isModifiedRaw();
},
}, },
methods: { methods: {
syncSource() { onInputChange(newVal) {
if (this.isWysiwygMode) { this.parsedSource.sync(newVal, this.isWysiwygMode);
this.parsedSource.syncBodyToRaw(); this.isModified = this.parsedSource.isModified();
return;
}
this.parsedSource.syncRawToBody();
}, },
onModeChange(mode) { onModeChange(mode) {
// Sequentially sync then switch modes (rich-content-editor's v-model computed source content update)
this.syncSource();
this.editorMode = mode; this.editorMode = mode;
this.$refs.editor.resetInitialValue(this.editableContent);
}, },
onSubmit() { onSubmit() {
this.syncSource(); this.$emit('submit', { content: this.parsedSource.content() });
this.$emit('submit', { content: this.editableContent.raw });
}, },
}, },
}; };
...@@ -80,16 +67,18 @@ export default { ...@@ -80,16 +67,18 @@ export default {
<div class="d-flex flex-grow-1 flex-column h-100"> <div class="d-flex flex-grow-1 flex-column h-100">
<edit-header class="py-2" :title="title" /> <edit-header class="py-2" :title="title" />
<rich-content-editor <rich-content-editor
v-model="editableContent[editableKey]" ref="editor"
:content="editableContent"
:initial-edit-type="editorMode" :initial-edit-type="editorMode"
class="mb-9 h-100" class="mb-9 h-100"
@modeChange="onModeChange" @modeChange="onModeChange"
@input="onInputChange"
/> />
<unsaved-changes-confirm-dialog :modified="modified" /> <unsaved-changes-confirm-dialog :modified="isModified" />
<publish-toolbar <publish-toolbar
class="gl-fixed gl-left-0 gl-bottom-0 gl-w-full" class="gl-fixed gl-left-0 gl-bottom-0 gl-w-full"
:return-url="returnUrl" :return-url="returnUrl"
:saveable="modified" :saveable="isModified"
:saving-changes="savingChanges" :saving-changes="savingChanges"
@submit="onSubmit" @submit="onSubmit"
/> />
......
...@@ -22,33 +22,43 @@ const parseSourceFile = raw => { ...@@ -22,33 +22,43 @@ const parseSourceFile = raw => {
return buildPayload(source, '', '', source); return buildPayload(source, '', '', source);
}; };
const computedRaw = () => `${editable.header}${editable.spacing}${editable.body}`; const syncEditable = () => {
const syncRawToBody = () => {
/* /*
We re-parse as markdown editing could have added non-body changes (preFrontMatter, frontMatter, or spacing). 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 Re-parsing additionally gets us the desired body that was extracted from the potentially 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.
*/ */
Object.assign(editable, parse(editable.raw)); editable = parse(editable.raw);
}; };
const syncBodyToRaw = () => { 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 isModified = () => initial.raw !== editable.raw;
const isModifiedBody = () => initial.raw !== computedRaw();
initial = parse(raw); initial = parse(raw);
editable = parse(raw); editable = parse(raw);
return { return {
editable, content,
isModifiedRaw, isModified,
isModifiedBody, sync,
syncBodyToRaw,
syncRawToBody,
}; };
}; };
......
...@@ -27,7 +27,7 @@ export default { ...@@ -27,7 +27,7 @@ export default {
AddImageModal, AddImageModal,
}, },
props: { props: {
value: { content: {
type: String, type: String,
required: true, required: true,
}, },
...@@ -66,23 +66,6 @@ export default { ...@@ -66,23 +66,6 @@ export default {
return this.$refs.editor; 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() { beforeDestroy() {
removeCustomEventListener( removeCustomEventListener(
this.editorApi, this.editorApi,
...@@ -93,6 +76,9 @@ export default { ...@@ -93,6 +76,9 @@ export default {
this.editorApi.eventManager.removeEventHandler('changeMode', this.onChangeMode); this.editorApi.eventManager.removeEventHandler('changeMode', this.onChangeMode);
}, },
methods: { methods: {
resetInitialValue(newVal) {
this.editorInstance.invoke('setMarkdown', newVal);
},
onContentChanged() { onContentChanged() {
this.$emit('input', getMarkdown(this.editorInstance)); this.$emit('input', getMarkdown(this.editorInstance));
}, },
...@@ -123,7 +109,7 @@ export default { ...@@ -123,7 +109,7 @@ export default {
<div> <div>
<toast-editor <toast-editor
ref="editor" ref="editor"
:initial-value="value" :initial-value="content"
:options="editorOptions" :options="editorOptions"
:preview-style="previewStyle" :preview-style="previewStyle"
:initial-edit-type="initialEditType" :initial-edit-type="initialEditType"
......
...@@ -52,7 +52,7 @@ describe('~/static_site_editor/components/edit_area.vue', () => { ...@@ -52,7 +52,7 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
it('renders rich content editor', () => { it('renders rich content editor', () => {
expect(findRichContentEditor().exists()).toBe(true); expect(findRichContentEditor().exists()).toBe(true);
expect(findRichContentEditor().props('value')).toBe(body); expect(findRichContentEditor().props('content')).toBe(body);
}); });
it('renders publish toolbar', () => { it('renders publish toolbar', () => {
...@@ -76,6 +76,15 @@ describe('~/static_site_editor/components/edit_area.vue', () => { ...@@ -76,6 +76,15 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
return wrapper.vm.$nextTick(); 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', () => { it('sets publish toolbar as saveable', () => {
expect(findPublishToolbar().props('saveable')).toBe(true); expect(findPublishToolbar().props('saveable')).toBe(true);
}); });
...@@ -103,35 +112,21 @@ describe('~/static_site_editor/components/edit_area.vue', () => { ...@@ -103,35 +112,21 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
}); });
it.each` it.each`
initialMode | targetMode initialMode | targetMode | resetValue
${EDITOR_TYPES.wysiwyg} | ${EDITOR_TYPES.markdown} ${EDITOR_TYPES.wysiwyg} | ${EDITOR_TYPES.markdown} | ${content}
${EDITOR_TYPES.markdown} | ${EDITOR_TYPES.wysiwyg} ${EDITOR_TYPES.markdown} | ${EDITOR_TYPES.wysiwyg} | ${body}
`('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}
`( `(
'calls $syncFnName source before switching from $initialMode to $targetMode', 'sets editorMode from $initialMode to $targetMode',
({ syncFnName, initialMode, targetMode }) => { ({ initialMode, targetMode, resetValue }) => {
setInitialMode(initialMode); setInitialMode(initialMode);
const spySyncSource = jest.spyOn(wrapper.vm, 'syncSource'); const resetInitialValue = jest.fn();
const spySyncParsedSource = jest.spyOn(wrapper.vm.parsedSource, syncFnName);
findRichContentEditor().setMethods({ resetInitialValue });
findRichContentEditor().vm.$emit('modeChange', targetMode); findRichContentEditor().vm.$emit('modeChange', targetMode);
expect(spySyncSource).toHaveBeenCalled(); expect(resetInitialValue).toHaveBeenCalledWith(resetValue);
expect(spySyncParsedSource).toHaveBeenCalled(); expect(wrapper.vm.editorMode).toBe(targetMode);
spySyncSource.mockReset();
spySyncParsedSource.mockReset();
}, },
); );
}); });
......
import { import { sourceContent as content, sourceContentBody as body } from '../mock_data';
sourceContent as content,
sourceContentHeader as header,
sourceContentSpacing as spacing,
sourceContentBody as body,
} from '../mock_data';
import parseSourceFile from '~/static_site_editor/services/parse_source_file'; import parseSourceFile from '~/static_site_editor/services/parse_source_file';
describe('parseSourceFile', () => { describe('parseSourceFile', () => {
const contentSimple = content;
const contentComplex = [content, content, content].join(''); 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` it.each`
sourceContent | sourceHeader | sourceSpacing | sourceBody | desc parsedSource
${contentSimple} | ${header} | ${spacing} | ${body} | ${'extracts header'} ${parseSourceFile(content)}
${contentComplex} | ${header} | ${spacing} | ${[body, content, content].join('')} | ${'extracts body'} ${parseSourceFile(contentComplex)}
`('$desc', ({ sourceContent, sourceHeader, sourceSpacing, sourceBody }) => { `('returns false by default', ({ parsedSource }) => {
const { editable } = parseSourceFile(sourceContent); expect(parsedSource.isModified()).toBe(false);
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);
});
}); });
describe('editable body to raw content default and changes', () => {
it.each` it.each`
sourceContent | desc parsedSource | isBody | target
${contentSimple} | ${'returns false by default for both raw and body'} ${parseSourceFile(content)} | ${undefined} | ${content}
${contentComplex} | ${'returns false by default for both raw and body'} ${parseSourceFile(content)} | ${false} | ${content}
`('$desc', ({ sourceContent }) => { ${parseSourceFile(content)} | ${true} | ${body}
const parsedSource = parseSourceFile(sourceContent); ${parseSourceFile(contentComplex)} | ${undefined} | ${contentComplex}
${parseSourceFile(contentComplex)} | ${false} | ${contentComplex}
expect(parsedSource.isModifiedRaw()).toBe(false); ${parseSourceFile(contentComplex)} | ${true} | ${complexBody}
expect(parsedSource.isModifiedBody()).toBe(false); `(
'returns only the $target content when the `isBody` parameter argument is $isBody',
({ parsedSource, isBody, target }) => {
expect(parsedSource.content(isBody)).toBe(target);
},
);
}); });
it.each` describe('modified content', () => {
sourceContent | editableKey | syncKey | isModifiedKey | desc const newBody = `${body} ${edit}`;
${contentSimple} | ${'body'} | ${'syncBodyToRaw'} | ${'isModifiedRaw'} | ${'returns true after modification and sync'} const newComplexBody = `${complexBody} ${edit}`;
${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]();
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);
},
);
}); });
}); });
...@@ -25,13 +25,13 @@ jest.mock('~/vue_shared/components/rich_content_editor/services/editor_service', ...@@ -25,13 +25,13 @@ jest.mock('~/vue_shared/components/rich_content_editor/services/editor_service',
describe('Rich Content Editor', () => { describe('Rich Content Editor', () => {
let wrapper; let wrapper;
const value = '## Some Markdown'; const content = '## Some Markdown';
const findEditor = () => wrapper.find({ ref: 'editor' }); const findEditor = () => wrapper.find({ ref: 'editor' });
const findAddImageModal = () => wrapper.find(AddImageModal); const findAddImageModal = () => wrapper.find(AddImageModal);
beforeEach(() => { beforeEach(() => {
wrapper = shallowMount(RichContentEditor, { wrapper = shallowMount(RichContentEditor, {
propsData: { value }, propsData: { content },
}); });
}); });
...@@ -41,7 +41,7 @@ describe('Rich Content Editor', () => { ...@@ -41,7 +41,7 @@ describe('Rich Content Editor', () => {
}); });
it('renders the correct content', () => { it('renders the correct content', () => {
expect(findEditor().props().initialValue).toBe(value); expect(findEditor().props().initialValue).toBe(content);
}); });
it('provides the correct editor options', () => { it('provides the correct editor options', () => {
...@@ -73,6 +73,18 @@ describe('Rich Content Editor', () => { ...@@ -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', () => { describe('when editor is loaded', () => {
it('adds the CUSTOM_EVENTS.openAddImageModal custom event listener', () => { it('adds the CUSTOM_EVENTS.openAddImageModal custom event listener', () => {
const mockEditorApi = { eventManager: { addEventType: jest.fn(), listen: jest.fn() } }; const mockEditorApi = { eventManager: { addEventType: jest.fn(), listen: jest.fn() } };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment