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

nishasy · 2024-11-16T01:50:55Z

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

Screen.Recording.2024-11-15.at.5.46.21.PM.mov

I received a couple suggestions in PR #1854. I'm implmementing them here: - Wrote a helper function that joins labels together so that we don't have to duplicate that code for every single locked figure. - Stopped calling `generateSpokenMathDetails()` on the whole aria string for the locked figure. Instead, it's now being called within the new helper function so that the labels can individually be converted, and we don't have to worry about the edge case with two different labels having dollar signs and causing all the content in between to be counted as TeX when it shouldn't. - Handled the case with a lone unescaped dollar sign, which should not count as TeX. Note: The lone unescaped dollar isn't handled for the TeX inside the graph itself yet. This will be in the next PR. Issue: https://khanacademy.atlassian.net/browse/LEMS-2548 Test plan: `yarn jest` Storybook - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags - Add two visible labels to the point, with one having "$1 and" as the label and the other having "$1" as the label. - Press the autogenerate button and confirm that the "and" is normal and not "a n d". Also confirm that the dollar signs show up normally. - Add two TeX labels and confirm that the auto-generated text is as expected. - Repeat for all the other locked figures.

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

github-actions · 2024-11-16T01:54:24Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (0b65428) and published it to npm. You
can install it using the tag PR1875.

Example:

yarn add @khanacademy/perseus@PR1875

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1875

github-actions · 2024-11-16T01:54:39Z

Size Change: -270 B (-0.02%)

Total Size: 1.29 MB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	421 kB	-270 B (-0.06%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	39 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	77.9 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-editor/dist/es/index.js`	697 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	3.57 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.5 kB

_{compressed-size-action}

khan-actions-bot · 2024-11-16T01:59:11Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/small-owls-relate.md, packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx, packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx, packages/perseus/src/widgets/interactive-graphs/utils.test.ts, packages/perseus/src/widgets/interactive-graphs/utils.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

benchristel

LGTM!

benchristel · 2024-11-18T19:05:42Z

packages/perseus/src/widgets/interactive-graphs/utils.test.ts

+        ${"$1"}   | ${"\\text{\\$1}"}
+        ${"1$"}   | ${"\\text{1\\$}"}
+        ${"$$1$"} | ${"\\text{1\\$}"}
+        ${"$1$$"} | ${"1\\text{\\$}"}


This is a nice set of test cases :)

nishasy · 2024-11-20T23:29:38Z

packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx

-        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();


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

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

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

anakaren-rojas

Double checked the axis labels and they render correctly when I tried a few different variations of TeX 🎉

anakaren-rojas · 2024-11-20T23:54:11Z

packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx

-        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();


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

anakaren-rojas · 2024-11-20T23:54:48Z

packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx

-        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();


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

nishasy · 2024-11-26T18:40:43Z

The parent pull-request (#1874) has been merged into main, but this branch (tex-lone-dollar) now has conflicts with the new base branch. These conflicts must be resolved before checks can complete on this pull-request.

@catandthemachines

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Patch Changes - [#1896](#1896) [`56fd44806`](56fd448) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Fixing intersecting polygon sides issues for unlimited sided polygon. - [#1894](#1894) [`01edfb8be`](01edfb8) Thanks [@nishasy](https://github.com/nishasy)! - [SR] Add comment about making translation tickets - [#1875](#1875) [`28cae9d2a`](28cae9d) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Labels] Count lone unescaped \\$ as regular dollar signs in TeX ## @khanacademy/[email protected] ### Patch Changes - [#1874](#1874) [`42d25c2d2`](42d25c2) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Fix \\$ edge cases for spoken math aria labels - [#1912](#1912) [`44e78a95d`](44e78a9) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Stop cursor jumps in number input fields - [#1875](#1875) [`28cae9d2a`](28cae9d) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Labels] Count lone unescaped \\$ as regular dollar signs in TeX - Updated dependencies \[[`56fd44806`](56fd448), [`01edfb8be`](01edfb8), [`28cae9d2a`](28cae9d)]: - @khanacademy/[email protected]

## 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

nishasy added 2 commits November 15, 2024 17:18

nishasy self-assigned this Nov 16, 2024

nishasy requested review from benchristel, catandthemachines, anakaren-rojas and a team November 16, 2024 01:52

nishasy marked this pull request as ready for review November 16, 2024 01:58

benchristel approved these changes Nov 18, 2024

View reviewed changes

nishasy marked this pull request as draft November 18, 2024 20:00

nishasy added 6 commits November 19, 2024 17:18

Use math only parser

3379e89

address PR comments

90c76cd

Merge branch 'main' into labels-util-function

8a22f88

Use the new mathOnlyParser

93cfb8f

Merge branch 'labels-util-function' into tex-lone-dollar

7aad8b1

update tests to reflect usage of \text{} around everything now

b6be09d

nishasy mentioned this pull request Nov 20, 2024

[Locked Figure Aria] Fix $ edge cases for spoken math aria labels #1874

Merged

nishasy added 3 commits November 20, 2024 15:19

Merge branch 'main' into labels-util-function

f71abc4

Merge branch 'labels-util-function' into tex-lone-dollar

b57fd42

Update another test that was using replaceOutsideTex() function

7827abc

nishasy marked this pull request as ready for review November 20, 2024 23:27

nishasy commented Nov 20, 2024

View reviewed changes

anakaren-rojas approved these changes Nov 20, 2024

View reviewed changes

catandthemachines approved these changes Nov 21, 2024

View reviewed changes

Base automatically changed from labels-util-function to main November 26, 2024 18:40

Merge branch 'main' into tex-lone-dollar

0b65428

nishasy merged commit 28cae9d into main Nov 26, 2024
9 checks passed

nishasy deleted the tex-lone-dollar branch November 26, 2024 18:46

khan-actions-bot mentioned this pull request Nov 26, 2024

Version Packages #1916

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

nishasy commented Nov 16, 2024 •

edited

Loading

github-actions bot commented Nov 16, 2024 •

edited

Loading

github-actions bot commented Nov 16, 2024 •

edited

Loading

khan-actions-bot commented Nov 16, 2024 •

edited

Loading

benchristel left a comment

benchristel Nov 18, 2024

nishasy Nov 20, 2024

anakaren-rojas Nov 20, 2024 •

edited

Loading

anakaren-rojas Nov 20, 2024

anakaren-rojas left a comment

anakaren-rojas Nov 20, 2024 •

edited

Loading

anakaren-rojas Nov 20, 2024

nishasy commented Nov 26, 2024

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

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

Conversation

nishasy commented Nov 16, 2024 • edited Loading

Summary:

Test plan:

Demo

github-actions bot commented Nov 16, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Nov 16, 2024 • edited Loading

khan-actions-bot commented Nov 16, 2024 • edited Loading

Gerald

benchristel left a comment

Choose a reason for hiding this comment

benchristel Nov 18, 2024

Choose a reason for hiding this comment

nishasy Nov 20, 2024

Choose a reason for hiding this comment

anakaren-rojas Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

anakaren-rojas Nov 20, 2024

Choose a reason for hiding this comment

anakaren-rojas left a comment

Choose a reason for hiding this comment

anakaren-rojas Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

anakaren-rojas Nov 20, 2024

Choose a reason for hiding this comment

nishasy commented Nov 26, 2024

nishasy commented Nov 16, 2024 •

edited

Loading

github-actions bot commented Nov 16, 2024 •

edited

Loading

github-actions bot commented Nov 16, 2024 •

edited

Loading

khan-actions-bot commented Nov 16, 2024 •

edited

Loading

anakaren-rojas Nov 20, 2024 •

edited

Loading

anakaren-rojas Nov 20, 2024 •

edited

Loading