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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3058e93
[Locked Figure Aria] Fix $ edge cases for spoken math aria labels
nishasy Nov 16, 2024
90c76cd
address PR comments
nishasy Nov 20, 2024
8a22f88
Merge branch 'main' into labels-util-function
nishasy Nov 20, 2024
93cfb8f
Use the new mathOnlyParser
nishasy Nov 20, 2024
f71abc4
Merge branch 'main' into labels-util-function
nishasy Nov 20, 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
5 changes: 5 additions & 0 deletions .changeset/kind-moose-lick.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": patch
---

[Locked Figure Aria] Fix $ edge cases for spoken math aria labels
22 changes: 12 additions & 10 deletions ...itor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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", () => {
Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand All @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});
});
Expand Down
15 changes: 4 additions & 11 deletions ...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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]})`;
Expand Down
22 changes: 12 additions & 10 deletions ...tor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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 = {
Expand Down Expand Up @@ -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.",
});
});

Expand All @@ -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.",
});
});

Expand All @@ -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.",
});
});

Expand All @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});
});
Expand Down
11 changes: 3 additions & 8 deletions ...s-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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.
Expand Down
22 changes: 12 additions & 10 deletions ...-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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", () => {
Expand Down Expand Up @@ -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.",
});
});

Expand All @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});
});
Expand Down
27 changes: 5 additions & 22 deletions ...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions ...editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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", () => {
Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});

Expand Down Expand Up @@ -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.",
});
});
});
Loading