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

[Locked Labels] Count lone unescaped $ as regular dollar signs #1875

Open
wants to merge 6 commits into
base: labels-util-function
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/small-owls-relate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

[Locked Labels] Count lone unescaped \$ as regular dollar signs in TeX
Original file line number Diff line number Diff line change
Expand Up @@ -1239,9 +1239,9 @@ describe("locked layer", () => {
expect(labels).toHaveLength(3);

// content
expect(labels[0]).toHaveTextContent("\\text{small }\\frac{1}{2}");
expect(labels[1]).toHaveTextContent("\\text{medium }E_0 = mc^2");
expect(labels[2]).toHaveTextContent("\\text{large }\\sqrt{2a}");
expect(labels[0]).toHaveTextContent("\\text{small $\\frac{1}{2}$}");
expect(labels[1]).toHaveTextContent("\\text{medium $E_0 = mc^2$}");
expect(labels[2]).toHaveTextContent("\\text{large $\\sqrt{2a}$}");

// styles
expect(labels[0]).toHaveStyle({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ describe("MafsGraph", () => {
};

render(<MafsGraph {...basePropsWithTexLabels} />);
expect(screen.getByText("1/2")).toBeInTheDocument();
expect(screen.getByText("3/4")).toBeInTheDocument();
expect(screen.getByText("\\text{$1/2$}")).toBeInTheDocument();
expect(screen.getByText("\\text{$3/4$}")).toBeInTheDocument();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anakaren-rojas I think my update affected some of your recent changes. Does this look okay to you?

Copy link
Contributor

@anakaren-rojas anakaren-rojas Nov 20, 2024

Choose a reason for hiding this comment

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

EDIT: Yes, looks good
For these, I think it would be

Copy link
Contributor

Choose a reason for hiding this comment

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

$\frac{1}{2}$ renders correctly when I test it out in storybook, so the functionality updates you made look right

});

it("renders plain text in axis Labels", () => {
Expand Down
175 changes: 166 additions & 9 deletions packages/perseus/src/widgets/interactive-graphs/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,46 +83,44 @@ describe("replaceOutsideTeX", () => {
const mathString = "$x^2$";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("x^2");
expect(convertedString).toEqual("\\text{$x^2$}");
});

test("$s within string", () => {
const mathString = "Expression $x^2$ is exponential";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual(
"\\text{Expression }x^2\\text{ is exponential}",
"\\text{Expression $x^2$ is exponential}",
);
});

test("$s first", () => {
const mathString = "$A$ is square";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("A\\text{ is square}");
expect(convertedString).toEqual("\\text{$A$ is square}");
});

test("regular text first", () => {
const mathString = "Square $A$";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("\\text{Square }A");
expect(convertedString).toEqual("\\text{Square $A$}");
});

test("multiple $s", () => {
const mathString = "$A$ is $B$";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("A\\text{ is }B");
expect(convertedString).toEqual("\\text{$A$ is $B$}");
});

test("multiple $s with surrounding text", () => {
const mathString = "Square $A$ is $B$ also";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual(
"\\text{Square }A\\text{ is }B\\text{ also}",
);
expect(convertedString).toEqual("\\text{Square $A$ is $B$ also}");
});

test("with a real $ inside a regular string", () => {
Expand All @@ -136,7 +134,7 @@ describe("replaceOutsideTeX", () => {
const mathString = "This sandwich is ${$}12$";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("\\text{This sandwich is }{$}12");
expect(convertedString).toEqual("\\text{This sandwich is ${$}12$}");
});

test("escapes curly braces", () => {
Expand All @@ -152,6 +150,165 @@ describe("replaceOutsideTeX", () => {

expect(convertedString).toEqual("\\text{\\\\}");
});

test("treats blockquote syntax as plain text", () => {
const mathString = "> ";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("\\text{> }");
});

test("abc", () => {
const mathString = "abc";
const convertedString = replaceOutsideTeX(mathString);

expect(convertedString).toEqual("\\text{abc}");
});

test.each`
input | expectedOutput
${"$"} | ${"\\text{\\$}"}
${"$$"} | ${"\\text{$$}"}
${"\\$"} | ${"\\text{\\$}"}
${"$1$"} | ${"\\text{$1$}"}
${"$1"} | ${"\\text{\\$1}"}
${"1$"} | ${"\\text{1\\$}"}
${"$$1$"} | ${"\\text{$$1\\$}"}
${"$1$$"} | ${"\\text{$1$\\$}"}
`("counts lone unescaped $ as TeX", ({input, expectedOutput}) => {
const convertedString = replaceOutsideTeX(input);
expect(convertedString).toEqual(expectedOutput);
});
});

describe("mathOnlyParser", () => {
test("empty string", () => {
const nodes = mathOnlyParser("");

expect(nodes).toEqual([]);
});

test("text-only string", () => {
const nodes = mathOnlyParser("abc");

expect(nodes).toEqual([{content: "abc", type: "text"}]);
});

test("math", () => {
const nodes = mathOnlyParser("$x^2$");

expect(nodes).toEqual([{content: "x^2", type: "math"}]);
});

test("math at the start", () => {
const nodes = mathOnlyParser("$x^2$ yippee");

expect(nodes).toEqual([
{content: "x^2", type: "math"},
{content: " yippee", type: "text"},
]);
});

test("math at the end", () => {
const nodes = mathOnlyParser("yippee $x^2$");

expect(nodes).toEqual([
{content: "yippee ", type: "text"},
{content: "x^2", type: "math"},
]);
});

test("math contained within text", () => {
const nodes = mathOnlyParser("The equation is $x^2$ yippee");

expect(nodes).toEqual([
{content: "The equation is ", type: "text"},
{content: "x^2", type: "math"},
{content: " yippee", type: "text"},
]);
});

test("multiple math blocks", () => {
const nodes = mathOnlyParser("$x^2$ and $y^2$");

expect(nodes).toEqual([
{content: "x^2", type: "math"},
{content: " and ", type: "text"},
{content: "y^2", type: "math"},
]);
});

test.each`
character
${">"}
${"> "}
${" "}
${"["}
${"]"}
${"("}
${")"}
${"^"}
${"*"}
${"/"}
`("nonspecial special character as text: '$character'", ({character}) => {
const nodes = mathOnlyParser(character);

expect(nodes).toEqual([{content: character, type: "text"}]);
});

test.each`
character
${"\\"}
${"\\\\"}
${"{"}
${"}"}
${"$"}
${"\\$"}
`("actually special character: '$character'", ({character}) => {
const nodes = mathOnlyParser(character);

expect(nodes).toEqual([{content: character, type: "specialCharacter"}]);
});

test("special character in text", () => {
const nodes = mathOnlyParser("a\\$b");

expect(nodes).toEqual([
{content: "a", type: "text"},
{content: "\\$", type: "specialCharacter"},
{content: "b", type: "text"},
]);
});

test("special character in math", () => {
const nodes = mathOnlyParser("$\\$$");

expect(nodes).toEqual([{content: "\\$", type: "math"}]);
});

test("mix of special characters", () => {
const nodes = mathOnlyParser("\\$\\\\\\$$");

expect(nodes).toEqual([
{content: "\\$", type: "specialCharacter"},
{content: "\\\\", type: "specialCharacter"},
{content: "\\$", type: "specialCharacter"},
{content: "$", type: "specialCharacter"},
]);
});

test("mix all types", () => {
const nodes = mathOnlyParser("Hello \\$ \\\\ world $\\frac{1}{2}$");

expect(nodes).toEqual([
{content: "Hello ", type: "text"},
{content: "\\$", type: "specialCharacter"},
{content: " ", type: "text"},
{content: "\\\\", type: "specialCharacter"},
{content: " world ", type: "text"},
{content: "\\frac{1}{2}", type: "math"},
]);
});
});

describe("mathOnlyParser", () => {
Expand Down
102 changes: 32 additions & 70 deletions packages/perseus/src/widgets/interactive-graphs/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {parse, pureMarkdownRules} from "@khanacademy/pure-markdown";
import {pureMarkdownRules} from "@khanacademy/pure-markdown";
import SimpleMarkdown from "@khanacademy/simple-markdown";

import {clampToBox, inset, MIN, size} from "./math";
Expand Down Expand Up @@ -74,75 +74,6 @@ export function isUnlimitedGraphState(
);
}

/**
* Replace all text outside of the $ TeX blocks with `\\text{...}`
* This way, the entire resulting string can be rendered within <TeX>
* and the text outside of the $ blocks will be non-TeX text.
*/
export function replaceOutsideTeX(mathString: string) {
// All the information we need is in the first section,
// whether it's typed as "blockmath" or "paragraph"
const firstSection = parse(mathString)[0];

// If it's blockMath, the outer level has the full math content.
if (firstSection.type === "blockMath") {
return firstSection.content;
}

// If it's a paragraph, we need to iterate through the sections
// to look for individual math blocks.
const condensedNodes = condenseTextNodes(firstSection.content);
let result = "";

for (const piece of condensedNodes) {
piece.type === "math"
? (result += piece.content)
: (result += `\\text{${escapeSpecialChars(piece.content)}}`);
}

return result;
}

type ParsedNode = {
type: "math" | "text";
content: string;
};

// Helper function for replaceOutsideTeX()
// Condense adjacent text nodes into a single text node
function condenseTextNodes(nodes: ParsedNode[] | undefined): Array<ParsedNode> {
const result: ParsedNode[] = [];

if (!nodes) {
return result;
}

let currentText = "";
for (const node of nodes) {
if (node.type === "math") {
if (currentText) {
result.push({type: "text", content: currentText});
currentText = "";
}
result.push(node);
} else {
currentText += node.content;
}
}

if (currentText) {
result.push({type: "text", content: currentText});
}

return result;
}

// Helper function for replaceOutsideTeX()
function escapeSpecialChars(str) {
// Escape $, \, {, and } characters
return str.replace(/([$\\{}])/g, "\\$1");
}

/**
* Parse a string of text and math into a list of objects with type and content
*
Expand Down Expand Up @@ -171,3 +102,34 @@ export const mathOnlyParser = SimpleMarkdown.parserFor(
},
{inline: true},
);

/**
* Replace all text outside of the $ TeX blocks with `\\text{...}`
* This way, the entire resulting string can be rendered within <TeX>
* and the text outside of the $ blocks will be non-TeX text.
*/
export function replaceOutsideTeX(mathString: string) {
// All the information we need is in the first section,
// whether it's typed as "blockmath" or "paragraph"
const parsed = mathOnlyParser(mathString);

let result = "";

for (const piece of parsed) {
piece.type === "math"
? (result += "$" + piece.content + "$")
: piece.type === "specialCharacter"
? (result += escapeIfUnescaped(piece.content))
: (result += piece.content);
}

return `\\text{${result}}`;
}

function escapeIfUnescaped(character: string) {
if (character.length === 1) {
return "\\" + character;
} else {
return character;
}
}