Skip to content

Simplify API to pass Markdown to the Content Editor

Enrique Alcántara requested to merge refactor-content-editor-markdown-io into master

What does this MR do and why?

It simplifies the Content Editor’s API to pass Markdown and return the Markdown generated by the editor. The Content Editor is a WYSIWYG editor for GitLab Flavored Markdown. Before this change, the Vue component that instantiated the Content Editor had to invoke the setSerializedContent and getSerializedContent methods to provide Markdown and retrieve the Markdown generated by the Content Editor:

<script>
export default {
  methods: {
    async loadInitialContent(contentEditor) {
      this.contentEditor = contentEditor;

      try {
        await this.contentEditor.setSerializedContent(this.content);
        this.trackContentEditorLoaded();
      } catch (e) {
        this.contentEditorRenderFailed = true;
      }
    },
    async handleFormSubmit(e) {
      e.preventDefault();

      if (this.useContentEditor) {
        this.content = this.contentEditor.getSerializedContent();
      }
    },
  }

}
</script>
<template>
<content-editor
  :render-markdown="renderMarkdown"
  :uploads-path="pageInfo.uploadsPath"
  @initialized="loadInitialContent"
/>
<input type="hidden" v-model="content" />
</template>

The new API relies on Vue’s properties and events which simplifies using the component in Vue applications:

<script>
export default {
  data() {
    return {
      markdown: 'initial markdown',
    };
  },
  methods: {
    onContentEditorChange({ changed, markdown, empty }) {
      this.markdown = markdown;
    }
  }

}
</script>
<template>
<content-editor
  :render-markdown="renderMarkdown"
  :uploads-path="pageInfo.uploadsPath"
  :markdown="markdown"
  @change="onContentEditorChange"
/>
<input type="hidden" v-model="content" />
</template>

Why did we implement a complex API that doesn’t conform with Vue's best practices?

We were driven by two performance concerns:

The Content Editor needs to transform a Markdown document into HTML to display the document in a contenteditable box. To perform this transformation, the editor makes a REST API request to the Markdown API. We were concerned about making a REST API request every time that the user changed the Markdown therefore we decided to expose an object that allows the parent component to control when the initial Markdown document is loaded (setSerializedContent).

The Content Editor also needs to transform the contenteditable’s DOM content back to Markdown (a process we call serialization). We were concerned about serializing the DOM every time that the user types in the Content Editor.

Why are these concerns not valid anymore?

There are several reasons:

  1. The Content Editor is not rendered when the user modifies the Markdown document using the Markdown’s source editor. 2022-09-02_10.38.51 Therefore, the REST API call won’t happen until the user switches to Rich Text. That addresses our first performance concern. We also make sure in ContentEditor.vue
  2. The serialization process is debounced and not expensive for Markdown documents that are not conspicuously large.
  3. As part of the preserve unchanged markdown effort, we are delivering two significant performance improvements:
    1. We will deserialize (Markdown -> HTML) the Markdown in the web browser which is significantly faster.
    2. We will reuse the original Markdown source in the serialization process for parts of the document that haven’t changed. That will also be an important performance improvement.

This MR is too large

This MR contains many changes, and I recommend reviewing them together to have a holistic understanding. I will provide contextual notes and a guide to easily navigate the changes in this MR.

How to read the changes in this MR

Some of the diffs in this MR are confusing even when the changes are simple. The diff algorithm treats tests that were deleted and others added as "modified tests". I recommend using the side-by-side view or using the "view file" options if you find diff view hard to follow.

Screen_Shot_2022-09-02_at_10.56.43_AM

  • Start at wiki_form.vue. It is the component that instantiates the Content Editor. You will understand these changes better when you see how the Content Editor is used.
  • Go to content_editor.js. See how we are removing to the eventBus.$emit calls in the setSerializedContent method. This is very important before jumping to the next file.
  • Then go to content_editor.vue. You will see a setSerializedContent method that is handling all the logic to emit events when the Markdown is being processed and to display the loading indicator. This method replaced all the eventBus logic from the previous file.
  • loading_indicator.vue and editor_state_observer are drastically simplified because they don’t have to listen to event_bus events anymore.
  • We still use the eventBus to display error alerts. We improved the content_editor_alert.vue event to allow passing actions to those alerts.

We also revisited the unit tests and simplified them based on the changes above. We increased the test coverage where we found a test coverage gap.

Screenshots or screen recordings

This MR doesn’t introduce user-facing changes.

How to set up and validate locally

This MR doesn’t introduce user-facing changes.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Enrique Alcántara

Merge request reports

Loading