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

## Summary: Right now, if you type "$1" into the locked label input, it shows up as "undefined1" in the graph TeX. - Update the parsed content to count a lone unescaped $ as a regular $. Issue: https://khanacademy.atlassian.net/browse/LEMS-2591 ## Test plan: `yarn jest packages/perseus/src/widgets/interactive-graphs/utils.test.ts` Storybook - Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-with-aria-label - Create a new locked figure - Add a label "$1" and confirm it shows up as "$1" in the preview - Play around with different escaped and unescaped dollar signs and confirm they work as expected ## Demo https://github.com/user-attachments/assets/9f826f2d-9b5c-405e-b5cc-fc20be438dcb Author: nishasy Reviewers: benchristel, nishasy, anakaren-rojas, catandthemachines Required Reviewers: Approved By: benchristel, anakaren-rojas, catandthemachines Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1875
Khan · Nov 26, 2024 · 28cae9d · 28cae9d
1 parent 42d25c2
commit 28cae9d
Show file tree

Hide file tree

Showing 5 changed files with 209 additions and 84 deletions.
diff --git a/.changeset/small-owls-relate.md b/.changeset/small-owls-relate.md
@@ -0,0 +1,6 @@
+---
+"@khanacademy/perseus": patch
+"@khanacademy/perseus-editor": patch
+---
+
+[Locked Labels] Count lone unescaped \$ as regular dollar signs in TeX
diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx
@@ -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({

diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx
@@ -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();
     });
 
     it("renders plain text in axis Labels", () => {

diff --git a/packages/perseus/src/widgets/interactive-graphs/utils.test.ts b/packages/perseus/src/widgets/interactive-graphs/utils.test.ts
@@ -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", () => {
@@ -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", () => {
@@ -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", () => {

diff --git a/packages/perseus/src/widgets/interactive-graphs/utils.ts b/packages/perseus/src/widgets/interactive-graphs/utils.ts
@@ -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";
@@ -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
  *
@@ -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;
+    }
+}