From 42d25c2d2d844d6408d76b07794f171d962b91a7 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 26 Nov 2024 10:40:39 -0800 Subject: [PATCH] [Locked Figure Aria] Fix $ edge cases for spoken math aria labels (#1874) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: I received a couple suggestions in PR https://github.com/Khan/perseus/pull/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. Author: nishasy Reviewers: benchristel, nishasy, catandthemachines, anakaren-rojas Required Reviewers: Approved By: benchristel, 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), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1874 --- .changeset/kind-moose-lick.md | 5 + .../locked-ellipse-settings.test.tsx | 22 ++-- .../locked-ellipse-settings.tsx | 15 +-- .../locked-function-settings.test.tsx | 22 ++-- .../locked-function-settings.tsx | 11 +- .../locked-line-settings.test.tsx | 22 ++-- .../locked-figures/locked-line-settings.tsx | 27 +---- .../locked-point-settings.test.tsx | 18 +-- .../locked-figures/locked-point-settings.tsx | 11 +- .../locked-polygon-settings.test.tsx | 16 +-- .../locked-polygon-settings.tsx | 11 +- .../locked-vector-settings.test.tsx | 16 +-- .../locked-figures/locked-vector-settings.tsx | 11 +- .../locked-figures/util.test.ts | 56 ++++++++++ .../locked-figures/util.ts | 103 +++++++++++++----- 15 files changed, 221 insertions(+), 145 deletions(-) create mode 100644 .changeset/kind-moose-lick.md diff --git a/.changeset/kind-moose-lick.md b/.changeset/kind-moose-lick.md new file mode 100644 index 0000000000..e46aa282d2 --- /dev/null +++ b/.changeset/kind-moose-lick.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-editor": patch +--- + +[Locked Figure Aria] Fix $ edge cases for spoken math aria labels diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx index 61dbab6e06..056ab99f53 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx @@ -6,7 +6,10 @@ import * as React from "react"; import {flags} from "../../../__stories__/flags-for-api-options"; import LockedEllipseSettings from "./locked-ellipse-settings"; -import {getDefaultFigureForType} from "./util"; +import { + getDefaultFigureForType, + mockedJoinLabelsAsSpokenMathForTests, +} from "./util"; import type {UserEvent} from "@testing-library/user-event"; @@ -29,9 +32,8 @@ const defaultLabel = getDefaultFigureForType("label"); // Mock the async function generateSpokenMathDetails jest.mock("./util", () => ({ ...jest.requireActual("./util"), - generateSpokenMathDetails: (input) => { - return Promise.resolve(`Spoken math details for ${input}`); - }, + joinLabelsAsSpokenMath: (input) => + mockedJoinLabelsAsSpokenMathForTests(input), })); describe("LockedEllipseSettings", () => { @@ -391,7 +393,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Circle with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", }); }); @@ -419,7 +421,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Circle with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", }); }); @@ -446,7 +448,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Ellipse with x radius 2 and y radius 3, centered at (0, 0). Appearance solid gray border, with no fill.", + "Ellipse with x radius 2 and y radius 3, centered at (0, 0). Appearance solid gray border, with no fill.", }); }); @@ -474,7 +476,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Ellipse with x radius 2 and y radius 3, centered at (0, 0), rotated by 90 degrees. Appearance solid gray border, with no fill.", + "Ellipse with x radius 2 and y radius 3, centered at (0, 0), rotated by 90 degrees. Appearance solid gray border, with no fill.", }); }); @@ -506,7 +508,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Circle A with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle spoken A with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", }); }); @@ -542,7 +544,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Circle A, B with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle spoken A, spoken B with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx index 5767913f5e..77b6e410ad 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx @@ -22,8 +22,8 @@ import LockedFigureSettingsActions from "./locked-figure-settings-actions"; import LockedLabelSettings from "./locked-label-settings"; import { generateLockedFigureAppearanceDescription, - generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type {LockedFigureSettingsCommonProps} from "./locked-figure-settings"; @@ -68,22 +68,15 @@ const LockedEllipseSettings = (props: Props) => { * with the math details converted into spoken words. */ async function getPrepopulatedAriaLabel() { - let visiblelabel = ""; - if (labels && labels.length > 0) { - visiblelabel += ` ${labels.map((l) => l.text).join(", ")}`; - } + const visiblelabel = await joinLabelsAsSpokenMath(labels); const isCircle = radius[0] === radius[1]; let str = ""; if (isCircle) { - str += await generateSpokenMathDetails( - `Circle${visiblelabel} with radius ${radius[0]}`, - ); + str += `Circle${visiblelabel} with radius ${radius[0]}`; } else { - str += await generateSpokenMathDetails( - `Ellipse${visiblelabel} with x radius ${radius[0]} and y radius ${radius[1]}`, - ); + str += `Ellipse${visiblelabel} with x radius ${radius[0]} and y radius ${radius[1]}`; } str += `, centered at (${center[0]}, ${center[1]})`; diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.test.tsx index 2a86af9b50..5caaf10b6e 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.test.tsx @@ -9,7 +9,10 @@ import {flags} from "../../../__stories__/flags-for-api-options"; // eslint-disable-next-line @typescript-eslint/no-unused-vars import examples from "./locked-function-examples"; import LockedFunctionSettings from "./locked-function-settings"; -import {getDefaultFigureForType} from "./util"; +import { + getDefaultFigureForType, + mockedJoinLabelsAsSpokenMathForTests, +} from "./util"; import type {Props} from "./locked-function-settings"; import type {UserEvent} from "@testing-library/user-event"; @@ -27,9 +30,8 @@ const defaultLabel = getDefaultFigureForType("label"); // Mock the async function generateSpokenMathDetails jest.mock("./util", () => ({ ...jest.requireActual("./util"), - generateSpokenMathDetails: (input) => { - return Promise.resolve(`Spoken math details for ${input}`); - }, + joinLabelsAsSpokenMath: (input) => + mockedJoinLabelsAsSpokenMathForTests(input), })); const exampleEquationsMock = { @@ -687,7 +689,7 @@ describe("Locked Function Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Function with equation y=x^2. Appearance solid gray.", + "Function with equation y=x^2. Appearance solid gray.", }); }); @@ -714,7 +716,7 @@ describe("Locked Function Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Function with equation x=y^2. Appearance solid gray.", + "Function with equation x=y^2. Appearance solid gray.", }); }); @@ -740,7 +742,7 @@ describe("Locked Function Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Function with equation y=x^2, domain from 1 to 2. Appearance solid gray.", + "Function with equation y=x^2, domain from 1 to 2. Appearance solid gray.", }); }); @@ -766,7 +768,7 @@ describe("Locked Function Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Function with equation y=x^2. Appearance solid gray.", + "Function with equation y=x^2. Appearance solid gray.", }); }); @@ -797,7 +799,7 @@ describe("Locked Function Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Function A with equation y=x^2. Appearance solid gray.", + "Function spoken A with equation y=x^2. Appearance solid gray.", }); }); @@ -832,7 +834,7 @@ describe("Locked Function Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Function A, B with equation y=x^2. Appearance solid gray.", + "Function spoken A, spoken B with equation y=x^2. Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx index 047ea2c2a6..a303ec87e2 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx @@ -30,8 +30,8 @@ import examples from "./locked-function-examples"; import LockedLabelSettings from "./locked-label-settings"; import { generateLockedFigureAppearanceDescription, - generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type {LockedFigureSettingsCommonProps} from "./locked-figure-settings"; @@ -90,14 +90,9 @@ const LockedFunctionSettings = (props: Props) => { * with the math details converted into spoken words. */ async function getPrepopulatedAriaLabel() { - let visiblelabel = ""; - if (labels && labels.length > 0) { - visiblelabel += ` ${labels.map((l) => l.text).join(", ")}`; - } + const visiblelabel = await joinLabelsAsSpokenMath(labels); - let str = await generateSpokenMathDetails( - `Function${visiblelabel} with equation ${equationPrefix}${equation}`, - ); + let str = `Function${visiblelabel} with equation ${equationPrefix}${equation}`; // Add the domain/range constraints to the aria label // if they are not the default values. diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx index 50643be9fe..0d444b82e3 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx @@ -6,7 +6,10 @@ import * as React from "react"; import {flags} from "../../../__stories__/flags-for-api-options"; import LockedLineSettings from "./locked-line-settings"; -import {getDefaultFigureForType} from "./util"; +import { + getDefaultFigureForType, + mockedJoinLabelsAsSpokenMathForTests, +} from "./util"; import type {UserEvent} from "@testing-library/user-event"; @@ -29,9 +32,8 @@ const defaultLabel = getDefaultFigureForType("label"); // Mock the async function generateSpokenMathDetails jest.mock("./util", () => ({ ...jest.requireActual("./util"), - generateSpokenMathDetails: (input) => { - return Promise.resolve(`Spoken math details for ${input}`); - }, + joinLabelsAsSpokenMath: (input) => + mockedJoinLabelsAsSpokenMathForTests(input), })); describe("LockedLineSettings", () => { @@ -623,7 +625,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Segment from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Segment from point at (0, 0) to point at (2, 2). Appearance solid gray.", }); }); @@ -649,7 +651,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Line from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Line from point at (0, 0) to point at (2, 2). Appearance solid gray.", }); }); @@ -680,7 +682,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Line A from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Line spoken A from point at (0, 0) to point at (2, 2). Appearance solid gray.", }); }); @@ -715,7 +717,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Line A, B from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Line spoken A, spoken B from point at (0, 0) to point at (2, 2). Appearance solid gray.", }); }); @@ -756,7 +758,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Line A from point C at (0, 0) to point D at (2, 2). Appearance solid gray.", + "Line spoken A from point spoken C at (0, 0) to point spoken D at (2, 2). Appearance solid gray.", }); }); @@ -807,7 +809,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Line A, B from point C, C2 at (0, 0) to point D, D2 at (2, 2). Appearance solid gray.", + "Line spoken A, spoken B from point spoken C, spoken C2 at (0, 0) to point spoken D, spoken D2 at (2, 2). Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx index 070b1fe4e7..4aaece85cf 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx @@ -27,8 +27,8 @@ import LockedLabelSettings from "./locked-label-settings"; import LockedPointSettings from "./locked-point-settings"; import { generateLockedFigureAppearanceDescription, - generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type {LockedFigureSettingsCommonProps} from "./locked-figure-settings"; @@ -79,29 +79,12 @@ const LockedLineSettings = (props: Props) => { * details converted into spoken words. */ async function getPrepopulatedAriaLabel() { - let visiblelabel = ""; - let point1VisibleLabel = ""; - let point2VisibleLabel = ""; + const visiblelabel = await joinLabelsAsSpokenMath(labels); + const point1VisibleLabel = await joinLabelsAsSpokenMath(point1.labels); + const point2VisibleLabel = await joinLabelsAsSpokenMath(point2.labels); - if (labels && labels.length > 0) { - visiblelabel += ` ${labels.map((l) => l.text).join(", ")}`; - } - - if (point1.labels && point1.labels.length > 0) { - point1VisibleLabel += ` ${point1.labels - .map((l) => l.text) - .join(", ")}`; - } + let str = `${capitalizeKind}${visiblelabel} from point${point1VisibleLabel} at (${point1.coord[0]}, ${point1.coord[1]}) to point${point2VisibleLabel} at (${point2.coord[0]}, ${point2.coord[1]})`; - if (point2.labels && point2.labels.length > 0) { - point2VisibleLabel += ` ${point2.labels - .map((l) => l.text) - .join(", ")}`; - } - - let str = await generateSpokenMathDetails( - `${capitalizeKind}${visiblelabel} from point${point1VisibleLabel} at (${point1.coord[0]}, ${point1.coord[1]}) to point${point2VisibleLabel} at (${point2.coord[0]}, ${point2.coord[1]})`, - ); const lineAppearance = generateLockedFigureAppearanceDescription( lineColor, lineStyle, diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx index a7e02bb6fc..8f772e4ed6 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx @@ -6,7 +6,10 @@ import * as React from "react"; import {flags} from "../../../__stories__/flags-for-api-options"; import LockedPointSettings from "./locked-point-settings"; -import {getDefaultFigureForType} from "./util"; +import { + getDefaultFigureForType, + mockedJoinLabelsAsSpokenMathForTests, +} from "./util"; import type {UserEvent} from "@testing-library/user-event"; @@ -29,9 +32,8 @@ const defaultLabel = getDefaultFigureForType("label"); // Mock the async function generateSpokenMathDetails jest.mock("./util", () => ({ ...jest.requireActual("./util"), - generateSpokenMathDetails: (input) => { - return Promise.resolve(`Spoken math details for ${input}`); - }, + joinLabelsAsSpokenMath: (input) => + mockedJoinLabelsAsSpokenMathForTests(input), })); describe("LockedPointSettings", () => { @@ -420,8 +422,7 @@ describe("LockedPointSettings", () => { // generateSpokenMathDetails is mocked to return the input string // with "Spoken math details for " prepended. expect(onChangeProps).toHaveBeenCalledWith({ - ariaLabel: - "Spoken math details for Point at (0, 0). Appearance solid gray.", + ariaLabel: "Point at (0, 0). Appearance solid gray.", }); }); @@ -453,8 +454,7 @@ describe("LockedPointSettings", () => { // generateSpokenMathDetails is mocked to return the input string // with "Spoken math details for " prepended. expect(onChangeProps).toHaveBeenCalledWith({ - ariaLabel: - "Spoken math details for Point A at (0, 0). Appearance solid gray.", + ariaLabel: "Point spoken A at (0, 0). Appearance solid gray.", }); }); @@ -491,7 +491,7 @@ describe("LockedPointSettings", () => { // with "Spoken math details for " prepended. expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Point A, B at (0, 0). Appearance solid gray.", + "Point spoken A, spoken B at (0, 0). Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx index bf985f9aae..103a74c783 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx @@ -24,8 +24,8 @@ import LockedFigureSettingsActions from "./locked-figure-settings-actions"; import LockedLabelSettings from "./locked-label-settings"; import { generateLockedFigureAppearanceDescription, - generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type {LockedFigureSettingsMovementType} from "./locked-figure-settings-actions"; @@ -116,14 +116,9 @@ const LockedPointSettings = (props: Props) => { * "Point label1, label2, label3 at (x, y)". */ async function getPrepopulatedAriaLabel() { - let visiblelabel = ""; - if (labels && labels.length > 0) { - visiblelabel += ` ${labels.map((l) => l.text).join(", ")}`; - } + const visiblelabel = await joinLabelsAsSpokenMath(labels); - let str = await generateSpokenMathDetails( - `Point${visiblelabel} at (${coord[0]}, ${coord[1]})`, - ); + let str = `Point${visiblelabel} at (${coord[0]}, ${coord[1]})`; const pointAppearance = generateLockedFigureAppearanceDescription(pointColor); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx index 88d6182d01..62c3dd0a0a 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx @@ -6,7 +6,10 @@ import * as React from "react"; import {flags} from "../../../__stories__/flags-for-api-options"; import LockedPolygonSettings from "./locked-polygon-settings"; -import {getDefaultFigureForType} from "./util"; +import { + getDefaultFigureForType, + mockedJoinLabelsAsSpokenMathForTests, +} from "./util"; import type {Coord} from "@khanacademy/perseus"; import type {UserEvent} from "@testing-library/user-event"; @@ -30,9 +33,8 @@ const defaultLabel = getDefaultFigureForType("label"); // Mock the async function generateSpokenMathDetails jest.mock("./util", () => ({ ...jest.requireActual("./util"), - generateSpokenMathDetails: (input) => { - return Promise.resolve(`Spoken math details for ${input}`); - }, + joinLabelsAsSpokenMath: (input) => + mockedJoinLabelsAsSpokenMathForTests(input), })); describe("LockedPolygonSettings", () => { @@ -606,7 +608,7 @@ describe("LockedPolygonSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Polygon with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", + "Polygon with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", }); }); @@ -642,7 +644,7 @@ describe("LockedPolygonSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Polygon A with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", + "Polygon spoken A with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", }); }); @@ -682,7 +684,7 @@ describe("LockedPolygonSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Polygon A, B with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", + "Polygon spoken A, spoken B with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx index 30d86d6fae..e56c9075f5 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx @@ -34,8 +34,8 @@ import LockedLabelSettings from "./locked-label-settings"; import PolygonSwatch from "./polygon-swatch"; import { generateLockedFigureAppearanceDescription, - generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type {LockedFigureSettingsCommonProps} from "./locked-figure-settings"; @@ -70,14 +70,9 @@ const LockedPolygonSettings = (props: Props) => { * with the math details converted into spoken words. */ async function getPrepopulatedAriaLabel() { - let visiblelabel = ""; - if (labels && labels.length > 0) { - visiblelabel += ` ${labels.map((l) => l.text).join(", ")}`; - } + const visiblelabel = await joinLabelsAsSpokenMath(labels); - let str = await generateSpokenMathDetails( - `Polygon${visiblelabel} with ${points.length} sides, vertices at `, - ); + let str = `Polygon${visiblelabel} with ${points.length} sides, vertices at `; // Add the coordinates of each point to the aria label str += points.map(([x, y]) => `(${x}, ${y})`).join(", "); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx index 1a84e92b48..1c52f82937 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx @@ -6,7 +6,10 @@ import * as React from "react"; import {flags} from "../../../__stories__/flags-for-api-options"; import LockedVectorSettings from "./locked-vector-settings"; -import {getDefaultFigureForType} from "./util"; +import { + getDefaultFigureForType, + mockedJoinLabelsAsSpokenMathForTests, +} from "./util"; import type {Props} from "./locked-vector-settings"; import type {UserEvent} from "@testing-library/user-event"; @@ -30,9 +33,8 @@ const defaultLabel = getDefaultFigureForType("label"); // Mock the async function generateSpokenMathDetails jest.mock("./util", () => ({ ...jest.requireActual("./util"), - generateSpokenMathDetails: (input) => { - return Promise.resolve(`Spoken math details for ${input}`); - }, + joinLabelsAsSpokenMath: (input) => + mockedJoinLabelsAsSpokenMathForTests(input), })); describe("Locked Vector Settings", () => { @@ -439,7 +441,7 @@ describe("Locked Vector Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Vector from (0, 0) to (2, 2). Appearance solid gray.", + "Vector from (0, 0) to (2, 2). Appearance solid gray.", }); }); @@ -470,7 +472,7 @@ describe("Locked Vector Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Vector A from (0, 0) to (2, 2). Appearance solid gray.", + "Vector spoken A from (0, 0) to (2, 2). Appearance solid gray.", }); }); @@ -505,7 +507,7 @@ describe("Locked Vector Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Spoken math details for Vector A, B from (0, 0) to (2, 2). Appearance solid gray.", + "Vector spoken A, spoken B from (0, 0) to (2, 2). Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx index 50aea7666f..098dd27c9c 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx @@ -25,8 +25,8 @@ import LockedFigureSettingsActions from "./locked-figure-settings-actions"; import LockedLabelSettings from "./locked-label-settings"; import { generateLockedFigureAppearanceDescription, - generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type {LockedFigureSettingsCommonProps} from "./locked-figure-settings"; @@ -70,14 +70,9 @@ const LockedVectorSettings = (props: Props) => { * with the math details converted into spoken words. */ async function getPrepopulatedAriaLabel() { - let visiblelabel = ""; - if (labels && labels.length > 0) { - visiblelabel += ` ${labels.map((l) => l.text).join(", ")}`; - } + const visiblelabel = await joinLabelsAsSpokenMath(labels); - let str = await generateSpokenMathDetails( - `Vector${visiblelabel} from (${tail[0]}, ${tail[1]}) to (${tip[0]}, ${tip[1]})`, - ); + let str = `Vector${visiblelabel} from (${tail[0]}, ${tail[1]}) to (${tip[0]}, ${tip[1]})`; const vectorAppearance = generateLockedFigureAppearanceDescription(lineColor); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts index b393be5f27..4a32da91d3 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts @@ -2,11 +2,13 @@ import { generateLockedFigureAppearanceDescription, generateSpokenMathDetails, getDefaultFigureForType, + joinLabelsAsSpokenMath, } from "./util"; import type { LockedFigureColor, LockedFigureFillType, + LockedLabelType, LockedLineStyle, } from "@khanacademy/perseus"; @@ -339,4 +341,58 @@ describe("generateMathDetails", () => { expect(convertedString).toBe("\\"); }); + + test("Should read lone dollar signs as regular dollar signs", async () => { + const mathString = "$50"; + const convertedString = await generateSpokenMathDetails(mathString); + + expect(convertedString).toBe("$50"); + }); + + test("Should read lone escaped dollar signs in text as regular dollar signs", async () => { + const mathString = "\\$50"; + const convertedString = await generateSpokenMathDetails(mathString); + + expect(convertedString).toBe("$50"); + }); +}); + +describe("joinLabelsAsSpokenText", () => { + test("returns empty string for undefined input", async () => { + const actualOutput = await joinLabelsAsSpokenMath(undefined); + + expect(actualOutput).toBe(""); + }); + + test("return empty string if input is an empty array", async () => { + const actualOutput = await joinLabelsAsSpokenMath([]); + + expect(actualOutput).toBe(""); + }); + + test.each` + input | expectedOutput + ${["a"]} | ${" a"} + ${["a", "b"]} | ${" a, b"} + ${["$A$", "$B$"]} | ${" upper A, upper B"} + ${["$1", "$2"]} | ${" $1, $2"} + ${["\\$1", "\\$2"]} | ${" $1, $2"} + ${["$\\$1$", "$\\$2$"]} | ${" normal dollar sign 1, normal dollar sign 2"} + ${["${$}1$", "${$}2$"]} | ${" dollar sign 1, dollar sign 2"} + ${["$$1$", "$$2$"]} | ${" 1$, 2$"} + ${["hello $world$"]} | ${" hello w o r l d"} + ${["$hello$ world"]} | ${" h e l l o world"} + ${["x^2"]} | ${" x^2"} + ${["$x^2$"]} | ${" x Superscript 2"} + ${["{}"]} | ${" {}"} + ${["${}$"]} | ${" "} + `("should join labels", async ({input, expectedOutput}) => { + const lockedLabels: LockedLabelType[] = input.map((label) => { + return {...getDefaultFigureForType("label"), text: label}; + }); + + const actualOutput = await joinLabelsAsSpokenMath(lockedLabels); + + expect(actualOutput).toBe(expectedOutput); + }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts index b81723cecc..500fd06b00 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts @@ -1,21 +1,20 @@ import {SpeechRuleEngine} from "@khanacademy/mathjax-renderer"; -import * as SimpleMarkdown from "@khanacademy/pure-markdown"; -import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core"; - -import type { - LockedEllipseType, - LockedFigure, - LockedFigureColor, - LockedFigureFillType, - LockedFigureType, - LockedFunctionType, - LockedLabelType, - LockedLineType, - LockedPointType, - LockedPolygonType, - LockedVectorType, - LockedLineStyle, +import { + type LockedEllipseType, + type LockedFigure, + type LockedFigureColor, + type LockedFigureFillType, + type LockedFigureType, + type LockedFunctionType, + type LockedLabelType, + type LockedLineType, + type LockedPointType, + type LockedPolygonType, + type LockedVectorType, + type LockedLineStyle, + mathOnlyParser, } from "@khanacademy/perseus"; +import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core"; const DEFAULT_COLOR = "grayH"; @@ -127,28 +126,78 @@ export function generateLockedFigureAppearanceDescription( } } +/** + * Given a string that may contain math within TeX represented by $...$, + * returns the spoken math equivalent using the SpeechRuleEngine. + * Exported for testing. + * + * Example: "Circle with radius $\frac{1}{2}$" ==> "Circle with radius one half" + */ export async function generateSpokenMathDetails(mathString: string) { const engine = await SpeechRuleEngine.setup("en"); let convertedSpeech = ""; // All the information we need is in the first section, // whether it's typed as "blockmath" or "paragraph" - const firstSection = SimpleMarkdown.parse(mathString)[0]; - - // If it's blockMath, the outer level has the full math content. - if (firstSection.type === "blockMath") { - convertedSpeech += engine.texToSpeech(firstSection.content); - } + const parsedContent = mathOnlyParser(mathString); // If it's a paragraph, we need to iterate through the sections // to look for individual math blocks. - if (firstSection.type === "paragraph") { - for (const piece of firstSection.content) { - piece.type === "math" - ? (convertedSpeech += engine.texToSpeech(piece.content)) - : (convertedSpeech += piece.content); + for (const piece of parsedContent) { + switch (piece.type) { + case "math": + convertedSpeech += engine.texToSpeech(piece.content); + break; + case "specialCharacter": + // We don't want the backslash from special character + // to show up in the generated aria label. + piece.content.length > 1 + ? (convertedSpeech += piece.content.slice(1)) + : (convertedSpeech += piece.content); + break; + default: + convertedSpeech += piece.content; + break; } } return convertedSpeech; } + +/** + * Take an array of LockedLabelType object and joins the text of each label + * with a comma and space in between. The text of each label is converted to + * spoken math using the SpeechRuleEngine. + */ +export async function joinLabelsAsSpokenMath( + labels: LockedLabelType[] | undefined, +): Promise { + if (!labels || labels.length === 0) { + return ""; + } + + const spokenLabelPromises = labels.map((label) => { + return generateSpokenMathDetails(label.text); + }); + + const spokenLabels = await Promise.all(spokenLabelPromises); + + return ` ${spokenLabels.join(", ")}`; +} + +// TODO(LEMS-2616): Stop using this mock in tests once we update the +// speech rule engine to read locale data from local files. +/** + * Non-async mocked version of joinLabelsAsSpokenMath for tests. + */ +export function mockedJoinLabelsAsSpokenMathForTests( + labels: LockedLabelType[] | undefined, +) { + if (!labels || labels.length === 0) { + return Promise.resolve(""); + } + + // Mock this so that each label's text says "spoken" before it. + const jointMock = labels.map((input) => ` spoken ${input.text}`).join(","); + return Promise.resolve(jointMock); +}