From 62a35bccadb58d45b7b7033ab6f564e539fd7b1d Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 20 Apr 2024 21:23:49 -0700 Subject: [PATCH 1/6] =?UTF-8?q?Optimize=20re-rendering=20of=20content=20in?= =?UTF-8?q?=20a=20preview=20pane=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …especially syntax highlighting. This change brings in `morphdom` to allow us to be more efficient about how we apply the changes needed when the Markdown source re-renders. Instead of replacing all the content with every single change, we apply only the selective edits needed to adapt our existing markup to the target markup. Once this process is done, we introduce one `TextEditor` instance for each `pre` tag in the markup, then persist those editor elements in the rendered output so that we don't have to spend so much time creating and destroying editors. This is a _huge_ performance win, especially in documents with lots of code blocks. The editor instances are cached using the `pre` elements as keys, which is now possible because the `pre` elements themselves are preserved across re-renders. Editors are created when new `pre` elements appear, and are destroyed when they are no longer needed, change their contents whenever the contents of the `pre` changes, and change language modes whenever the code fence language identifier changes. This approach is now used no matter which Markdown renderer is employed; we manage syntax highlighting ourselves in all cases rather than letting `atom.ui.markdown` do it. --- packages/markdown-preview/lib/main.js | 47 ++- .../lib/markdown-preview-view.js | 33 +- packages/markdown-preview/lib/renderer.js | 323 +++++++++++------- packages/markdown-preview/package-lock.json | 14 +- packages/markdown-preview/package.json | 4 +- .../spec/markdown-preview-view-spec.js | 15 +- .../styles/markdown-preview.less | 7 + 7 files changed, 303 insertions(+), 140 deletions(-) diff --git a/packages/markdown-preview/lib/main.js b/packages/markdown-preview/lib/main.js index 0b87479b7..14bb95a36 100644 --- a/packages/markdown-preview/lib/main.js +++ b/packages/markdown-preview/lib/main.js @@ -12,10 +12,18 @@ const isMarkdownPreviewView = function (object) { } module.exports = { - activate () { + activate() { this.disposables = new CompositeDisposable() this.commandSubscriptions = new CompositeDisposable() + this.style = new CSSStyleSheet() + + // When we upgrade Electron, we can push onto `adoptedStyleSheets` + // directly. For now, we have to do this silly thing. + let styleSheets = Array.from(document.adoptedStyleSheets ?? []) + styleSheets.push(this.style) + document.adoptedStyleSheets = styleSheets + this.disposables.add( atom.config.observe('markdown-preview.grammars', grammars => { this.commandSubscriptions.dispose() @@ -53,6 +61,22 @@ module.exports = { }) ) + this.disposables.add( + atom.config.observe('editor.fontFamily', (fontFamily) => { + // Keep the user's `fontFamily` setting in sync with preview styles. + // `pre` blocks will use this font automatically, but `code` elements + // need a specific style rule. + // + // Since this applies to all content, we should declare this only once, + // instead of once per preview view. + this.style.replaceSync(` + .markdown-preview code { + font-family: ${fontFamily} !important; + } + `) + }) + ) + const previewFile = this.previewFile.bind(this) for (const extension of [ 'markdown', @@ -94,12 +118,12 @@ module.exports = { ) }, - deactivate () { + deactivate() { this.disposables.dispose() this.commandSubscriptions.dispose() }, - createMarkdownPreviewView (state) { + createMarkdownPreviewView(state) { if (state.editorId || fs.isFileSync(state.filePath)) { if (MarkdownPreviewView == null) { MarkdownPreviewView = require('./markdown-preview-view') @@ -108,7 +132,7 @@ module.exports = { } }, - toggle () { + toggle() { if (isMarkdownPreviewView(atom.workspace.getActivePaneItem())) { atom.workspace.destroyActivePaneItem() return @@ -129,11 +153,11 @@ module.exports = { } }, - uriForEditor (editor) { + uriForEditor(editor) { return `markdown-preview://editor/${editor.id}` }, - removePreviewForEditor (editor) { + removePreviewForEditor(editor) { const uri = this.uriForEditor(editor) const previewPane = atom.workspace.paneForURI(uri) if (previewPane != null) { @@ -144,7 +168,7 @@ module.exports = { } }, - addPreviewForEditor (editor) { + addPreviewForEditor(editor) { const uri = this.uriForEditor(editor) const previousActivePane = atom.workspace.getActivePane() const options = { searchAllPanes: true } @@ -161,7 +185,7 @@ module.exports = { }) }, - previewFile ({ target }) { + previewFile({ target }) { const filePath = target.dataset.path if (!filePath) { return @@ -178,7 +202,7 @@ module.exports = { }) }, - async copyHTML () { + async copyHTML() { const editor = atom.workspace.getActiveTextEditor() if (editor == null) { return @@ -191,13 +215,14 @@ module.exports = { const html = await renderer.toHTML( text, editor.getPath(), - editor.getGrammar() + editor.getGrammar(), + editor.id ) atom.clipboard.write(html) }, - saveAsHTML () { + saveAsHTML() { const activePaneItem = atom.workspace.getActivePaneItem() if (isMarkdownPreviewView(activePaneItem)) { atom.workspace.getActivePane().saveItemAs(activePaneItem) diff --git a/packages/markdown-preview/lib/markdown-preview-view.js b/packages/markdown-preview/lib/markdown-preview-view.js index 1f6410d4a..34e964f16 100644 --- a/packages/markdown-preview/lib/markdown-preview-view.js +++ b/packages/markdown-preview/lib/markdown-preview-view.js @@ -1,4 +1,5 @@ const path = require('path') +const morphdom = require('morphdom') const { Emitter, Disposable, CompositeDisposable, File } = require('atom') const _ = require('underscore-plus') @@ -17,6 +18,7 @@ module.exports = class MarkdownPreviewView { this.element = document.createElement('div') this.element.classList.add('markdown-preview') this.element.tabIndex = -1 + this.emitter = new Emitter() this.loaded = false this.disposables = new CompositeDisposable() @@ -32,6 +34,7 @@ module.exports = class MarkdownPreviewView { }) ) } + this.editorCache = new renderer.EditorCache(editorId) } serialize() { @@ -52,6 +55,7 @@ module.exports = class MarkdownPreviewView { destroy() { this.disposables.dispose() this.element.remove() + this.editorCache.destroy() } registerScrollCommands() { @@ -83,7 +87,7 @@ module.exports = class MarkdownPreviewView { return this.emitter.on('did-change-title', callback) } - onDidChangeModified(callback) { + onDidChangeModified(_callback) { // No op to suppress deprecation warning return new Disposable() } @@ -309,18 +313,33 @@ module.exports = class MarkdownPreviewView { async renderMarkdownText(text) { const { scrollTop } = this.element - try { - const domFragment = await renderer.toDOMFragment( + const [domFragment, done] = await renderer.toDOMFragment( text, this.getPath(), - this.getGrammar() + this.getGrammar(), + this.editorId ) this.loading = false this.loaded = true - this.element.textContent = '' - this.element.appendChild(domFragment) + + // Clone the existing container + let newElement = this.element.cloneNode(false) + newElement.appendChild(domFragment) + + morphdom(this.element, newElement, { + onBeforeNodeDiscarded(node) { + // Don't discard `atom-text-editor` elements despite the fact that + // they don't exist in the new content. + if (node.nodeName === 'ATOM-TEXT-EDITOR') { + return false + } + } + }) + + await done(this.element) + this.emitter.emit('did-change-markdown') this.element.scrollTop = scrollTop } catch (error) { @@ -400,7 +419,7 @@ module.exports = class MarkdownPreviewView { .join('\n') .replace(/atom-text-editor/g, 'pre.editor-colors') .replace(/:host/g, '.host') // Remove shadow-dom :host selector causing problem on FF - .replace(cssUrlRegExp, function (match, assetsName, offset, string) { + .replace(cssUrlRegExp, function (_match, assetsName, _offset, _string) { // base64 encode assets const assetPath = path.join(__dirname, '../assets', assetsName) const originalData = fs.readFileSync(assetPath, 'binary') diff --git a/packages/markdown-preview/lib/renderer.js b/packages/markdown-preview/lib/renderer.js index 636ad68c5..14d4db59d 100644 --- a/packages/markdown-preview/lib/renderer.js +++ b/packages/markdown-preview/lib/renderer.js @@ -17,91 +17,147 @@ const emojiFolder = path.join( 'pngs' ) -exports.toDOMFragment = async function (text, filePath, grammar, callback) { +// Creating `TextEditor` instances is costly, so we'll try to re-use instances +// when a preview changes. +class EditorCache { + static BY_ID = new Map() - text ??= ""; - - if (atom.config.get("markdown-preview.useOriginalParser")) { - const domFragment = render(text, filePath); - - await highlightCodeBlocks(domFragment, grammar, makeAtomEditorNonInteractive); - - return domFragment; - - } else { - // We use the new parser! - const domFragment = atom.ui.markdown.render(text, - { - renderMode: "fragment", - filePath: filePath, - breaks: atom.config.get('markdown-preview.breakOnSingleNewline'), - useDefaultEmoji: true, - sanitizeAllowUnknownProtocols: atom.config.get('markdown-preview.allowUnsafeProtocols') - } - ); - const domHTMLFragment = atom.ui.markdown.convertToDOM(domFragment); - await atom.ui.markdown.applySyntaxHighlighting(domHTMLFragment, - { - renderMode: "fragment", - syntaxScopeNameFunc: scopeForFenceName, - grammar: grammar - } - ); - - return domHTMLFragment; + static findOrCreateById(id) { + let cache = EditorCache.BY_ID.get(id) + if (!cache) { + cache = new EditorCache(id) + EditorCache.BY_ID.set(id, cache) + } + return cache } + + constructor(id) { + this.id = id + this.editorsByPre = new Map() + this.possiblyUnusedEditors = new Set() + } + + destroy() { + let editors = Array.from(this.editorsByPre.values()) + for (let editor of editors) { + editor.destroy() + } + this.editorsByPre.clear() + this.possiblyUnusedEditors.clear() + EditorCache.BY_ID.delete(this.id) + } + + // Called when we start a render. Every `TextEditor` is assumed to be stale, + // but any editor that is successfully looked up from the cache during this + // render is saved from culling. + beginRender() { + this.possiblyUnusedEditors.clear() + for (let editor of this.editorsByPre.values()) { + this.possiblyUnusedEditors.add(editor) + } + } + + // Cache an editor by the PRE element that it's standing in for. + addEditor(pre, editor) { + this.editorsByPre.set(pre, editor) + } + + getEditor(pre) { + let editor = this.editorsByPre.get(pre) + if (editor) { + // Cache hit! This editor will be reused, so we should prevent it from + // getting culled. + this.possiblyUnusedEditors.delete(editor) + } + return editor + } + + endRender() { + // Any editor that didn't get claimed during the render is orphaned and + // should be disposed of. + let toBeDeleted = new Set() + for (let [pre, editor] of this.editorsByPre.entries()) { + if (!this.possiblyUnusedEditors.has(editor)) continue + toBeDeleted.add(pre) + } + + this.possiblyUnusedEditors.clear() + + for (let pre of toBeDeleted) { + let editor = this.editorsByPre.get(pre) + let element = editor.getElement() + if (element.parentNode) { + element.remove() + } + this.editorsByPre.delete(pre) + editor.destroy() + } + } +} + +exports.EditorCache = EditorCache + +function chooseRender(text, filePath) { + if (atom.config.get("markdown-preview.useOriginalParser")) { + // Legacy rendering with `marked`. + return render(text, filePath) + } else { + // Built-in rendering with `markdown-it`. + let html = atom.ui.markdown.render(text, { + renderMode: "fragment", + filePath: filePath, + breaks: atom.config.get('markdown-preview.breakOnSingleNewline'), + useDefaultEmoji: true, + sanitizeAllowUnknownProtocols: atom.config.get('markdown-preview.allowUnsafeProtocols') + }) + return atom.ui.markdown.convertToDOM(html) + } +} + +exports.toDOMFragment = async function (text, filePath, grammar, editorId) { + text ??= "" + let defaultLanguage = getDefaultLanguageForGrammar(grammar) + + // We cache editor instances in this code path because it's the one used by + // the preview pane, so we expect it to be updated quite frequently. + let cache = EditorCache.findOrCreateById(editorId) + cache.beginRender() + + const domFragment = chooseRender(text, filePath) + annotatePreElements(domFragment, defaultLanguage) + + return [ + domFragment, + async (element) => { + await highlightCodeBlocks(element, grammar, cache, makeAtomEditorNonInteractive) + cache.endRender() + } + ] } exports.toHTML = async function (text, filePath, grammar) { - text ??= ""; - if (atom.config.get("markdown-preview.useOriginalParser")) { - const domFragment = render(text, filePath) - const div = document.createElement('div') + // We don't cache editor instances in this code path because it's the one + // used by the “Copy HTML” command, so this is likely to be a one-off for + // which caches won't help. - div.appendChild(domFragment) - document.body.appendChild(div) + const domFragment = chooseRender(text, filePath) + const div = document.createElement('div') + annotatePreElements(domFragment, getDefaultLanguageForGrammar(grammar)) + div.appendChild(domFragment) + document.body.appendChild(div) - await highlightCodeBlocks(div, grammar, convertAtomEditorToStandardElement) + await highlightCodeBlocks(div, grammar, null, convertAtomEditorToStandardElement) - const result = div.innerHTML - div.remove() + const result = div.innerHTML; + div.remove(); - return result - } else { - // We use the new parser! - const domFragment = atom.ui.markdown.render(text, - { - renderMode: "full", - filePath: filePath, - breaks: atom.config.get('markdown-preview.breakOnSingleNewline'), - useDefaultEmoji: true, - sanitizeAllowUnknownProtocols: atom.config.get('markdown-preview.allowUnsafeProtocols') - } - ); - const domHTMLFragment = atom.ui.markdown.convertToDOM(domFragment); - - const div = document.createElement("div"); - div.appendChild(domHTMLFragment); - document.body.appendChild(div); - - await atom.ui.markdown.applySyntaxHighlighting(div, - { - renderMode: "full", - syntaxScopeNameFunc: scopeForFenceName, - grammar: grammar - } - ); - - const result = div.innerHTML; - div.remove(); - - return result; - } + return result; } -var render = function (text, filePath) { +// Render with the package's own `marked` library. +function render(text, filePath) { if (marked == null || yamlFrontMatter == null || cheerio == null) { marked = require('marked') yamlFrontMatter = require('yaml-front-matter') @@ -124,12 +180,13 @@ var render = function (text, filePath) { let html = marked.parse(renderYamlTable(vars) + __content) - // emoji-images is too aggressive, so replace images in monospace tags with the actual emoji text. + // emoji-images is too aggressive, so replace images in monospace tags with + // the actual emoji text. const $ = cheerio.load(emoji(html, emojiFolder, 20)) - $('pre img').each((index, element) => + $('pre img').each((_index, element) => $(element).replaceWith($(element).attr('title')) ) - $('code img').each((index, element) => + $('code img').each((_index, element) => $(element).replaceWith($(element).attr('title')) ) @@ -159,7 +216,7 @@ function renderYamlTable(variables) { const markdownRows = [ entries.map(entry => entry[0]), - entries.map(entry => '--'), + entries.map(_ => '--'), entries.map((entry) => { if (typeof entry[1] === "object" && !Array.isArray(entry[1])) { // Remove all newlines, or they ruin formatting of parent table @@ -175,7 +232,7 @@ function renderYamlTable(variables) { ) } -var resolveImagePaths = function (element, filePath) { +function resolveImagePaths(element, filePath) { const [rootDirectory] = atom.project.relativizePath(filePath) const result = [] @@ -219,55 +276,90 @@ var resolveImagePaths = function (element, filePath) { return result } -var highlightCodeBlocks = function (domFragment, grammar, editorCallback) { - let defaultLanguage, fontFamily - if ( - (grammar != null ? grammar.scopeName : undefined) === 'source.litcoffee' - ) { - defaultLanguage = 'coffee' - } else { - defaultLanguage = 'text' - } +function getDefaultLanguageForGrammar(grammar) { + return grammar?.scopeName === 'source.litcoffee' ? 'coffee' : 'text' +} - if ((fontFamily = atom.config.get('editor.fontFamily'))) { - for (const codeElement of domFragment.querySelectorAll('code')) { - codeElement.style.fontFamily = fontFamily - } +function annotatePreElements(fragment, defaultLanguage) { + for (let preElement of fragment.querySelectorAll('pre')) { + const codeBlock = preElement.firstElementChild ?? preElement + const className = codeBlock.getAttribute('class') + const fenceName = className?.replace(/^language-/, '') ?? defaultLanguage + preElement.classList.add('editor-colors', `lang-${fenceName}`) } +} + +function reassignEditorToLanguage(editor, languageScope) { + // When we successfully reassign the language on an editor, its + // `data-grammar` attribute updates on its own. + let result = atom.grammars.assignLanguageMode(editor, languageScope) + if (result) return true + + // When we fail to assign the language on an editor — maybe its package is + // deactivated — it won't reset itself to the default grammar, so we have to + // do it ourselves. + result = atom.grammars.assignLanguageMode(editor, `text.plain.null-grammar`) + if (!result) return false +} + +// After render, create an `atom-text-editor` for each `pre` element so that we +// enjoy syntax highlighting. +function highlightCodeBlocks(element, grammar, cache, editorCallback) { + let defaultLanguage = getDefaultLanguageForGrammar(grammar) const promises = [] - for (const preElement of domFragment.querySelectorAll('pre')) { - const codeBlock = - preElement.firstElementChild != null - ? preElement.firstElementChild - : preElement + + for (const preElement of element.querySelectorAll('pre')) { + const codeBlock = preElement.firstElementChild ?? preElement const className = codeBlock.getAttribute('class') - const fenceName = - className != null ? className.replace(/^language-/, '') : defaultLanguage + const fenceName = className?.replace(/^language-/, '') ?? defaultLanguage + let editorText = codeBlock.textContent.replace(/\r?\n$/, '') - const editor = new TextEditor({ - readonly: true, - keyboardInputEnabled: false - }) - const editorElement = editor.getElement() + // If this PRE element was present in the last render, then we should + // already have a cached text editor available for use. + let editor = cache?.getEditor(preElement) ?? null + let editorElement + if (!editor) { + editor = new TextEditor({ keyboardInputEnabled: false }) + editorElement = editor.getElement() + editorElement.setUpdatedSynchronously(true) + editor.setReadOnly(true) + cache?.addEditor(preElement, editor) + } else { + editorElement = editor.getElement() + } - preElement.classList.add('editor-colors', `lang-${fenceName}`) - editorElement.setUpdatedSynchronously(true) - preElement.innerHTML = '' - preElement.parentNode.insertBefore(editorElement, preElement) - editor.setText(codeBlock.textContent.replace(/\r?\n$/, '')) - atom.grammars.assignLanguageMode(editor, scopeForFenceName(fenceName)) - editor.setVisible(true) + // If the PRE changed its content, we need to change the content of its + // `TextEditor`. + if (editor.getText() !== editorText) { + editor.setReadOnly(false) + editor.setText(editorText) + editor.setReadOnly(true) + } + + // If the PRE changed its language, we need to change the language of its + // `TextEditor`. + let scopeDescriptor = editor.getRootScopeDescriptor()[0] + let languageScope = scopeForFenceName(fenceName) + if (languageScope !== scopeDescriptor && `.${languageScope}` !== scopeDescriptor) { + reassignEditorToLanguage(editor, languageScope) + } + + // If the editor is brand new, we'll have to insert it; otherwise it should + // already be in the right place. + if (!editorElement.parentNode) { + preElement.parentNode.insertBefore(editorElement, preElement) + editor.setVisible(true) + } promises.push(editorCallback(editorElement, preElement)) } return Promise.all(promises) } -var makeAtomEditorNonInteractive = function (editorElement, preElement) { - preElement.remove() - editorElement.setAttributeNode(document.createAttribute('gutter-hidden')) // Hide gutter - editorElement.removeAttribute('tabindex') // Make read-only +function makeAtomEditorNonInteractive(editorElement) { + editorElement.setAttributeNode(document.createAttribute('gutter-hidden')) + editorElement.removeAttribute('tabindex') // Remove line decorations from code blocks. for (const cursorLineDecoration of editorElement.getModel() @@ -276,9 +368,12 @@ var makeAtomEditorNonInteractive = function (editorElement, preElement) { } } -var convertAtomEditorToStandardElement = (editorElement, preElement) => { +function convertAtomEditorToStandardElement(editorElement, preElement) { return new Promise(function (resolve) { const editor = editorElement.getModel() + // In this code path, we're transplanting the highlighted editor HTML into + // the existing `pre` element, so we should empty its contents first. + preElement.innerHTML = '' const done = () => editor.component.getNextUpdatePromise().then(function () { for (const line of editorElement.querySelectorAll( diff --git a/packages/markdown-preview/package-lock.json b/packages/markdown-preview/package-lock.json index 887d4d766..689c1e03d 100644 --- a/packages/markdown-preview/package-lock.json +++ b/packages/markdown-preview/package-lock.json @@ -16,6 +16,7 @@ "fs-plus": "^3.0.0", "github-markdown-css": "^5.5.1", "marked": "5.0.3", + "morphdom": "^2.7.2", "underscore-plus": "^1.0.0", "yaml-front-matter": "^4.1.1" }, @@ -23,7 +24,8 @@ "temp": "^0.8.1" }, "engines": { - "atom": "*" + "atom": "*", + "node": ">=12" } }, "node_modules/@types/node": { @@ -278,6 +280,11 @@ "mkdirp": "bin/cmd.js" } }, + "node_modules/morphdom": { + "version": "2.7.2", + "resolved": "https://registry.npmjs.org/morphdom/-/morphdom-2.7.2.tgz", + "integrity": "sha512-Dqb/lHFyTi7SZpY0a5R4I/0Edo+iPMbaUexsHHsLAByyixCDiLHPHyVoKVmrpL0THcT7V9Cgev9y21TQYq6wQg==" + }, "node_modules/nth-check": { "version": "1.0.2", "license": "BSD-2-Clause", @@ -567,6 +574,11 @@ "minimist": "0.0.8" } }, + "morphdom": { + "version": "2.7.2", + "resolved": "https://registry.npmjs.org/morphdom/-/morphdom-2.7.2.tgz", + "integrity": "sha512-Dqb/lHFyTi7SZpY0a5R4I/0Edo+iPMbaUexsHHsLAByyixCDiLHPHyVoKVmrpL0THcT7V9Cgev9y21TQYq6wQg==" + }, "nth-check": { "version": "1.0.2", "requires": { diff --git a/packages/markdown-preview/package.json b/packages/markdown-preview/package.json index 0bb4a6e75..5ec7e4428 100644 --- a/packages/markdown-preview/package.json +++ b/packages/markdown-preview/package.json @@ -6,7 +6,8 @@ "repository": "https://github.com/pulsar-edit/pulsar", "license": "MIT", "engines": { - "atom": "*" + "atom": "*", + "node": ">=12" }, "scripts": { "generate-github-markdown-css": "node scripts/generate-github-markdown-css.js" @@ -19,6 +20,7 @@ "fs-plus": "^3.0.0", "github-markdown-css": "^5.5.1", "marked": "5.0.3", + "morphdom": "^2.7.2", "underscore-plus": "^1.0.0", "yaml-front-matter": "^4.1.1" }, diff --git a/packages/markdown-preview/spec/markdown-preview-view-spec.js b/packages/markdown-preview/spec/markdown-preview-view-spec.js index c34d5a727..02a2c7623 100644 --- a/packages/markdown-preview/spec/markdown-preview-view-spec.js +++ b/packages/markdown-preview/spec/markdown-preview-view-spec.js @@ -198,12 +198,15 @@ function f(x) { () => renderSpy.callCount === 1 ) - runs(function () { - const rubyEditor = preview.element.querySelector( - "atom-text-editor[data-grammar='source ruby']" - ) - expect(rubyEditor).toBeNull() - }) + waitsFor( + 'atom-text-editor to reassign all language modes after re-render', + () => { + let rubyEditor = preview.element.querySelector( + "atom-text-editor[data-grammar='source ruby']" + ) + return rubyEditor == null + } + ) waitsForPromise(() => atom.packages.activatePackage('language-ruby')) diff --git a/packages/markdown-preview/styles/markdown-preview.less b/packages/markdown-preview/styles/markdown-preview.less index bff8cec53..0853c74ef 100644 --- a/packages/markdown-preview/styles/markdown-preview.less +++ b/packages/markdown-preview/styles/markdown-preview.less @@ -2,6 +2,13 @@ // Global Markdown Preview styles .markdown-preview { + + // Hide a `pre` that comes directly after an `atom-text-editor` because the + // `atom-text-editor` is the syntax-highlighted representation. + atom-text-editor + pre { + display: none; + } + atom-text-editor { // only show scrollbars on hover .scrollbars-visible-always & { From 18e0da8baae8db3d197fb2162ebeb39aab32eec9 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 21 Apr 2024 11:36:06 -0700 Subject: [PATCH 2/6] Fix `atom.ui.markdown` issue with rendering of HTML in code blocks --- spec/ui-spec.js | 21 +++++++++++++++++++++ src/ui.js | 4 ++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/spec/ui-spec.js b/spec/ui-spec.js index 8ad5e0e66..26dda6081 100644 --- a/spec/ui-spec.js +++ b/spec/ui-spec.js @@ -1,3 +1,4 @@ +const dedent = require('dedent'); describe("Renders Markdown", () => { describe("properly when given no opts", () => { @@ -7,6 +8,26 @@ describe("Renders Markdown", () => { }); }); + it(`escapes HTML in code blocks properly`, () => { + let input = dedent` + Lorem ipsum dolor. + + \`\`\`html +

sit amet

+ \`\`\` + ` + + let expected = dedent` +

Lorem ipsum dolor.

+
<p>sit amet</p>
+    
+ ` + + expect( + atom.ui.markdown.render(input).trim() + ).toBe(expected); + }) + describe("transforms links correctly", () => { it("makes no changes to a fqdn link", () => { expect(atom.ui.markdown.render("[Hello World](https://github.com)")) diff --git a/src/ui.js b/src/ui.js index 2a4662c44..66d208209 100644 --- a/src/ui.js +++ b/src/ui.js @@ -249,8 +249,8 @@ function renderMarkdown(content, givenOpts = {}) { // Here we can add some simple additions that make code highlighting possible later on, // but doesn't actually preform any code highlighting. - md.options.highlight = function(str, lang) { - return `
${str}
`; + md.options.highlight = function (str, lang) { + return `
${md.utils.escapeHtml(str)}
`; }; // Process disables From b46a7e8e86d1688ef19ed559ec9b65015fcb8a03 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 21 Apr 2024 19:48:53 -0700 Subject: [PATCH 3/6] =?UTF-8?q?Don't=20make=20`TextEditor`=20instances=20r?= =?UTF-8?q?ender=20synchronously=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …because we don't need it for our use case. Helps performance. --- packages/markdown-preview/lib/renderer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/markdown-preview/lib/renderer.js b/packages/markdown-preview/lib/renderer.js index 14d4db59d..342c7751f 100644 --- a/packages/markdown-preview/lib/renderer.js +++ b/packages/markdown-preview/lib/renderer.js @@ -322,7 +322,6 @@ function highlightCodeBlocks(element, grammar, cache, editorCallback) { if (!editor) { editor = new TextEditor({ keyboardInputEnabled: false }) editorElement = editor.getElement() - editorElement.setUpdatedSynchronously(true) editor.setReadOnly(true) cache?.addEditor(preElement, editor) } else { From 054100b43ce6e9a7642dd2300990a51e55673f89 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 21 Apr 2024 22:05:44 -0700 Subject: [PATCH 4/6] =?UTF-8?q?Rework=20loading=20indicator=20for=20`markd?= =?UTF-8?q?own-preview`=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …and make the pane appear earlier on first open. --- .../lib/markdown-preview-view.js | 24 ++++++++--- .../spec/markdown-preview-spec.js | 7 ++++ .../styles/markdown-preview.less | 41 +++++++++++++++---- 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/packages/markdown-preview/lib/markdown-preview-view.js b/packages/markdown-preview/lib/markdown-preview-view.js index 34e964f16..78a222efc 100644 --- a/packages/markdown-preview/lib/markdown-preview-view.js +++ b/packages/markdown-preview/lib/markdown-preview-view.js @@ -274,7 +274,22 @@ module.exports = class MarkdownPreviewView { return this.getMarkdownSource() .then(source => { if (source != null) { - return this.renderMarkdownText(source) + if (this.loaded) { + return this.renderMarkdownText(source); + } else { + // If we haven't loaded yet, defer before we render the Markdown + // for the first time. This allows the pane to appear and to + // display the loading indicator. Otherwise the first render + // happens before the pane is even visible. + // + // This doesn't slow anything down; it just shifts the work around + // so that the pane appears earlier in the cycle. + return new Promise((resolve) => { + setTimeout(() => { + resolve(this.renderMarkdownText(source)) + }, 0) + }) + } } }) .catch(reason => this.showError({ message: reason })) @@ -339,6 +354,7 @@ module.exports = class MarkdownPreviewView { }) await done(this.element) + this.element.classList.remove('loading') this.emitter.emit('did-change-markdown') this.element.scrollTop = scrollTop @@ -444,11 +460,7 @@ module.exports = class MarkdownPreviewView { showLoading() { this.loading = true - this.element.textContent = '' - const div = document.createElement('div') - div.classList.add('markdown-spinner') - div.textContent = 'Loading Markdown\u2026' - this.element.appendChild(div) + this.element.classList.add('loading') } selectAll() { diff --git a/packages/markdown-preview/spec/markdown-preview-spec.js b/packages/markdown-preview/spec/markdown-preview-spec.js index 8ea5bed7c..64627bcf3 100644 --- a/packages/markdown-preview/spec/markdown-preview-spec.js +++ b/packages/markdown-preview/spec/markdown-preview-spec.js @@ -41,6 +41,13 @@ describe('Markdown Preview', function () { .getActiveItem()) ) + waitsFor( + 'preview to finish loading', + () => { + return !preview.element.classList.contains('loading') + } + ) + runs(() => { expect(preview).toBeInstanceOf(MarkdownPreviewView) expect(preview.getPath()).toBe( diff --git a/packages/markdown-preview/styles/markdown-preview.less b/packages/markdown-preview/styles/markdown-preview.less index 0853c74ef..8e44e81b6 100644 --- a/packages/markdown-preview/styles/markdown-preview.less +++ b/packages/markdown-preview/styles/markdown-preview.less @@ -2,6 +2,7 @@ // Global Markdown Preview styles .markdown-preview { + contain: paint; // Hide a `pre` that comes directly after an `atom-text-editor` because the // `atom-text-editor` is the syntax-highlighted representation. @@ -35,14 +36,38 @@ .task-list-item { list-style-type: none; } + + &.loading { + display: flex; + flex-direction: column; + justify-content: center; + + // `.loading` on the preview element automatically shows the spinner/text. + // We add a slight animation delay so that, when the preview content is + // quick to appear (as usually happens), the spinner won't be shown. It + // only shows up when preview content takes a while to render. + &:before { + display: block; + content: 'Loading Markdown…'; + margin: auto; + background-image: url(images/octocat-spinner-128.gif); + background-repeat: no-repeat; + background-size: 64px; + background-position: top center; + padding-top: 70px; + text-align: center; + opacity: 0; + animation-duration: 1s; + animation-name: appear-after-short-delay; + animation-delay: 0.75s; + animation-fill-mode: forwards; + } + } } -.markdown-spinner { - margin: auto; - background-image: url(images/octocat-spinner-128.gif); - background-repeat: no-repeat; - background-size: 64px; - background-position: top center; - padding-top: 70px; - text-align: center; +// Not an actual animation; we just use an animation so that it can appear +// after a short delay. +@keyframes appear-after-short-delay { + 0% { opacity: 1; } + 100% { opacity: 1; } } From 74589feb9cb0986b614900a505575158bc659f72 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 5 May 2024 10:55:00 -0700 Subject: [PATCH 5/6] Remove the loading indicator when there's a rendering error --- packages/markdown-preview/lib/markdown-preview-view.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/markdown-preview/lib/markdown-preview-view.js b/packages/markdown-preview/lib/markdown-preview-view.js index 78a222efc..52586c106 100644 --- a/packages/markdown-preview/lib/markdown-preview-view.js +++ b/packages/markdown-preview/lib/markdown-preview-view.js @@ -448,6 +448,7 @@ module.exports = class MarkdownPreviewView { showError(result) { this.element.textContent = '' + this.element.classList.remove('loading') const h2 = document.createElement('h2') h2.textContent = 'Previewing Markdown Failed' this.element.appendChild(h2) From 678c1b7bd78942c4bd2deee9ebf142bac37a9bf4 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Tue, 14 May 2024 18:37:24 -0700 Subject: [PATCH 6/6] Address feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maurício Szabo --- packages/markdown-preview/lib/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/markdown-preview/lib/main.js b/packages/markdown-preview/lib/main.js index 14bb95a36..e5981d191 100644 --- a/packages/markdown-preview/lib/main.js +++ b/packages/markdown-preview/lib/main.js @@ -18,7 +18,7 @@ module.exports = { this.style = new CSSStyleSheet() - // When we upgrade Electron, we can push onto `adoptedStyleSheets` + // TODO: When we upgrade Electron, we can push onto `adoptedStyleSheets` // directly. For now, we have to do this silly thing. let styleSheets = Array.from(document.adoptedStyleSheets ?? []) styleSheets.push(this.style)