Updating the Source Editor to behave in WebIDE

app/assets/javascripts/editor/constants.js

@@ -33,3 +33,4 @@ export const EXTENSION_MARKDOWN_PREVIEW_PANEL_CLASS = 'md';
 export const EXTENSION_MARKDOWN_PREVIEW_PANEL_PARENT_CLASS = 'source-editor-preview';
 export const EXTENSION_MARKDOWN_PREVIEW_ACTION_ID = 'markdown-preview';
 export const EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH = 0.5; // 50% of the width
+export const EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY = 250; // ms

app/assets/javascripts/editor/extensions/source_editor_markdown_ext.js

@@ -10,6 +10,7 @@ import {
   EXTENSION_MARKDOWN_PREVIEW_ACTION_ID,
   EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH,
   EXTENSION_MARKDOWN_PREVIEW_PANEL_PARENT_CLASS,
+  EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY,
 } from '../constants';
 import { SourceEditorExtension } from './source_editor_extension_base';
 
@@ -50,9 +51,29 @@ export class EditorMarkdownExtension extends SourceEditorExtension {
         el: undefined,
         action: undefined,
         shown: false,
+        modelChangeListener: undefined,
       },
     });
     this.setupPreviewAction.call(instance);
+
+    instance.getModel().onDidChangeLanguage(({ newLanguage, oldLanguage } = {}) => {
+      if (newLanguage === 'markdown' && oldLanguage !== newLanguage) {
+        instance.setupPreviewAction();
+      } else {
+        instance.cleanup();
+      }
+    });
+
+    instance.onDidChangeModel(() => {
+      const model = instance.getModel();
+      if (model) {
+        const { language } = model.getLanguageIdentifier();
+        instance.cleanup();
+        if (language === 'markdown') {
+          instance.setupPreviewAction();
+        }
+      }
+    });
   }
 
   static togglePreviewLayout() {
@@ -78,6 +99,9 @@ export class EditorMarkdownExtension extends SourceEditorExtension {
   }
 
   cleanup() {
+    if (this.preview.modelChangeListener) {
+      this.preview.modelChangeListener.dispose();
+    }
     this.preview.action.dispose();
     if (this.preview.shown) {
       EditorMarkdownExtension.togglePreviewPanel.call(this);
@@ -126,22 +150,14 @@ export class EditorMarkdownExtension extends SourceEditorExtension {
     EditorMarkdownExtension.togglePreviewPanel.call(this);
 
     if (!this.preview?.shown) {
-      this.modelChangeListener = this.onDidChangeModelContent(
-        debounce(this.fetchPreview.bind(this), 250),
+      this.preview.modelChangeListener = this.onDidChangeModelContent(
+        debounce(this.fetchPreview.bind(this), EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY),
       );
     } else {
-      this.modelChangeListener.dispose();
+      this.preview.modelChangeListener.dispose();
     }
 
     this.preview.shown = !this.preview?.shown;
-
-    this.getModel().onDidChangeLanguage(({ newLanguage, oldLanguage } = {}) => {
-      if (newLanguage === 'markdown' && oldLanguage !== newLanguage) {
-        this.setupPreviewAction();
-      } else {
-        this.cleanup();
-      }
-    });
   }
 
   getSelectedText(selection = this.getSelection()) {

app/assets/javascripts/editor/utils.js

@@ -16,12 +16,18 @@ export const setupEditorTheme = () => {
   monacoEditor.setTheme(theme ? themeName : DEFAULT_THEME);
 };
 
-export const getBlobLanguage = (path) => {
-  const ext = `.${path.split('.').pop()}`;
+export const getBlobLanguage = (blobPath) => {
+  const defaultLanguage = 'plaintext';
+
+  if (!blobPath) {
+    return defaultLanguage;
+  }
+
+  const ext = `.${blobPath.split('.').pop()}`;
   const language = monacoLanguages
     .getLanguages()
     .find((lang) => lang.extensions.indexOf(ext) !== -1);
-  return language ? language.id : 'plaintext';
+  return language ? language.id : defaultLanguage;
 };
 
 export const setupCodeSnippet = (el) => {

app/assets/javascripts/ide/components/repo_editor.vue

@@ -38,6 +38,8 @@ import { getPathParent, readFileAsDataURL, registerSchema, isTextFile } from '..
 import FileAlert from './file_alert.vue';
 import FileTemplatesBar from './file_templates/bar.vue';
 
+const MARKDOWN_FILE_TYPE = 'markdown';
+
 export default {
   name: 'RepoEditor',
   components: {
@@ -201,7 +203,7 @@ export default {
     showContentViewer(val) {
       if (!val) return;
 
-      if (this.fileType === 'markdown') {
+      if (this.fileType === MARKDOWN_FILE_TYPE) {
         const { content, images } = extractMarkdownImagesFromEntries(this.file, this.entries);
         this.content = content;
         this.images = images;
@@ -309,6 +311,23 @@ export default {
           }),
         );
 
+        if (this.fileType === MARKDOWN_FILE_TYPE) {
+          import('~/editor/extensions/source_editor_markdown_ext')
+            .then(({ EditorMarkdownExtension: MarkdownExtension } = {}) => {
+              this.editor.use(
+                new MarkdownExtension({
+                  instance: this.editor,
+                  projectPath: this.currentProjectId,
+                }),
+              );
+            })
+            .catch((e) =>
+              createFlash({
+                message: e,
+              }),
+            );
+        }
+
         this.$nextTick(() => {
           this.setupEditor();
         });
@@ -406,7 +425,11 @@ export default {
       const reImage = /^image\/(png|jpg|jpeg|gif)$/;
       const file = event.clipboardData.files[0];
 
-      if (editor.hasTextFocus() && this.fileType === 'markdown' && reImage.test(file?.type)) {
+      if (
+        editor.hasTextFocus() &&
+        this.fileType === MARKDOWN_FILE_TYPE &&
+        reImage.test(file?.type)
+      ) {
         // don't let the event be passed on to Monaco.
         event.preventDefault();
         event.stopImmediatePropagation();

app/assets/stylesheets/framework/source_editor.scss

@@ -34,6 +34,10 @@
     @include gl-py-4;
     @include gl-w-full;
   }
+
+  .gl-source-editor {
+    @include gl-order-n1;
+  }
 }
 
 .monaco-editor.gl-source-editor {

spec/frontend/editor/source_editor_markdown_ext_spec.js

 import MockAdapter from 'axios-mock-adapter';
-import { Range, Position } from 'monaco-editor';
+import { Range, Position, editor as monacoEditor } from 'monaco-editor';
 import waitForPromises from 'helpers/wait_for_promises';
 import {
   EXTENSION_MARKDOWN_PREVIEW_PANEL_CLASS,
   EXTENSION_MARKDOWN_PREVIEW_ACTION_ID,
   EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH,
   EXTENSION_MARKDOWN_PREVIEW_PANEL_PARENT_CLASS,
+  EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY,
 } from '~/editor/constants';
 import { EditorMarkdownExtension } from '~/editor/extensions/source_editor_markdown_ext';
 import SourceEditor from '~/editor/source_editor';
@@ -27,7 +28,8 @@ describe('Markdown Extension for Source Editor', () => {
   const secondLine = 'multiline';
   const thirdLine = 'string with some **markup**';
   const text = `${firstLine}\n${secondLine}\n${thirdLine}`;
-  const filePath = 'foo.md';
+  const plaintextPath = 'foo.txt';
+  const markdownPath = 'foo.md';
   const responseData = '<div>FooBar</div>';
 
   const setSelection = (startLineNumber = 1, startColumn = 1, endLineNumber = 1, endColumn = 1) => {
@@ -52,7 +54,7 @@ describe('Markdown Extension for Source Editor', () => {
     editor = new SourceEditor();
     instance = editor.createInstance({
       el: editorEl,
-      blobPath: filePath,
+      blobPath: markdownPath,
       blobContent: text,
     });
     editor.use(new EditorMarkdownExtension({ instance, projectPath }));
@@ -70,16 +72,107 @@ describe('Markdown Extension for Source Editor', () => {
       el: undefined,
       action: expect.any(Object),
       shown: false,
+      modelChangeListener: undefined,
     });
     expect(instance.projectPath).toBe(projectPath);
   });
 
+  describe('model language changes listener', () => {
+    let cleanupSpy;
+    let actionSpy;
+
+    beforeEach(async () => {
+      cleanupSpy = jest.spyOn(instance, 'cleanup');
+      actionSpy = jest.spyOn(instance, 'setupPreviewAction');
+      await togglePreview();
+    });
+
+    it('cleans up when switching away from markdown', () => {
+      expect(instance.cleanup).not.toHaveBeenCalled();
+      expect(instance.setupPreviewAction).not.toHaveBeenCalled();
+
+      instance.updateModelLanguage(plaintextPath);
+
+      expect(cleanupSpy).toHaveBeenCalled();
+      expect(actionSpy).not.toHaveBeenCalled();
+    });
+
+    it.each`
+      oldLanguage    | newLanguage    | setupCalledTimes
+      ${'plaintext'} | ${'markdown'}  | ${1}
+      ${'markdown'}  | ${'markdown'}  | ${0}
+      ${'markdown'}  | ${'plaintext'} | ${0}
+      ${'markdown'}  | ${undefined}   | ${0}
+      ${undefined}   | ${'markdown'}  | ${1}
+    `(
+      'correctly handles re-enabling of the action when switching from $oldLanguage to $newLanguage',
+      ({ oldLanguage, newLanguage, setupCalledTimes } = {}) => {
+        expect(actionSpy).not.toHaveBeenCalled();
+        instance.updateModelLanguage(oldLanguage);
+        instance.updateModelLanguage(newLanguage);
+        expect(actionSpy).toHaveBeenCalledTimes(setupCalledTimes);
+      },
+    );
+  });
+
+  describe('model change listener', () => {
+    let cleanupSpy;
+    let actionSpy;
+
+    beforeEach(() => {
+      cleanupSpy = jest.spyOn(instance, 'cleanup');
+      actionSpy = jest.spyOn(instance, 'setupPreviewAction');
+      instance.togglePreview();
+    });
+
+    afterEach(() => {
+      jest.clearAllMocks();
+    });
+
+    it('does not do anything if there is no model', () => {
+      instance.setModel(null);
+
+      expect(cleanupSpy).not.toHaveBeenCalled();
+      expect(actionSpy).not.toHaveBeenCalled();
+    });
+
+    it('cleans up the preview when the model changes', () => {
+      instance.setModel(monacoEditor.createModel('foo'));
+      expect(cleanupSpy).toHaveBeenCalled();
+    });
+
+    it.each`
+      language       | setupCalledTimes
+      ${'markdown'}  | ${1}
+      ${'plaintext'} | ${0}
+      ${undefined}   | ${0}
+    `(
+      'correctly handles actions when the new model is $language',
+      ({ language, setupCalledTimes } = {}) => {
+        instance.setModel(monacoEditor.createModel('foo', language));
+
+        expect(actionSpy).toHaveBeenCalledTimes(setupCalledTimes);
+      },
+    );
+  });
+
   describe('cleanup', () => {
     beforeEach(async () => {
       mockAxios.onPost().reply(200, { body: responseData });
       await togglePreview();
     });
 
+    it('disposes the modelChange listener and does not fetch preview on content changes', () => {
+      expect(instance.preview.modelChangeListener).toBeDefined();
+      jest.spyOn(instance, 'fetchPreview');
+
+      instance.cleanup();
+      instance.setValue('Foo Bar');
+      jest.advanceTimersByTime(EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY);
+
+      expect(instance.fetchPreview).not.toHaveBeenCalled();
+    });
+
     it('removes the contextual menu action', () => {
       expect(instance.getAction(EXTENSION_MARKDOWN_PREVIEW_ACTION_ID)).toBeDefined();
 
@@ -219,47 +312,6 @@ describe('Markdown Extension for Source Editor', () => {
       expect(instance.preview.shown).toBe(false);
     });
 
-    describe('model language changes', () => {
-      const plaintextPath = 'foo.txt';
-      const markdownPath = 'foo.md';
-      let cleanupSpy;
-      let actionSpy;
-
-      beforeEach(() => {
-        cleanupSpy = jest.spyOn(instance, 'cleanup');
-        actionSpy = jest.spyOn(instance, 'setupPreviewAction');
-        instance.togglePreview();
-      });
-
-      it('cleans up when switching away from markdown', async () => {
-        expect(instance.cleanup).not.toHaveBeenCalled();
-        expect(instance.setupPreviewAction).not.toHaveBeenCalled();
-
-        instance.updateModelLanguage(plaintextPath);
-
-        expect(cleanupSpy).toHaveBeenCalled();
-        expect(actionSpy).not.toHaveBeenCalled();
-      });
-
-      it('re-enables the action when switching back to markdown', () => {
-        instance.updateModelLanguage(plaintextPath);
-
-        jest.clearAllMocks();
-
-        instance.updateModelLanguage(markdownPath);
-
-        expect(cleanupSpy).not.toHaveBeenCalled();
-        expect(actionSpy).toHaveBeenCalled();
-      });
-
-      it('does not re-enable the action if we do not change the language', () => {
-        instance.updateModelLanguage(markdownPath);
-
-        expect(cleanupSpy).not.toHaveBeenCalled();
-        expect(actionSpy).not.toHaveBeenCalled();
-      });
-    });
-
     describe('panel DOM element set up', () => {
       it('sets up an element to contain the preview and stores it on instance', () => {
         expect(instance.preview.el).toBeUndefined();
@@ -335,9 +387,9 @@ describe('Markdown Extension for Source Editor', () => {
         });
 
         it('stores disposable listener for model changes', async () => {
-          expect(instance.modelChangeListener).toBeUndefined();
+          expect(instance.preview.modelChangeListener).toBeUndefined();
           await togglePreview();
-          expect(instance.modelChangeListener).toBeDefined();
+          expect(instance.preview.modelChangeListener).toBeDefined();
         });
       });
 
@@ -354,7 +406,7 @@ describe('Markdown Extension for Source Editor', () => {
 
         it('disposes the model change event listener', () => {
           const disposeSpy = jest.fn();
-          instance.modelChangeListener = {
+          instance.preview.modelChangeListener = {
             dispose: disposeSpy,
           };
           instance.togglePreview();

spec/frontend/editor/utils_spec.js

@@ -53,6 +53,7 @@ describe('Source Editor utils', () => {
       ${'foo.js'}    | ${'javascript'}
       ${'foo.js.rb'} | ${'ruby'}
       ${'foo.bar'}   | ${'plaintext'}
+      ${undefined}   | ${'plaintext'}
     `(
       'sets the $expectedThemeName theme when $themeName is set in the user preference',
       ({ path, expectedLanguage }) => {

spec/frontend/ide/components/repo_editor_spec.js

@@ -166,6 +166,11 @@ describe('RepoEditor', () => {
       expect(tabs).toHaveLength(1);
       expect(tabs.at(0).text()).toBe('Edit');
     });
+
+    it('does not get markdown extension by default', async () => {
+      await createComponent();
+      expect(vm.editor.projectPath).toBeUndefined();
+    });
   });
 
   describe('when file is markdown', () => {
@@ -213,6 +218,11 @@ describe('RepoEditor', () => {
       });
       expect(findTabs()).toHaveLength(0);
     });
+
+    it('uses the markdown extension and sets it up correctly', async () => {
+      await createComponent({ activeFile });
+      expect(vm.editor.projectPath).toBe(vm.currentProjectId);
+    });
   });
 
   describe('when file is binary and not raw', () => {