From e7ad604afce45feab7d268582ec6db41d6ab5e06 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Mon, 24 Feb 2025 14:31:30 -0800 Subject: [PATCH] [SR] Update Linear System strings (#2252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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: https://github.com/Khan/perseus/pull/2252 --- .changeset/clever-cameras-thank.md | 6 ++ packages/kmath/src/geometry.ts | 19 +++- packages/perseus/src/strings.ts | 35 ++++++- .../perseus/src/widgets/interactive-graph.tsx | 5 +- .../graphs/linear-system.test.tsx | 91 ++++++++++++++----- .../graphs/linear-system.tsx | 44 ++++++--- .../interactive-graphs/graphs/linear.test.tsx | 2 +- 7 files changed, 160 insertions(+), 42 deletions(-) create mode 100644 .changeset/clever-cameras-thank.md diff --git a/.changeset/clever-cameras-thank.md b/.changeset/clever-cameras-thank.md new file mode 100644 index 0000000000..a15618ec53 --- /dev/null +++ b/.changeset/clever-cameras-thank.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/kmath": patch +"@khanacademy/perseus": patch +--- + +[SR] Update Linear System strings diff --git a/packages/kmath/src/geometry.ts b/packages/kmath/src/geometry.ts index 3ded900a65..3cdd65b44f 100644 --- a/packages/kmath/src/geometry.ts +++ b/packages/kmath/src/geometry.ts @@ -317,7 +317,7 @@ export function getLineIntersection( // TODO(LP-10725): update these to be 2-tuples firstPoints: ReadonlyArray, secondPoints: ReadonlyArray, -): string { +): [number, number] | null { const x1 = firstPoints[0][0]; const y1 = firstPoints[0][1]; const x2 = firstPoints[1][0]; @@ -330,7 +330,8 @@ 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)) / @@ -338,5 +339,19 @@ export function getLineIntersection( const y = ((x1 * y2 - y1 * x2) * (y3 - y4) - (y1 - y2) * (x3 * y4 - y3 * x4)) / determinant; + return [x, y]; +} + +export function getLineIntersectionString( + firstPoints: ReadonlyArray, + secondPoints: ReadonlyArray, +): 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) + ")"; } diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index 582605b304..536965c469 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -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, @@ -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.", @@ -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.", @@ -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}) => @@ -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}.`, diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index d216446164..a7f3e4e2da 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -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, @@ -738,7 +739,7 @@ class InteractiveGraph extends React.Component { "\n" + getLineEquation(coords[1][0], coords[1][1]) + "\n" + - getLineIntersection(coords[0], coords[1]) + getLineIntersectionString(coords[0], coords[1]) ); } diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx index b44796ee1a..20ae199993 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx @@ -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.", ); }); @@ -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."} @@ -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", @@ -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( - , - ); + `( + "should have describedby on all interactive elements (parallel lines)", + ({index}) => { + // Arrange + render( + , + ); - // 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( + , + ); + + // 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 diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx index 0bff6b4bb3..c0a02d54b5 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -1,3 +1,4 @@ +import {geometry} from "@khanacademy/kmath"; import * as React from "react"; import {usePerseusI18n} from "../../../components/i18n-context"; @@ -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 { @@ -60,20 +70,21 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { slopeDescription: getSlopeStringForLine(line, strings), }; }); + const individualLineDescriptions = linesAriaInfo + .map( + ({ + pointsDescriptionId, + interceptDescriptionId, + slopeDescriptionId, + }) => + `${pointsDescriptionId} ${interceptDescriptionId} ${slopeDescriptionId}`, + ) + .join(" "); return ( - `${pointsDescriptionId} ${interceptDescriptionId} ${slopeDescriptionId}`, - ) - .join(" ")} + aria-describedby={`${individualLineDescriptions} ${intersectionId}`} > {lines?.map((line, i) => ( { 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)); }} @@ -151,6 +168,9 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { ), )} + + {intersectionDescription} + ); }; diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx index 2e91ec8412..d9da039cff 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx @@ -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."}