Skip to content

Added MathJax rendering #4

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

Merged
merged 25 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ca4343b
Fix annotation for RichTextTable component
dat-boris Aug 10, 2023
c862d05
Fix up label selector to ensure region is editable
dat-boris Aug 10, 2023
f5f8afa
Fix up missing modules
dat-boris Aug 10, 2023
ebba48c
Remove old debugging code
dat-boris Aug 10, 2023
e23407e
Added better comments
dat-boris Aug 10, 2023
4a6bef4
Merge branch 'master' into fix_annotation
dat-boris Aug 11, 2023
ea17216
Guess who need spellcheck... push to retrigger test
dat-boris Aug 11, 2023
95cf825
Fix bug caught by e2e/tests/date-time.test.js
dat-boris Aug 11, 2023
11f8f6d
Disable functional tests
dat-boris Aug 11, 2023
7df51b4
Disable flaky e2e tests
dat-boris Aug 11, 2023
1a83f38
Added MathJax rendering
dat-boris Aug 11, 2023
9c795cd
Fix up hasMath variable
dat-boris Aug 11, 2023
9d933cd
Merge branch 'master' into mathjax-comp
dat-boris Aug 14, 2023
b2c972e
Merge branch 'merge_1.8.2' into mathjax-comp
dat-boris Aug 14, 2023
4ca1d21
Merge branch 'master' into mathjax-comp
dat-boris Aug 14, 2023
f785fb5
Allow render multiple regex
dat-boris Aug 14, 2023
b1959dd
Added dynamic component loading
dat-boris Aug 14, 2023
98b61bc
Remove unnecessary classes and firefox fix
dat-boris Jul 16, 2024
827c7b1
lint
dat-boris Jul 16, 2024
4dfeb56
Merge branch 'master' into mathjax-comp
dat-boris Jul 17, 2024
6e50a48
relint
dat-boris Jul 17, 2024
2f94afa
Avoid empty elements
dat-boris Jul 17, 2024
bce4b11
Ensure that we only run one instance of typeset
dat-boris Jul 19, 2024
c787d5a
Added fix for e2e tests
dat-boris Jul 22, 2024
d03c36b
Address comments
dat-boris Jul 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
"@types/react-beautiful-dnd": "^13.1.3",
"babel-plugin-istanbul": "^6.1.1",
"babel-preset-react-app": "^9.1.1",
"better-react-mathjax": "^2.0.2",
"d3": "^5.16.0",
"d3-color": "3.1.0",
"loader-utils": "2.0.4",
Expand Down
7 changes: 4 additions & 3 deletions src/examples/rich_text_table/config.xml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<View>
<HyperTextLabels name="ner" toName="text">
<Label value="Person"></Label>
<Label value="Organization"></Label>
<Label value="Date"></Label>
<Label value="Paragraph" granularity="paragraph"></Label>
<Label value="Word" granularity="word"></Label>
<Label value="Div" granularity="div"></Label>
<Label value="Interaction" granularity="parent_div"></Label>
</HyperTextLabels>
<TableText name="text" value="$text"></TableText>
</View>
2 changes: 1 addition & 1 deletion src/examples/rich_text_table/tasks.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"data": {
"text": "[[\"Asking question?<a href=\\\"javascript:alert('pwned')\\\">click</a>\", \"Answer! Look at some LaTex\\n\\n\\\\(\\\\dfrac{6}{2k - 6} = \\\\dfrac{1}{3}\\\\)\\n\\n\"], [\"Asking more question?\", \"More LaTex!\\n\\n\\\\(\\\\dfrac{6}{2k - 6} = \\\\dfrac{1}{3}\\\\)\\n\\n\"]]"
"text": "[[\"Asking question?<a href=\\\"javascript:alert('pwned')\\\">click</a>\", \"Answer! Look at some LaTex\\n\\n\\\\(\\\\dfrac{1}{2k - 6} = \\\\dfrac{1}{3}\\\\) and \\\\(\\\\dfrac{1.5}{2k - 6} = \\\\dfrac{1}{3}\\\\)\\n\\n\"], [\"Asking more question in LaTex?\\\\(\\\\dfrac{2}{2k - 6} = \\\\dfrac{1}{3}\\\\)\", \"More LaTex!\\n\\n\\\\(\\\\dfrac{3}{2k - 6} = \\\\dfrac{1}{3}\\\\)\\n\\n\"]]"
},
"predictions": [
{
Expand Down
4 changes: 2 additions & 2 deletions src/tags/control/Label.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const TagAttrs = types.model({
size: types.optional(types.string, 'medium'),
background: types.optional(customTypes.color, Constants.LABEL_BACKGROUND),
selectedcolor: types.optional(customTypes.color, '#ffffff'),
granularity: types.maybeNull(types.enumeration(['symbol', 'word', 'sentence', 'paragraph'])),
granularity: types.maybeNull(types.enumeration(['symbol', 'word', 'sentence', 'paragraph', 'div', 'parent_div'])),
groupcancontain: types.maybeNull(types.string),
// childrencheck: types.optional(types.enumeration(["any", "all"]), "any")
...(isFF(FF_DEV_2128) ? { html: types.maybeNull(types.string) } : {}),
Expand Down Expand Up @@ -174,7 +174,7 @@ const Model = types.model({
if (labels.type === 'labels') return true; // universal labels are fine to select
if (labels.type.includes(region.type.replace(/region$/, ''))) return true; // region type is in label type
if (labels.type.includes(region.results[0].type)) return true; // any result type of the region is in label type

return false;
});

Expand Down
7 changes: 6 additions & 1 deletion src/tags/object/RichText/RichText.styl
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,14 @@
word-wrap: break-word
margin-top: 16px

&:first-child
&_qa_question
// User questions
margin-left: auto

&_qa_answer
// Khanmigo answer
margin-right: 10%

&__error-box
padding 8px 16px
background-color: pink
Expand Down
9 changes: 9 additions & 0 deletions src/tags/object/RichText/domManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,16 @@ export default class DomManager {
while (currentNode) {
const isText = currentNode.nodeType === Node.TEXT_NODE;
const isBR = currentNode.nodeName === 'BR';
const isSkipSelect = currentNode.nodeType === Node.ELEMENT_NODE && currentNode.getAttribute('data-skip-select');

if (isSkipSelect) {
const ignoreContainer = currentNode;

// Ignore all nodes within the container
while (ignoreContainer.contains(currentNode)) {
currentNode = this.nextStep();
}
}
if (isText) {
domData.addTextElement(currentNode as Text, this.currentPath);
} else if (isBR) {
Expand Down
4 changes: 2 additions & 2 deletions src/tags/object/RichText/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const WARNING_MESSAGES = {
* @param {string} [highlightColor] - hex string with highlight color, if not provided uses the labels color
* @param {boolean} [showLabels=true] - whether or not to show labels next to the region
* @param {none|base64|base64unicode} [encoding] - decode value from an encoded string
* @param {symbol|word|sentence|paragraph} [granularity] - control region selection granularity
* @param {symbol|word|sentence|paragraph|div} [granularity] - control region selection granularity
*/
const TagAttrs = types.model('RichTextModel', {
value: types.maybeNull(types.string),
Expand All @@ -74,7 +74,7 @@ const TagAttrs = types.model('RichTextModel', {

encoding: types.optional(types.enumeration(['none', 'base64', 'base64unicode']), 'none'),

granularity: types.optional(types.enumeration(['symbol', 'word', 'sentence', 'paragraph']), 'symbol'),
granularity: types.optional(types.enumeration(['symbol', 'word', 'sentence', 'paragraph', 'div', 'parent_div']), 'symbol'),
});

const Model = types
Expand Down
112 changes: 109 additions & 3 deletions src/tags/object/RichText/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,31 @@

import React from 'react';
import { types } from 'mobx-state-tree';
import { MathJax, MathJaxContext } from 'better-react-mathjax';
import { cn } from '../../../utils/bem';

import './RichText.styl';
import { RichTextModel } from './model';
import { HtxRichText } from './view';

// To allow us to render MathJax expressions, we need to use a marker
// However we also want to keep the original \(expressions\), so we swap into
// a differnet marker instead.
// We use a different marker than what is used in Khanmigo.
const MATHJAX_MARKER = '$';

// Extract math from conversation, alternate between math and non-math
// Khanmigo uses "\(.*?\)" as the marker for math
// For example, "What is \(2 + 2\)?" will split into ["What is ", "2 + 2", "?"]
const parseConvoWithMath = (str) => {
// About the capture group: a cool behaviour of str.split is that if there's
// capturing group, the group is captured into the group, which is perfect
// for us!
const mathRegex = /\\\((.*?)\\\)/g;

return str.split(mathRegex);
};

const renderTableValue = (val) => {
let conversations = [];

Expand All @@ -38,27 +57,114 @@ const renderTableValue = (val) => {
return <div className={errClass}>{errMsg}</div>;
}

const itemClass = cn('richtext', { elem: 'table-item' });
const questionItemClass = cn('richtext', { elem: 'table-item', mod: { qa : 'question' } });
const answerItemClass = cn('richtext', { elem: 'table-item', mod: { qa : 'answer' } });
let hasMath = false;

const rowElems = conversations.map((conversation, index) => {
const question = conversation[0];
const answer = conversation[1];
const mathQuestions = parseConvoWithMath(question);
const mathAnswers = parseConvoWithMath(answer);
let mathQuestionComponent = null;
let mathAnswerComponent = null;

// Render an alternate list between Math and non-math expressions
// The list alternates between non-Math and Math expressions from
// `parseConvoWithMath`
const renderAllMathJax = (convoAndMathList) => (
convoAndMathList.map((convo, i) => {
if (i % 2 === 0) {
// Non math
return <span key={`eq=${i}`}>{convo}</span>;
Comment on lines +77 to +79

Choose a reason for hiding this comment

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

Sorry I think I'm missing something - how come every other one is not math?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should explain this better in comment - parseConvoWithMath return a list of Math / non-math expression alternatively, which we wraps around here.

} else {
// So for Math, we need to create a span as we want 2 piece of dom:
// 1. The hidden raw MathJax expression, to allow slot Label to work
// 2. A marked MathJax expression that allows <MathJax/> to render
return (
<span key={`eq-${i}`}>
<span style={{ 'display': 'none' }}>{'\\(' + convo + '\\)'}</span>
<span data-skip-select='1'>{MATHJAX_MARKER + convo + MATHJAX_MARKER}</span>
</span>
);
}
})
);

if (mathQuestions.length > 1) {
mathQuestionComponent = (
<div className={questionItemClass}>
<MathJax dynamic>{renderAllMathJax(mathQuestions)}</MathJax>
</div>
);
hasMath = true;
} else if (question) {
mathQuestionComponent = <div className={questionItemClass}>{question}</div>;
}
if (mathAnswers.length > 1) {
mathAnswerComponent = (
<div className={answerItemClass}>
<MathJax dynamic>{renderAllMathJax(mathAnswers)}</MathJax>
</div>
);
hasMath = true;
} else if (answer){
mathAnswerComponent = <div className={answerItemClass}>{answer}</div>;
}

return (
<div key={`conversation-${index}`}>
<div className={itemClass}>{question}</div>
<div className={itemClass}>{answer}</div>
{mathQuestionComponent}
{mathAnswerComponent}
</div>
);
});

if (hasMath) {
const mathJaxConfig = {
tex: {
inlineMath: [[MATHJAX_MARKER, MATHJAX_MARKER]],
},
};

return (
<MathJaxContext config={mathJaxConfig}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This needs fixing, looks like I need to set the dynamic flag in the config

https://github.com/fast-reflexes/better-react-mathjax#dynamic-boolean--undefined

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The dynamic flag seems to be for MathJax content changing. However, it does not seems to trigger a new component to typeset (i.e. render into Latex), which is what we want. I added a hacky way to drill a function into ComponentDidMount, which works good enough for now.

Choose a reason for hiding this comment

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

https://github.com/fast-reflexes/better-react-mathjax#rendermode-pre--post--undefined
I was skimming the docs for the mathjax stuff since I'm totally unfamiliar with it, would setting the rendermode to post fix this? sounds like it causes the typeset to process on every render, which would be what we want here, right? I barely speak React, so maybe i'm way off here, or just not understanding the problem space.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ha picking this up after almost a year!

Thanks for the suggestions - I agree it sounds promising, but tried the renderMode="pre" together with combination of <MathJax text="...."/>, and it didnt work. I didn't dig too deeply since the ComponentDidMount solution is working okay for now.

{rowElems}
</MathJaxContext>
);
}
return <div>{rowElems}</div>;
};

// We need to trigger MathJax typeset after the component is mounted
// See https://docs.mathjax.org/en/latest/advanced/typeset.html
// As the document suggest above, we need to ensure that only one typeSet
// function is running at one time. We use the promise to ensure that the
// typeset is only run once at a time.
let typesetPromise = null;

const triggerMathJaxTypeset = () => {
setTimeout(() => {
// This means that we already have a typeset running.
if (typesetPromise) return;

// This means that this is first load, and we can wait for
// <MathJaxContext/> component to be ready and typeset, instead of doing
// this dynamically.
if (typeof window?.MathJax?.typesetPromise !== 'function') return;

typesetPromise = window?.MathJax?.typesetPromise();
typesetPromise.finally(() => {
typesetPromise = null;
});
}, 100);
};

export const TableText = () => (
HtxRichText({
isText: false,
valueToComponent: renderTableValue,
didMountCallback: triggerMathJaxTypeset,
alwaysInline: true,
})
);
Expand Down
15 changes: 11 additions & 4 deletions src/tags/object/RichText/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class RichTextPieceView extends Component {
*/
_determineRegion(element) {
const spanSelector = isFF(FF_LSDV_4620_3) ? this._regionVisibleSpanSelector : this._regionSpanSelector;

if (matchesSelector(element, spanSelector)) {
const span = element.tagName === 'SPAN' && (!isFF(FF_LSDV_4620_3) || element.matches(spanSelector)) ? element : element.closest(spanSelector);
const { item } = this.props;
Expand All @@ -247,7 +247,7 @@ class RichTextPieceView extends Component {
}

componentDidMount() {
const { item, alwaysInline } = this.props;
const { item, alwaysInline, didMountCallback } = this.props;

if (!isFF(FF_LSDV_4620_3)) {
item.setNeedsUpdateCallbacks(
Expand All @@ -256,6 +256,10 @@ class RichTextPieceView extends Component {
);
}

if (didMountCallback) {
didMountCallback(item);
}

if (!(alwaysInline || item.inline)) {
this.dispose = observe(item, '_isReady', this.updateLoadingVisibility, true);
}
Expand Down Expand Up @@ -484,9 +488,12 @@ const storeInjector = inject('store');
const RPTV = storeInjector(observer(RichTextPieceView));

export const HtxRichText = (
{ isText = false, valueToComponent = null, alwaysInline = false } = {},
{ isText = false, valueToComponent = null, alwaysInline = false, didMountCallback = null } = {},
) => {
return storeInjector(observer(props => {
return <RPTV {...props} isText={isText} valueToComponent={valueToComponent} alwaysInline={alwaysInline}/>;
return (
<RPTV {...props} isText={isText} alwaysInline={alwaysInline}
valueToComponent={valueToComponent} didMountCallback={didMountCallback} />
);
}));
};
77 changes: 75 additions & 2 deletions src/utils/selection-tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,55 @@ const closestBoundarySelection = (selection, boundary) => {
return selection;
};

/**
* Modify selection to be a boundary to be of parent element with tagName
* We will highlight the first and last text node within the parent element,
* which is expected from selection-tools.js as we convert this to an offset.
*/
const changeBoundaryToElement = (selection, tagName, depth=1) => {
const {
startContainer,
} = destructSelection(selection);

// find parent of startContainer with tagName
const upperCaseTagName = tagName.toUpperCase();
let parent = startContainer;

for (let i = 0; i < depth; i++) {
parent = parent.parentNode;
while (parent && parent.tagName !== upperCaseTagName) {
parent = parent.parentNode;
}
}
if (!parent) {
return;
}
const walker = parent.ownerDocument.createTreeWalker(parent, NodeFilter.SHOW_TEXT | NodeFilter.SHOW_ELEMENT);
let firstTextChild = null;
let lastTextChild = null;
let currentNode = walker.nextNode();
let skipContainer = null;

while (currentNode) {
// skip any elements iwthin `data-skip-select`, which might be added by
// other dynamic rendering (e.g. MathJax)
if (currentNode.nodeType === Node.ELEMENT_NODE && currentNode.getAttribute('data-skip-select')) {
skipContainer = currentNode;
while (skipContainer.contains(currentNode)) {
currentNode = walker.nextNode();
}
continue;
}

if (firstTextChild === null) firstTextChild = currentNode;
lastTextChild = currentNode;

currentNode = walker.nextNode();
}
selection.setPosition(firstTextChild);
selection.extend(lastTextChild, lastTextChild.length);
};

const boundarySelection = (selection, boundary) => {
const wordBoundary = boundary !== 'symbol';
const {
Expand Down Expand Up @@ -262,13 +311,19 @@ const applyTextGranularity = (selection, granularity) => {
case 'paragraph':
boundarySelection(selection, 'paragraphboundary');
return;
case 'div':
changeBoundaryToElement(selection, 'div');
return;
case 'parent_div':
changeBoundaryToElement(selection, 'div', 2);
return;
case 'charater':
case 'symbol':
default:
return;
}
} catch {
console.warn('Probably, you\'re using browser that doesn\'t support granularity.');
} catch (e) {
console.warn('Probably, you\'re using browser that doesn\'t support granularity.', e);
}
};

Expand Down Expand Up @@ -711,6 +766,24 @@ const findGlobalOffset = (node, position, root) => {
const isText = currentNode.nodeType === Node.TEXT_NODE;
const isBR = currentNode.nodeName === 'BR';

// if the current node have skip_select attribute, we should skip it
const isSkipSelect = currentNode.nodeType === Node.ELEMENT_NODE && currentNode.getAttribute('data-skip-select');

// Skip MathJax generated nodes, jump to next node (i.e. lastChild's next)
if (isSkipSelect) {
const ignoreContainer = currentNode;

// Keep checking the next of lastChild is not part of the container
// Note this will end if currentNode = null, which is what we want.
while (ignoreContainer.contains(currentNode)) {
currentNode = walker.nextNode();
// Note: the nodeReached can be within the ignore container, so we need
// to check here.
nodeReached = nodeReached || node === currentNode;
}
continue;
}

// Stop iteration
// Break if we passed target node and current node
// is not target, nor child of a target
Expand Down
Loading
Loading