Skip to content
代码片段 群组 项目
未验证 提交 2d29ad27 编辑于 作者: Mark Florian's avatar Mark Florian
浏览文件

Fix math block alert rendering

This replaces the static markup to recreate an alert with a real
`GlAlert` instance for maths blocks. This fixes incorrect styling of the
alert, incorrect positioning of the `<copy-code>` element, and also
simplifies wiring up of events.

Before, the `<copy-code>` button was displaying over the alert's dismiss
button. Now, it correctly displays within the maths code block it's
associated with.

Since `<copy-code>` is absolutely positioned, and its nearest positioned
ancestor is the `.js-markdown-code` element, and the alert was the first
child of that element, the `<copy-code>` button was rendered over the
alert instead of the code block below.

Now, the alert is the immediate previous sibling of the
`.js-markdown-code` element, instead of its first child.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/455642 and
https://gitlab.com/gitlab-org/gitlab/-/issues/455665.

Changelog: fixed
上级 96b9c5b4
No related branches found
No related tags found
无相关合并请求
import { GlAlert } from '@gitlab/ui';
import { escape } from 'lodash';
import { spriteIcon } from '~/lib/utils/common_utils';
import Vue from 'vue';
import { differenceInMilliseconds } from '~/lib/utils/datetime_utility';
import { s__, sprintf } from '~/locale';
......@@ -14,6 +15,7 @@ const MAX_MATH_CHARS = 1000;
const MAX_MACRO_EXPANSIONS = 1000;
const MAX_USER_SPECIFIED_EMS = 20;
const MAX_RENDER_TIME_MS = 2000;
const LAZY_ALERT_SHOWN_CLASS = 'lazy-alert-shown';
// Wait for the browser to reflow the layout. Reflowing SVG takes time.
// This has to wrap the inner function, otherwise IE/Edge throw "invalid calling object".
......@@ -74,7 +76,6 @@ class SafeMathRenderer {
this.renderElement = this.renderElement.bind(this);
this.render = this.render.bind(this);
this.attachEvents = this.attachEvents.bind(this);
this.pageName = document.querySelector('body').dataset.page;
}
......@@ -86,48 +87,30 @@ class SafeMathRenderer {
const el = chosenEl || this.queue.shift();
const forceRender = Boolean(chosenEl) || !gon.math_rendering_limits_enabled;
const text = el.textContent;
const isTextTooLong = text.length > MAX_MATH_CHARS;
el.removeAttribute('style');
if (!forceRender && (this.totalMS >= MAX_RENDER_TIME_MS || text.length > MAX_MATH_CHARS)) {
// Show un-rendered math code
const codeElement = document.createElement('pre');
codeElement.className = 'code';
codeElement.textContent = el.textContent;
codeElement.dataset.mathStyle = el.dataset.mathStyle;
let message;
if (text.length > MAX_MATH_CHARS) {
message = sprintf(
s__(
'math|This math block exceeds %{maxMathChars} characters, and may cause performance issues on this page.',
),
{ maxMathChars: MAX_MATH_CHARS },
);
} else {
message = s__('math|Displaying this math block may cause performance issues on this page.');
}
const html = `
<div class="alert gl-alert gl-alert-warning alert-dismissible lazy-render-math-container js-lazy-render-math-container fade show" role="alert">
${spriteIcon('warning', 'gl-text-orange-600 s16 gl-alert-icon')}
<div class="display-flex gl-alert-content">
<div>${message}</div>
<div class="gl-alert-actions">
<button class="js-lazy-render-math btn gl-alert-action btn-confirm btn-md gl-button">Display anyway</button>
</div>
</div>
<button type="button" class="close js-close" aria-label="Close">
${spriteIcon('close', 's16')}
</button>
</div>
`;
if (!el.classList.contains('lazy-alert-shown')) {
// eslint-disable-next-line no-unsanitized/property
el.innerHTML = html;
el.append(codeElement);
el.classList.add('lazy-alert-shown');
if (!forceRender && (this.totalMS >= MAX_RENDER_TIME_MS || isTextTooLong)) {
if (!el.classList.contains(LAZY_ALERT_SHOWN_CLASS)) {
el.classList.add(LAZY_ALERT_SHOWN_CLASS);
// Show un-rendered math code
const codeElement = document.createElement('pre');
codeElement.className = 'code';
codeElement.textContent = el.textContent;
codeElement.dataset.mathStyle = el.dataset.mathStyle;
el.replaceChildren(codeElement);
this.renderAlert({
// We do not want to put the alert in the <copy-code> element's nearest
// positioned ancestor, otherwise it will display over the alert instead of
// the code block. Instead, put the alert *before* that ancestor.
mountBeforeEl: el.closest('.js-markdown-code'),
isTextTooLong,
onDisplayAnyway: () => {
this.renderElement(codeElement);
},
});
}
// Render the next math
......@@ -194,27 +177,45 @@ class SafeMathRenderer {
setTimeout(this.renderElement, 400);
}
attachEvents() {
document.body.addEventListener('click', (event) => {
const alert = event.target.closest('.js-lazy-render-math-container');
// eslint-disable-next-line class-methods-use-this
renderAlert({ mountBeforeEl, isTextTooLong, onDisplayAnyway }) {
let alert;
if (!alert) {
return;
}
const dismiss = () => {
alert.$destroy();
alert.$el.remove();
};
// Handle alert close
if (event.target.closest('.js-close')) {
alert.remove();
return;
}
const displayAnyway = () => {
dismiss();
onDisplayAnyway();
};
// Handle "render anyway"
if (event.target.classList.contains('js-lazy-render-math')) {
const pre = alert.nextElementSibling;
alert.remove();
this.renderElement(pre);
}
const message = isTextTooLong
? sprintf(
s__(
'math|This math block exceeds %{maxMathChars} characters, and may cause performance issues on this page.',
),
{ maxMathChars: MAX_MATH_CHARS },
)
: s__('math|Displaying this math block may cause performance issues on this page.');
alert = new Vue({
render(h) {
return h(
GlAlert,
{
class: 'gl-mb-5',
props: { variant: 'warning', primaryButtonText: s__('math|Display anyway') },
on: { dismiss, primaryAction: displayAnyway },
},
message,
);
},
});
alert.$mount();
mountBeforeEl.before(alert.$el);
}
}
......@@ -227,7 +228,6 @@ export default function renderMath(elements) {
.then(([katex]) => {
const renderer = new SafeMathRenderer(elements, katex);
renderer.render();
renderer.attachEvents();
})
.catch(() => {});
}
......@@ -61028,6 +61028,9 @@ msgstr ""
msgid "manual"
msgstr ""
 
msgid "math|Display anyway"
msgstr ""
msgid "math|Displaying this math block may cause performance issues on this page."
msgstr ""
 
......@@ -70,7 +70,7 @@
create_and_visit_issue_with_description(lazy_load_description)
page.within '.description > .md' do
expect(page).to have_selector('.js-lazy-render-math-container', text: /math block exceeds 1000 characters/)
expect(page).to have_selector('[role="alert"]', text: /math block exceeds 1000 characters/)
end
end
......@@ -111,9 +111,9 @@
# the find is needed to ensure the lazy container is loaded, otherwise
# it can be a flaky test, similar to
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25408
find('.js-lazy-render-math-container')
find('[role="alert"]')
expect(page).to have_selector('.js-lazy-render-math-container', text: /math block exceeds 1000 characters/)
expect(page).to have_selector('[role="alert"]', text: /math block exceeds 1000 characters/)
end
end
end
......@@ -128,8 +128,7 @@
page.within '.description > .md' do
expect(page).not_to have_selector('button', text: 'Display anyway')
expect(page)
.not_to have_selector('.js-lazy-render-math-container', text: /math block exceeds 1000 characters/)
expect(page).not_to have_selector('[role="alert"]', text: /math block exceeds 1000 characters/)
end
end
......
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册