From 7852e1f13d2bf721868f25cdd95a368f8827c30e Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 11 Apr 2023 11:57:02 +0200 Subject: [PATCH 1/5] Fix multiple markdown rendering bugs --- web_src/js/markup/asciicast.js | 2 +- web_src/js/markup/common.js | 8 ++++++++ web_src/js/markup/content.js | 4 ++-- web_src/js/markup/math.js | 18 +++++++----------- web_src/js/markup/mermaid.js | 31 +++++++++++++------------------ 5 files changed, 31 insertions(+), 32 deletions(-) create mode 100644 web_src/js/markup/common.js diff --git a/web_src/js/markup/asciicast.js b/web_src/js/markup/asciicast.js index 902cfcb7316bc..97b18743a15d3 100644 --- a/web_src/js/markup/asciicast.js +++ b/web_src/js/markup/asciicast.js @@ -1,4 +1,4 @@ -export async function renderAsciinemaPlayer() { +export async function renderAsciicast() { const els = document.querySelectorAll('.asciinema-player-container'); if (!els.length) return; diff --git a/web_src/js/markup/common.js b/web_src/js/markup/common.js new file mode 100644 index 0000000000000..251206ffa6171 --- /dev/null +++ b/web_src/js/markup/common.js @@ -0,0 +1,8 @@ +export function displayError(el, err) { + el.classList.remove('is-loading'); + const errorNode = document.createElement('div'); + errorNode.setAttribute('class', 'ui message error markup-block-error gt-mono'); + errorNode.textContent = err.str || err.message || String(err); + el.before(errorNode); + el.setAttribute('data-done', 'true'); +} diff --git a/web_src/js/markup/content.js b/web_src/js/markup/content.js index e4ec3d0b4baa6..1d29dc07f29ad 100644 --- a/web_src/js/markup/content.js +++ b/web_src/js/markup/content.js @@ -1,7 +1,7 @@ import {renderMermaid} from './mermaid.js'; import {renderMath} from './math.js'; import {renderCodeCopy} from './codecopy.js'; -import {renderAsciinemaPlayer} from './asciicast.js'; +import {renderAsciicast} from './asciicast.js'; import {initMarkupTasklist} from './tasklist.js'; // code that runs for all markup content @@ -9,7 +9,7 @@ export function initMarkupContent() { renderMermaid(); renderMath(); renderCodeCopy(); - renderAsciinemaPlayer(); + renderAsciicast(); } // code that only runs for comments diff --git a/web_src/js/markup/math.js b/web_src/js/markup/math.js index dcc656ea82cee..da0d91396f199 100644 --- a/web_src/js/markup/math.js +++ b/web_src/js/markup/math.js @@ -1,14 +1,8 @@ -function displayError(el, err) { - const target = targetElement(el); - target.classList.remove('is-loading'); - const errorNode = document.createElement('div'); - errorNode.setAttribute('class', 'ui message error markup-block-error gt-mono'); - errorNode.textContent = err.str || err.message || String(err); - target.before(errorNode); -} +import {displayError} from './common.js'; function targetElement(el) { - // The target element is either the current element if it has the `is-loading` class or the pre that contains it + // The target element is either the current element if it has the + // `is-loading` class or the pre that contains it return el.classList.contains('is-loading') ? el : el.closest('pre'); } @@ -22,6 +16,8 @@ export async function renderMath() { ]); for (const el of els) { + const target = targetElement(el); + if (target.hasAttribute('data-done')) continue; const source = el.textContent; const displayMode = el.classList.contains('display'); const nodeName = displayMode ? 'p' : 'span'; @@ -33,9 +29,9 @@ export async function renderMath() { maxExpand: 50, displayMode, }); - targetElement(el).replaceWith(tempEl); + target.replaceWith(tempEl); } catch (error) { - displayError(el, error); + displayError(target, error); } } } diff --git a/web_src/js/markup/mermaid.js b/web_src/js/markup/mermaid.js index b519e2dcdc352..2bda17cdc2f66 100644 --- a/web_src/js/markup/mermaid.js +++ b/web_src/js/markup/mermaid.js @@ -1,5 +1,6 @@ import {isDarkTheme} from '../utils.js'; import {makeCodeCopyButton} from './codecopy.js'; +import {displayError} from './common.js'; const {mermaidMaxSourceCharacters} = window.config; @@ -9,14 +10,6 @@ const iframeCss = ` #mermaid {display: block; margin: 0 auto} `; -function displayError(el, err) { - el.closest('pre').classList.remove('is-loading'); - const errorNode = document.createElement('div'); - errorNode.setAttribute('class', 'ui message error markup-block-error gt-mono'); - errorNode.textContent = err.str || err.message || String(err); - el.closest('pre').before(errorNode); -} - export async function renderMermaid() { const els = document.querySelectorAll('.markup code.language-mermaid'); if (!els.length) return; @@ -30,18 +23,19 @@ export async function renderMermaid() { }); for (const el of els) { - const source = el.textContent; + const pre = el.closest('pre'); + if (pre.hasAttribute('data-done')) continue; + const source = el.textContent; if (mermaidMaxSourceCharacters >= 0 && source.length > mermaidMaxSourceCharacters) { - displayError(el, new Error(`Mermaid source of ${source.length} characters exceeds the maximum allowed length of ${mermaidMaxSourceCharacters}.`)); + displayError(pre, new Error(`Mermaid source of ${source.length} characters exceeds the maximum allowed length of ${mermaidMaxSourceCharacters}.`)); continue; } try { await mermaid.parse(source); } catch (err) { - displayError(el, err); - el.closest('pre').classList.remove('is-loading'); + displayError(pre, err); continue; } @@ -49,14 +43,15 @@ export async function renderMermaid() { // can't use bindFunctions here because we can't cross the iframe boundary. This // means js-based interactions won't work but they aren't intended to work either const {svg} = await mermaid.render('mermaid', source); - const heightStr = (svg.match(/viewBox="(.+?)"/) || ['', ''])[1].split(/\s+/)[3]; - if (!heightStr) return displayError(el, new Error('Could not determine chart height')); const iframe = document.createElement('iframe'); iframe.classList.add('markup-render'); - iframe.sandbox = 'allow-scripts'; - iframe.style.height = `${Math.ceil(parseFloat(heightStr))}px`; + iframe.sandbox = 'allow-scripts allow-same-origin'; iframe.srcdoc = `${svg}`; + iframe.addEventListener('load', (e) => { + const height = e.target.contentWindow.document.body.clientHeight; + iframe.style.height = `${height}px`; + }); const mermaidBlock = document.createElement('div'); mermaidBlock.classList.add('mermaid-block'); @@ -66,9 +61,9 @@ export async function renderMermaid() { btn.setAttribute('data-clipboard-text', source); mermaidBlock.append(btn); - el.closest('pre').replaceWith(mermaidBlock); + pre.replaceWith(mermaidBlock); } catch (err) { - displayError(el, err); + displayError(pre, err); } } } From 6f503233c72a986cb3a59fdb912d6ca2f816af0c Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 11 Apr 2023 12:23:56 +0200 Subject: [PATCH 2/5] use data-render-done --- web_src/js/markup/common.js | 2 +- web_src/js/markup/math.js | 2 +- web_src/js/markup/mermaid.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web_src/js/markup/common.js b/web_src/js/markup/common.js index 251206ffa6171..d7a556cad4412 100644 --- a/web_src/js/markup/common.js +++ b/web_src/js/markup/common.js @@ -4,5 +4,5 @@ export function displayError(el, err) { errorNode.setAttribute('class', 'ui message error markup-block-error gt-mono'); errorNode.textContent = err.str || err.message || String(err); el.before(errorNode); - el.setAttribute('data-done', 'true'); + el.setAttribute('data-render-done', 'true'); } diff --git a/web_src/js/markup/math.js b/web_src/js/markup/math.js index da0d91396f199..8427637a0f3d4 100644 --- a/web_src/js/markup/math.js +++ b/web_src/js/markup/math.js @@ -17,7 +17,7 @@ export async function renderMath() { for (const el of els) { const target = targetElement(el); - if (target.hasAttribute('data-done')) continue; + if (target.hasAttribute('data-render-done')) continue; const source = el.textContent; const displayMode = el.classList.contains('display'); const nodeName = displayMode ? 'p' : 'span'; diff --git a/web_src/js/markup/mermaid.js b/web_src/js/markup/mermaid.js index 2bda17cdc2f66..1e96dd957c7ba 100644 --- a/web_src/js/markup/mermaid.js +++ b/web_src/js/markup/mermaid.js @@ -24,7 +24,7 @@ export async function renderMermaid() { for (const el of els) { const pre = el.closest('pre'); - if (pre.hasAttribute('data-done')) continue; + if (pre.hasAttribute('data-render-done')) continue; const source = el.textContent; if (mermaidMaxSourceCharacters >= 0 && source.length > mermaidMaxSourceCharacters) { From 38759ce35cfeefb544dc17464b7cfd0b90f42293 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 11 Apr 2023 12:43:07 +0200 Subject: [PATCH 3/5] use iframe variable --- web_src/js/markup/mermaid.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/markup/mermaid.js b/web_src/js/markup/mermaid.js index 1e96dd957c7ba..3c60820902f6c 100644 --- a/web_src/js/markup/mermaid.js +++ b/web_src/js/markup/mermaid.js @@ -48,8 +48,8 @@ export async function renderMermaid() { iframe.classList.add('markup-render'); iframe.sandbox = 'allow-scripts allow-same-origin'; iframe.srcdoc = `${svg}`; - iframe.addEventListener('load', (e) => { - const height = e.target.contentWindow.document.body.clientHeight; + iframe.addEventListener('load', () => { + const height = iframe.contentWindow.document.body.clientHeight; iframe.style.height = `${height}px`; }); From dd5d2bab4cda5223f80cc821b1af6d53c26d54d5 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 12 Apr 2023 12:54:58 +0200 Subject: [PATCH 4/5] improve mermaid loading sequence --- web_src/css/base.css | 2 +- web_src/css/helpers.css | 3 ++- web_src/js/markup/mermaid.js | 31 +++++++++++++++++-------------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/web_src/css/base.css b/web_src/css/base.css index c48a36c854764..bdf601951b717 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -9,7 +9,7 @@ /* non-color variables */ --border-radius: 0.28571429rem; --opacity-disabled: 0.55; - --height-loading: 12rem; + --height-loading: 16rem; /* base colors */ --color-primary: #4183c4; --color-primary-contrast: #ffffff; diff --git a/web_src/css/helpers.css b/web_src/css/helpers.css index 834a507b68793..256dc751c34d7 100644 --- a/web_src/css/helpers.css +++ b/web_src/css/helpers.css @@ -25,7 +25,8 @@ .gt-overflow-x-scroll { overflow-x: scroll !important; } .gt-cursor-default { cursor: default !important; } .gt-items-start { align-items: flex-start !important; } -.gt-whitespace-pre { white-space: pre !important } +.gt-whitespace-pre { white-space: pre !important; } +.gt-invisible { visibility: hidden !important; } .gt-mono { font-family: var(--fonts-monospace) !important; diff --git a/web_src/js/markup/mermaid.js b/web_src/js/markup/mermaid.js index 3c60820902f6c..865a414c93367 100644 --- a/web_src/js/markup/mermaid.js +++ b/web_src/js/markup/mermaid.js @@ -4,11 +4,9 @@ import {displayError} from './common.js'; const {mermaidMaxSourceCharacters} = window.config; -const iframeCss = ` - :root {color-scheme: normal} - body {margin: 0; padding: 0; overflow: hidden} - #mermaid {display: block; margin: 0 auto} -`; +const iframeCss = `:root {color-scheme: normal} +body {margin: 0; padding: 0; overflow: hidden} +#mermaid {display: block; margin: 0 auto}`; export async function renderMermaid() { const els = document.querySelectorAll('.markup code.language-mermaid'); @@ -45,23 +43,28 @@ export async function renderMermaid() { const {svg} = await mermaid.render('mermaid', source); const iframe = document.createElement('iframe'); - iframe.classList.add('markup-render'); - iframe.sandbox = 'allow-scripts allow-same-origin'; + iframe.classList.add('markup-render', 'gt-invisible'); iframe.srcdoc = `${svg}`; - iframe.addEventListener('load', () => { - const height = iframe.contentWindow.document.body.clientHeight; - iframe.style.height = `${height}px`; - }); const mermaidBlock = document.createElement('div'); - mermaidBlock.classList.add('mermaid-block'); + mermaidBlock.classList.add('mermaid-block', 'is-loading', 'gt-hidden'); mermaidBlock.append(iframe); const btn = makeCodeCopyButton(); btn.setAttribute('data-clipboard-text', source); - mermaidBlock.append(btn); - pre.replaceWith(mermaidBlock); + + iframe.addEventListener('load', () => { + pre.replaceWith(mermaidBlock); + mermaidBlock.classList.remove('gt-hidden'); + iframe.style.height = `${iframe.contentWindow.document.body.clientHeight}px`; + setTimeout(() => { // avoid flash of iframe background + mermaidBlock.classList.remove('is-loading'); + iframe.classList.remove('gt-invisible'); + }, 0); + }); + + document.body.append(mermaidBlock); } catch (err) { displayError(pre, err); } From 38e6431680312b20197799294b77a817e8bfc573 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 12 Apr 2023 15:04:49 +0200 Subject: [PATCH 5/5] use pre for error node, remove border from it --- web_src/css/markup/content.css | 2 +- web_src/js/markup/common.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web_src/css/markup/content.css b/web_src/css/markup/content.css index 90f8c7091e9a6..d0f11e8e76500 100644 --- a/web_src/css/markup/content.css +++ b/web_src/css/markup/content.css @@ -542,7 +542,7 @@ .markup-block-error { display: block !important; /* override fomantic .ui.form .error.message {display: none} */ - border: 1px solid var(--color-error-border) !important; + border: none !important; margin-bottom: 0 !important; border-bottom-left-radius: 0 !important; border-bottom-right-radius: 0 !important; diff --git a/web_src/js/markup/common.js b/web_src/js/markup/common.js index d7a556cad4412..aff4a3242368e 100644 --- a/web_src/js/markup/common.js +++ b/web_src/js/markup/common.js @@ -1,7 +1,7 @@ export function displayError(el, err) { el.classList.remove('is-loading'); - const errorNode = document.createElement('div'); - errorNode.setAttribute('class', 'ui message error markup-block-error gt-mono'); + const errorNode = document.createElement('pre'); + errorNode.setAttribute('class', 'ui message error markup-block-error'); errorNode.textContent = err.str || err.message || String(err); el.before(errorNode); el.setAttribute('data-render-done', 'true');