Skip to content

Commit

Permalink
[SR] Update Linear System strings (#2252)
Browse files Browse the repository at this point in the history
## Summary:
We're doing a final pass on strings now at the end of the phase 1
interactive graph screen reader work.

- Linear system graph descriptions should include the point of intersection
  between the two lines.
- Linear system lines' grab handle labels should have minimal line info
  instead of just repeating the description.

Issue: https://khanacademy.atlassian.net/browse/LEMS-2782

## Test plan:
`yarn jest packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx`

Storybook
- http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-linear-system

Author: nishasy

Reviewers: catandthemachines

Required Reviewers:

Approved By: catandthemachines

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2252
  • Loading branch information
nishasy authored Feb 24, 2025
1 parent e5d17bb commit e7ad604
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 42 deletions.
6 changes: 6 additions & 0 deletions .changeset/clever-cameras-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/kmath": patch
"@khanacademy/perseus": patch
---

[SR] Update Linear System strings
19 changes: 17 additions & 2 deletions packages/kmath/src/geometry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ export function getLineIntersection(
// TODO(LP-10725): update these to be 2-tuples
firstPoints: ReadonlyArray<Coord>,
secondPoints: ReadonlyArray<Coord>,
): string {
): [number, number] | null {
const x1 = firstPoints[0][0];
const y1 = firstPoints[0][1];
const x2 = firstPoints[1][0];
Expand All @@ -330,13 +330,28 @@ export function getLineIntersection(
const determinant = (x1 - x2) * (y3 - y4) - (y1 - y2) * (x3 - x4);

if (Math.abs(determinant) < 1e-9) {
return "Lines are parallel";
// Lines are parallel
return null;
}
const x =
((x1 * y2 - y1 * x2) * (x3 - x4) - (x1 - x2) * (x3 * y4 - y3 * x4)) /
determinant;
const y =
((x1 * y2 - y1 * x2) * (y3 - y4) - (y1 - y2) * (x3 * y4 - y3 * x4)) /
determinant;
return [x, y];
}

export function getLineIntersectionString(
firstPoints: ReadonlyArray<Coord>,
secondPoints: ReadonlyArray<Coord>,
): string {
const intersection = getLineIntersection(firstPoints, secondPoints);

if (intersection === null) {
return "Lines are parallel";
}

const [x, y] = intersection;
return "Intersection: (" + x.toFixed(3) + ", " + y.toFixed(3) + ")";
}
35 changes: 33 additions & 2 deletions packages/perseus/src/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,21 @@ export type PerseusStrings = {
x: string;
y: string;
}): string;
srLinearSystemGrabHandle({
lineNumber,
point1X,
point1Y,
point2X,
point2Y,
}: {
lineNumber: number;
point1X: string;
point1Y: string;
point2X: string;
point2Y: string;
}): string;
srLinearSystemIntersection({x, y}: {x: string; y: string}): string;
srLinearSystemParallel: string;
srRayGraph: string;
srRayPoints: ({
point1X,
Expand Down Expand Up @@ -704,7 +719,7 @@ export const strings = {
srLinearGraphBothIntercepts:
"The line crosses the X-axis at %(xIntercept)s comma 0 and the Y-axis at 0 comma %(yIntercept)s.",
srLinearGraphOriginIntercept:
"The line crosses the x and y axes at the graph's origin.",
"The line crosses the X and Y axes at the graph's origin.",
srLinearGrabHandle:
"Line going through point %(point1X)s comma %(point1Y)s and point %(point2X)s comma %(point2Y)s.",
srAngleStartingSide: "Point 3, starting side at %(x)s comma %(y)s.",
Expand Down Expand Up @@ -736,6 +751,11 @@ export const strings = {
"Line %(lineNumber)s has two points, point 1 at %(point1X)s comma %(point1Y)s and point 2 at %(point2X)s comma %(point2Y)s.",
srLinearSystemPoint:
"Point %(pointSequence)s on line %(lineNumber)s at %(x)s comma %(y)s.",
srLinearSystemGrabHandle:
"Line %(lineNumber)s going through point %(point1X)s comma %(point1Y)s and point %(point2X)s comma %(point2Y)s.",
srLinearSystemIntersection:
"Line 1 and line 2 intersect at point %(x)s comma %(y)s.",
srLinearSystemParallel: "Line 1 and line 2 are parallel.",
srRayGraph: "A ray on a coordinate plane.",
srRayPoints:
"The endpoint is at %(point1X)s comma %(point1Y)s and the ray goes through point %(point2X)s comma %(point2Y)s.",
Expand Down Expand Up @@ -994,7 +1014,7 @@ export const mockStrings: PerseusStrings = {
srLinearGraphBothIntercepts: ({xIntercept, yIntercept}) =>
`The line crosses the X-axis at ${xIntercept} comma 0 and the Y-axis at 0 comma ${yIntercept}.`,
srLinearGraphOriginIntercept:
"The line crosses the x and y axes at the graph's origin.",
"The line crosses the X and Y axes at the graph's origin.",
srLinearGrabHandle: ({point1X, point1Y, point2X, point2Y}) =>
`Line going through point ${point1X} comma ${point1Y} and point ${point2X} comma ${point2Y}.`,
srAngleStartingSide: ({x, y}) =>
Expand Down Expand Up @@ -1053,6 +1073,17 @@ export const mockStrings: PerseusStrings = {
`Line ${lineNumber} has two points, point 1 at ${point1X} comma ${point1Y} and point 2 at ${point2X} comma ${point2Y}.`,
srLinearSystemPoint: ({lineNumber, pointSequence, x, y}) =>
`Point ${pointSequence} on line ${lineNumber} at ${x} comma ${y}.`,
srLinearSystemGrabHandle: ({
lineNumber,
point1X,
point1Y,
point2X,
point2Y,
}) =>
`Line ${lineNumber} going through point ${point1X} comma ${point1Y} and point ${point2X} comma ${point2Y}.`,
srLinearSystemIntersection: ({x, y}) =>
`Line 1 and line 2 intersect at point ${x} comma ${y}.`,
srLinearSystemParallel: "Line 1 and line 2 are parallel.",
srRayGraph: "A ray on a coordinate plane.",
srRayPoints: ({point1X, point1Y, point2X, point2Y}) =>
`The endpoint is at ${point1X} comma ${point1Y} and the ray goes through point ${point2X} comma ${point2Y}.`,
Expand Down
5 changes: 3 additions & 2 deletions packages/perseus/src/widgets/interactive-graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ import type {PropsFor} from "@khanacademy/wonder-blocks-core";

const {getClockwiseAngle} = angles;

const {getLineEquation, getLineIntersection, magnitude, vector} = geometry;
const {getLineEquation, getLineIntersectionString, magnitude, vector} =
geometry;

const defaultBackgroundImage = {
url: null,
Expand Down Expand Up @@ -738,7 +739,7 @@ class InteractiveGraph extends React.Component<Props, State> {
"\n" +
getLineEquation(coords[1][0], coords[1][1]) +
"\n" +
getLineIntersection(coords[0], coords[1])
getLineIntersectionString(coords[0], coords[1])
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe("Linear System graph screen reader", () => {
// Assert
expect(linearSystemGraph).toBeInTheDocument();
expect(linearSystemGraph).toHaveAccessibleDescription(
"Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5. The line crosses the Y-axis at 0 comma 5. Its slope is zero. Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5. The line crosses the Y-axis at 0 comma -5. Its slope is zero.",
"Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5. The line crosses the Y-axis at 0 comma 5. Its slope is zero. Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5. The line crosses the Y-axis at 0 comma -5. Its slope is zero. Line 1 and line 2 are parallel.",
);
});

Expand All @@ -74,8 +74,8 @@ describe("Linear System graph screen reader", () => {
`(`Line $lineNumber`, ({lineNumber}) => {
test.each`
case | coords | interceptDescription
${"origin intercept"} | ${[[1, 1], [2, 2]]} | ${"The line crosses the x and y axes at the graph's origin."}
${"both x and y intercepts"} | ${[[4, 4], [7, 1]]} | ${"The line crosses the X-axis at 8 comma 0 and the Y-axis at 0 comma 8."}
${"origin intercept"} | ${[[1, 1], [2, 2]]} | ${"The line crosses the X and Y axes at the graph's origin."}
${"both X and Y intercepts"} | ${[[4, 4], [7, 1]]} | ${"The line crosses the X-axis at 8 comma 0 and the Y-axis at 0 comma 8."}
${"x intercept only"} | ${[[5, 5], [5, 2]]} | ${"The line crosses the X-axis at 5 comma 0."}
${"y intercept only"} | ${[[5, 5], [2, 5]]} | ${"The line crosses the Y-axis at 0 comma 5."}
${"overlaps y-axis"} | ${[[0, 5], [0, 2]]} | ${"The line crosses the X-axis at 0 comma 0."}
Expand Down Expand Up @@ -177,7 +177,7 @@ describe("Linear System graph screen reader", () => {
);
expect(grabHandle).toHaveAttribute(
"aria-label",
`The line crosses the Y-axis at 0 comma 3. Its slope is zero.`,
`Line ${lineNumber} going through point -2 comma 3 and point 3 comma 3.`,
);
expect(point2).toHaveAttribute(
"aria-label",
Expand All @@ -190,27 +190,72 @@ describe("Linear System graph screen reader", () => {
${"point1"} | ${0}
${"grabHandle"} | ${1}
${"point2"} | ${2}
`("should have describedby on all interactive elements", ({index}) => {
// Arrange
render(
<MafsGraph
{...baseMafsGraphProps}
state={baseLinearSystemState}
/>,
);
`(
"should have describedby on all interactive elements (parallel lines)",
({index}) => {
// Arrange
render(
<MafsGraph
{...baseMafsGraphProps}
state={baseLinearSystemState}
/>,
);

// Act
const interactiveElements = screen.getAllByRole("button");
const element = interactiveElements[index + (lineNumber - 1) * 3];
// Act
const interactiveElements = screen.getAllByRole("button");
const element =
interactiveElements[index + (lineNumber - 1) * 3];

// Assert
expect(element.getAttribute("aria-describedby")).toContain(
"-slope",
);
expect(element.getAttribute("aria-describedby")).toContain(
"-intercept",
);
});
const expectedDescription = `The line crosses the Y-axis at 0 comma ${lineNumber === 1 ? 5 : -5}. Its slope is zero. Line 1 and line 2 are parallel.`;

// Assert
expect(element).toHaveAccessibleDescription(
expectedDescription,
);
},
);

test.each`
element | index
${"point1"} | ${0}
${"grabHandle"} | ${1}
${"point2"} | ${2}
`(
"should have describedby on all interactive elements (intersecting lines)",
({index}) => {
// Arrange
render(
<MafsGraph
{...baseMafsGraphProps}
state={{
...baseLinearSystemState,
coords: [
[
[-2, -2],
[2, 2],
],
[
[-2, 2],
[2, -2],
],
],
}}
/>,
);

// Act
const interactiveElements = screen.getAllByRole("button");
const element =
interactiveElements[index + (lineNumber - 1) * 3];

const expectedDescription = `The line crosses the X and Y axes at the graph's origin. Its slope ${lineNumber === 1 ? "increases" : "decreases"} from left to right. Line 1 and line 2 intersect at point 0 comma 0.`;

// Assert
expect(element).toHaveAccessibleDescription(
expectedDescription,
);
},
);

test.each`
elementName | index
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {geometry} from "@khanacademy/kmath";
import * as React from "react";

import {usePerseusI18n} from "../../../components/i18n-context";
Expand Down Expand Up @@ -39,6 +40,15 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => {

const {strings, locale} = usePerseusI18n();
const id = React.useId();
const intersectionId = `${id}-intersection`;

const intersectionPoint = geometry.getLineIntersection(lines[0], lines[1]);
const intersectionDescription = intersectionPoint
? strings.srLinearSystemIntersection({
x: srFormatNumber(intersectionPoint[0], locale),
y: srFormatNumber(intersectionPoint[1], locale),
})
: strings.srLinearSystemParallel;

const linesAriaInfo = lines.map((line, i) => {
return {
Expand All @@ -60,20 +70,21 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => {
slopeDescription: getSlopeStringForLine(line, strings),
};
});
const individualLineDescriptions = linesAriaInfo
.map(
({
pointsDescriptionId,
interceptDescriptionId,
slopeDescriptionId,
}) =>
`${pointsDescriptionId} ${interceptDescriptionId} ${slopeDescriptionId}`,
)
.join(" ");

return (
<g
aria-label={strings.srLinearSystemGraph}
aria-describedby={linesAriaInfo
.map(
({
pointsDescriptionId,
interceptDescriptionId,
slopeDescriptionId,
}) =>
`${pointsDescriptionId} ${interceptDescriptionId} ${slopeDescriptionId}`,
)
.join(" ")}
aria-describedby={`${individualLineDescriptions} ${intersectionId}`}
>
{lines?.map((line, i) => (
<MovableLine
Expand All @@ -92,9 +103,15 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => {
x: srFormatNumber(line[1][0], locale),
y: srFormatNumber(line[1][1], locale),
}),
grabHandleAriaLabel: `${linesAriaInfo[i].interceptDescription} ${linesAriaInfo[i].slopeDescription}`,
grabHandleAriaLabel: strings.srLinearSystemGrabHandle({
lineNumber: i + 1,
point1X: srFormatNumber(line[0][0], locale),
point1Y: srFormatNumber(line[0][1], locale),
point2X: srFormatNumber(line[1][0], locale),
point2Y: srFormatNumber(line[1][1], locale),
}),
}}
ariaDescribedBy={`${linesAriaInfo[i].interceptDescriptionId} ${linesAriaInfo[i].slopeDescriptionId}`}
ariaDescribedBy={`${linesAriaInfo[i].interceptDescriptionId} ${linesAriaInfo[i].slopeDescriptionId} ${intersectionId}`}
onMoveLine={(delta: vec.Vector2) => {
dispatch(actions.linearSystem.moveLine(i, delta));
}}
Expand Down Expand Up @@ -151,6 +168,9 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => {
</>
),
)}
<g id={intersectionId} style={a11y.srOnly}>
{intersectionDescription}
</g>
</g>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe("Linear graph screen reader", () => {

test.each`
case | coords | interceptDescription
${"origin intercept"} | ${[[1, 1], [2, 2]]} | ${"The line crosses the x and y axes at the graph's origin."}
${"origin intercept"} | ${[[1, 1], [2, 2]]} | ${"The line crosses the X and Y axes at the graph's origin."}
${"both x and y intercepts"} | ${[[4, 4], [7, 1]]} | ${"The line crosses the X-axis at 8 comma 0 and the Y-axis at 0 comma 8."}
${"x intercept only"} | ${[[5, 5], [5, 2]]} | ${"The line crosses the X-axis at 5 comma 0."}
${"y intercept only"} | ${[[5, 5], [2, 5]]} | ${"The line crosses the Y-axis at 0 comma 5."}
Expand Down

0 comments on commit e7ad604

Please sign in to comment.