Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak katex options #21828

Merged
merged 2 commits into from Nov 17, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions web_src/js/markup/math.js
Expand Up @@ -23,12 +23,14 @@ export async function renderMath() {

for (const el of els) {
const source = el.textContent;
const options = {display: el.classList.contains('display')};
const nodeName = el.classList.contains('display') ? 'p' : 'span';

try {
const markup = katex.renderToString(source, options);
const tempEl = document.createElement(options.display ? 'p' : 'span');
tempEl.innerHTML = markup;
const tempEl = document.createElement(nodeName);
katex.render(source, tempEl, {
maxSize: 25,
maxExpand: 50,
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the maxSize?
Won't that break nested fractions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took inspiration from the GitLab config where they use 20 for both values. maxSize should as far as I understand limit the visual size of rendered elements only. This limit is only there to prevent abuse. We can go higher of course, the values are kind of arbritrary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference: #27812
Default value is 1000.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I guess then we can remove this parameter either completely, or we set it to its default value.

});
targetElement(el).replaceWith(tempEl);
} catch (error) {
displayError(el, error);
Expand Down