From 30333ff7b2b0c5dccee63c45b5ef98a6315c43d2 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 17 Dec 2024 17:04:33 -0800 Subject: [PATCH 01/18] [SR] Linear graph - add grab handle description and aria lives The [SRUX doc](https://khanacademy.atlassian.net/wiki/spaces/LC/pages/3460366348/Linear) still needs a label for the grab handle, but I tried my best in the meantime. - Add a label and describedby for the grab handle. - Add aria-live states for the different interactive elements so they don't override each other. Issue: https://khanacademy.atlassian.net/browse/LEMS-1726 Test plan: `yarn jest packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx` Storybook - http://localhost:6006/iframe.html?id=perseuseditor-widgets-interactive-graph--interactive-graph-linear&viewMode=story - Try all the different slopes and intercepts - Move different elements and confirm that the updated aria-label is what is read out, and none of the other elements override the currently focused one. --- packages/perseus/src/strings.ts | 19 ++++ .../__snapshots__/movable-line.test.tsx.snap | 18 ++- .../graphs/components/movable-line.tsx | 63 +++++++++-- .../interactive-graphs/graphs/linear.test.tsx | 103 ++++++++++++++++-- .../interactive-graphs/graphs/linear.tsx | 7 ++ 5 files changed, 186 insertions(+), 24 deletions(-) diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index 58ccd2b6d7..6df397b3da 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -209,6 +209,17 @@ export type PerseusStrings = { yIntercept: string; }) => string; srLinearGraphOriginIntercept: string; + srLinearGrabHandle: ({ + point1X, + point1Y, + point2X, + point2Y, + }: { + point1X: string; + point1Y: string; + point2X: string; + point2Y: string; + }) => string; // The above strings are used for interactive graph SR descriptions. }; @@ -478,6 +489,12 @@ export const strings: { "Screenreader-only description of the line's intercept when the intercept is the graph's origin.", message: "The line crosses the x and y axes at the graph's origin.", }, + srLinearGrabHandle: { + context: + "Screenreader-only label on the grab handle for the line on a linear graph.", + message: + "Line from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", + }, // The above strings are used for interactive graph SR descriptions. }; @@ -678,5 +695,7 @@ export const mockStrings: PerseusStrings = { `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.", + srLinearGrabHandle: ({point1X, point1Y, point2X, point2Y}) => + `Line from ${point1X} comma ${point1Y} to ${point2X} comma ${point2Y}.`, // The above strings are used for interactive graph SR descriptions. }; diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/__snapshots__/movable-line.test.tsx.snap b/packages/perseus/src/widgets/interactive-graphs/graphs/components/__snapshots__/movable-line.test.tsx.snap index 560d781cdf..b3e109b8af 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/components/__snapshots__/movable-line.test.tsx.snap +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/components/__snapshots__/movable-line.test.tsx.snap @@ -16,15 +16,17 @@ exports[`Rendering Does NOT render extensions of line when option is disabled 1` > @@ -61,7 +63,7 @@ exports[`Rendering Does NOT render extensions of line when option is disabled 1` @@ -201,7 +205,7 @@ exports[`Rendering Does NOT render extensions of line when option is not provide @@ -395,7 +401,7 @@ exports[`Rendering Does render extensions of line when option is enabled 1`] = ` { const { + points: [start, end], ariaLabels, ariaDescribedBy, - onMoveLine = () => {}, - onMovePoint = () => {}, color, - points: [start, end], extend, + onMoveLine = () => {}, + onMovePoint = () => {}, } = props; + // Aria live states for (0) point 1, (1) point 2, and (2) grab handle. + // When moving an element, set its aria live to "polite" and the others + // to "off". Otherwise, other connected elements that move at the same + // time might override the currently focused element's aria live. + const [ariaLives, setAriaLives] = React.useState>([ + "off", + "off", + "off", + ]); + // We use separate focusableHandle elements, instead of letting the movable // points themselves be focusable, to allow the tab order of the points to // be different from the rendering order. We had to solve for the following @@ -59,28 +71,42 @@ export const MovableLine = (props: Props) => { useControlPoint({ ariaLabel: ariaLabels?.point1AriaLabel, ariaDescribedBy: ariaDescribedBy, + ariaLive: ariaLives[0], point: start, sequenceNumber: 1, color, - onMove: (p) => onMovePoint(0, p), + onMove: (p) => { + setAriaLives(["polite", "off", "off"]); + onMovePoint(0, p); + }, }); const {visiblePoint: visiblePoint2, focusableHandle: focusableHandle2} = useControlPoint({ ariaLabel: ariaLabels?.point2AriaLabel, ariaDescribedBy: ariaDescribedBy, + ariaLive: ariaLives[1], point: end, sequenceNumber: 2, color, - onMove: (p) => onMovePoint(1, p), + onMove: (p) => { + setAriaLives(["off", "polite", "off"]); + onMovePoint(1, p); + }, }); const line = ( { + setAriaLives(["off", "off", "polite"]); + onMoveLine(delta); + }} /> ); @@ -100,8 +126,9 @@ const defaultStroke = "var(--movable-line-stroke-color)"; type LineProps = { start: vec.Vector2; end: vec.Vector2; - onMove: (delta: vec.Vector2) => unknown; - stroke?: string | undefined; + ariaLabel?: string; + ariaDescribedBy?: string; + ariaLive?: AriaLive; /* Extends the line to the edge of the graph with an arrow */ extend?: | undefined @@ -109,10 +136,21 @@ type LineProps = { start: boolean; end: boolean; }; + stroke?: string | undefined; + onMove: (delta: vec.Vector2) => unknown; }; const Line = (props: LineProps) => { - const {start, end, onMove, extend, stroke = defaultStroke} = props; + const { + start, + end, + ariaLabel, + ariaDescribedBy, + ariaLive, + extend, + stroke = defaultStroke, + onMove, + } = props; const [startPtPx, endPtPx] = useTransformVectorsToPixels(start, end); const { @@ -150,9 +188,16 @@ const Line = (props: LineProps) => { {/** * This transparent line creates a nice big click/touch target. 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 6953d153b3..5b514f5879 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.test.tsx @@ -1,4 +1,5 @@ import {render, screen} from "@testing-library/react"; +import {userEvent as userEventLib} from "@testing-library/user-event"; import * as React from "react"; import {Dependencies} from "@khanacademy/perseus"; @@ -8,6 +9,7 @@ import {MafsGraph} from "../mafs-graph"; import {getBaseMafsGraphPropsForTests} from "../utils"; import type {InteractiveGraphState} from "../types"; +import type {UserEvent} from "@testing-library/user-event"; const baseMafsGraphProps = getBaseMafsGraphPropsForTests(); const baseLinearState: InteractiveGraphState = { @@ -27,7 +29,11 @@ const baseLinearState: InteractiveGraphState = { const overallGraphLabel = "A line on a coordinate plane."; describe("Linear graph screen reader", () => { + let userEvent: UserEvent; beforeEach(() => { + userEvent = userEventLib.setup({ + advanceTimers: jest.advanceTimersByTime, + }); jest.spyOn(Dependencies, "getDependencies").mockReturnValue( testDependencies, ); @@ -50,21 +56,36 @@ describe("Linear graph screen reader", () => { ); }); - test("should have aria labels for both points on the line", () => { + test("should have aria labels and describedbys for both points and grab handle on the line", () => { // Arrange render(); // Act - // eslint-disable-next-line testing-library/no-node-access - const points = screen.getAllByTestId("movable-point__focusable-handle"); + // Moveable elements: point 1, grab handle, point 2 + const movableElements = screen.getAllByRole("button"); + const [point1, grabHandle, point2] = movableElements; // Assert - // Check aria-label for both points on the line. - expect(points[0]).toHaveAttribute( + // Check aria-label and describedby on interactive elements. + // (The actual description text is tested separately below.) + expect(point1).toHaveAttribute("aria-label", "Point 1 at -5 comma 5"); + // We don't know the exact ID because of React.useID(), but we can + // check the suffix. + expect(point1.getAttribute("aria-describedby")).toContain("-intercept"); + expect(point1.getAttribute("aria-describedby")).toContain("-slope"); + + expect(grabHandle).toHaveAttribute( "aria-label", - "Point 1 at -5 comma 5", + "Line from -5 comma 5 to 5 comma 5.", ); - expect(points[1]).toHaveAttribute("aria-label", "Point 2 at 5 comma 5"); + expect(grabHandle.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(grabHandle.getAttribute("aria-describedby")).toContain("-slope"); + + expect(point2).toHaveAttribute("aria-label", "Point 2 at 5 comma 5"); + expect(point2.getAttribute("aria-describedby")).toContain("-intercept"); + expect(point2.getAttribute("aria-describedby")).toContain("-slope"); }); test("points description should include points info", () => { @@ -87,7 +108,7 @@ describe("Linear graph screen reader", () => { ${"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."} `( - `slope description should include slope info for $case`, + "slope description should include slope info for $case", ({coords, interceptDescription}) => { // Arrange render( @@ -115,7 +136,7 @@ describe("Linear graph screen reader", () => { ${"horizontal line"} | ${[[1, 1], [3, 1]]} | ${"Its slope is zero."} ${"vertical line"} | ${[[1, 1], [1, 3]]} | ${"Its slope is undefined."} `( - `slope description should include slope info for $case`, + "slope description should include slope info for $case", ({coords, slopeDescription}) => { // Arrange render( @@ -135,4 +156,68 @@ describe("Linear graph screen reader", () => { expect(linearGraph).toHaveTextContent(slopeDescription); }, ); + + test("aria label reflects updated values", async () => { + // Arrange + + // Act + render( + , + ); + + const interactiveElements = screen.getAllByRole("button"); + const [point1, grabHandle, point2] = interactiveElements; + + // Assert + // Check updated aria-label for the linear graph. + expect(point1).toHaveAttribute("aria-label", "Point 1 at -2 comma 3"); + expect(grabHandle).toHaveAttribute( + "aria-label", + "Line from -2 comma 3 to 3 comma 3.", + ); + expect(point2).toHaveAttribute("aria-label", "Point 2 at 3 comma 3"); + }); + + test.each` + elementName | index + ${"point1"} | ${0} + ${"grabHandle"} | ${1} + ${"point2"} | ${2} + `( + "Should update the aria-live when $elementName is moved", + async ({index}) => { + // Arrange + render( + , + ); + const interactiveElements = screen.getAllByRole("button"); + const [point1, grabHandle, point2] = interactiveElements; + const movingElement = interactiveElements[index]; + + // Act - Move the element + movingElement.focus(); + await userEvent.keyboard("{ArrowRight}"); + + const expectedAriaLive = ["off", "off", "off"]; + expectedAriaLive[index] = "polite"; + + // Assert + expect(point1).toHaveAttribute("aria-live", expectedAriaLive[0]); + expect(grabHandle).toHaveAttribute( + "aria-live", + expectedAriaLive[1], + ); + expect(point2).toHaveAttribute("aria-live", expectedAriaLive[2]); + }, + ); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx index b20f1d25c4..ccee745c4f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx @@ -43,6 +43,12 @@ const LinearGraph = (props: LinearGraphProps, key: number) => { point2X: srFormatNumber(line[1][0], locale), point2Y: srFormatNumber(line[1][1], locale), }); + const grabHandleAriaLabel = strings.srLinearGrabHandle({ + point1X: srFormatNumber(line[0][0], locale), + point1Y: srFormatNumber(line[0][1], locale), + point2X: srFormatNumber(line[1][0], locale), + point2Y: srFormatNumber(line[1][1], locale), + }); // Slope description const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); @@ -96,6 +102,7 @@ const LinearGraph = (props: LinearGraphProps, key: number) => { > { From 586c962d7ce086c605ff940ed9ececca57f9673f Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 17 Dec 2024 17:08:20 -0800 Subject: [PATCH 02/18] docs(changeset): [SR] Linear graph - add grab handle description and aria lives --- .changeset/cyan-bees-appear.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cyan-bees-appear.md diff --git a/.changeset/cyan-bees-appear.md b/.changeset/cyan-bees-appear.md new file mode 100644 index 0000000000..8b03f8ce2d --- /dev/null +++ b/.changeset/cyan-bees-appear.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +[SR] Linear graph - add grab handle description and aria lives From a5ef0f87c59c6d08850e6ccccdc4507f09b9824a Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 18 Dec 2024 13:12:24 -0800 Subject: [PATCH 03/18] Linear System SR --- packages/perseus/src/strings.ts | 76 ++++ .../graphs/linear-system.test.tsx | 330 ++++++++++++++++++ .../graphs/linear-system.tsx | 100 +++++- .../interactive-graphs/graphs/linear.tsx | 44 +-- .../interactive-graphs/graphs/utils.ts | 49 +++ .../interactive-graphs/mafs-graph.test.tsx | 8 +- 6 files changed, 559 insertions(+), 48 deletions(-) create mode 100644 packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index 2443644c8c..d97c1ad712 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -258,6 +258,44 @@ export type PerseusStrings = { tsX: string; tsY: string; }) => string; + srLinearSystemGraph: string; + srLinearSystemPoints: ({ + lineSequence, + point1X, + point1Y, + point2X, + point2Y, + }: { + lineSequence: number; + point1X: string; + point1Y: string; + point2X: string; + point2Y: string; + }) => string; + srLinearSystemPoint({ + lineSequence, + pointSequence, + x, + y, + }: { + lineSequence: number; + pointSequence: number; + x: string; + y: string; + }): string; + srLinearSystemGrabHandle: ({ + lineSequence, + point1X, + point1Y, + point2X, + point2Y, + }: { + lineSequence: number; + point1X: string; + point1Y: string; + point2X: string; + point2Y: string; + }) => string; // The above strings are used for interactive graph SR descriptions. }; @@ -550,6 +588,25 @@ export const strings: { message: "The angle measure is %(angleMeasure)s degrees with a vertex at %(vertexX)s comma %(vertexY)s, a point on the initial side at %(isX)s comma %(isY)s and a point on the terminal side at %(tsX)s comma %(tsY)s", }, + srLinearSystemGraph: "Two lines on a coordinate plane.", + srLinearSystemPoints: { + context: + "Additional information about the points for a specific line within the linear system graph.", + message: + "Line %(lineSequence)s has two points, point 1 at %(point1X)s comma %(point1Y)s and point 2 at %(point2X)s comma %(point2Y)s.", + }, + srLinearSystemPoint: { + context: + "Screenreader-accessible description of a point on a line within a linear system graph.", + message: + "Point %(pointSequence)s on line %(lineSequence)s at %(x)s comma %(y)s.", + }, + srLinearSystemGrabHandle: { + context: + "Screenreader-only label on the grab handle for a line within a linear system graph.", + message: + "Line %(lineSequence)s from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", + }, // The above strings are used for interactive graph SR descriptions. }; @@ -767,5 +824,24 @@ export const mockStrings: PerseusStrings = { tsY, }) => `The angle measure is ${angleMeasure} degrees with a vertex at ${vertexX} comma ${vertexY}, a point on the initial side at ${isX} comma ${isY} and a point on the terminal side at ${tsX} comma ${tsY}.`, + srLinearSystemGraph: "Two lines on a coordinate plane.", + srLinearSystemPoints: ({ + lineSequence, + point1X, + point1Y, + point2X, + point2Y, + }) => + `Line ${lineSequence} has two points, point 1 at ${point1X} comma ${point1Y} and point 2 at ${point2X} comma ${point2Y}.`, + srLinearSystemPoint: ({lineSequence, pointSequence, x, y}) => + `Point ${pointSequence} on line ${lineSequence} at ${x} comma ${y}.`, + srLinearSystemGrabHandle: ({ + lineSequence, + point1X, + point1Y, + point2X, + point2Y, + }) => + `Line ${lineSequence} from ${point1X} comma ${point1Y} to ${point2X} comma ${point2Y}.`, // The above strings are used for interactive graph SR descriptions. }; 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 new file mode 100644 index 0000000000..d0b2183c34 --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx @@ -0,0 +1,330 @@ +import {render, screen} from "@testing-library/react"; +import {userEvent as userEventLib} from "@testing-library/user-event"; +import * as React from "react"; + +import {Dependencies} from "@khanacademy/perseus"; + +import {testDependencies} from "../../../../../../testing/test-dependencies"; +import {MafsGraph} from "../mafs-graph"; +import {getBaseMafsGraphPropsForTests} from "../utils"; + +import type {InteractiveGraphState} from "../types"; +import type {UserEvent} from "@testing-library/user-event"; + +const baseMafsGraphProps = getBaseMafsGraphPropsForTests(); +const baseLinearSystemState: InteractiveGraphState = { + type: "linear-system", + coords: [ + [ + [-5, 5], + [5, 5], + ], + [ + [-5, -5], + [5, -5], + ], + ], + hasBeenInteractedWith: false, + range: [ + [-10, 10], + [-10, 10], + ], + snapStep: [1, 1], +}; + +const overallGraphLabel = "Two lines on a coordinate plane."; + +describe("Linear System graph screen reader", () => { + let userEvent: UserEvent; + beforeEach(() => { + userEvent = userEventLib.setup({ + advanceTimers: jest.advanceTimersByTime, + }); + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( + testDependencies, + ); + }); + + test("should have aria label and describedby for overall linear system graph", () => { + // Arrange + render( + , + ); + + // Act + const linearSystemGraph = screen.getByLabelText( + "Two lines on a coordinate plane.", + ); + + // Assert + expect(linearSystemGraph).toBeInTheDocument(); + expect(linearSystemGraph).toHaveAttribute( + "aria-describedby", + ":r1:-line1-points :r1:-line1-intercept :r1:-line1-slope :r1:-line2-points :r1:-line2-intercept :r1:-line2-slope", + ); + }); + + test("should have aria labels and describedbys for both points and grab handle on the line", () => { + // Arrange + render( + , + ); + + // Act + // Moveable elements: point 1, grab handle, point 2 + const movableElements = screen.getAllByRole("button"); + const [ + line1Point1, + line1Grab, + line1Point2, + line2Point1, + line2Grab, + line2Point2, + ] = movableElements; + + // Assert + // Check aria-label and describedby on interactive elements. + // (The actual description text is tested separately below.) + expect(line1Point1).toHaveAttribute( + "aria-label", + "Point 1 on line 1 at -5 comma 5.", + ); + // We don't know the exact ID because of React.useID(), but we can + // check the suffix. + expect(line1Point1.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(line1Point1.getAttribute("aria-describedby")).toContain( + "-slope", + ); + + expect(line1Grab).toHaveAttribute( + "aria-label", + "Line 1 from -5 comma 5 to 5 comma 5.", + ); + expect(line1Grab.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(line1Grab.getAttribute("aria-describedby")).toContain("-slope"); + + expect(line1Point2).toHaveAttribute( + "aria-label", + "Point 2 on line 1 at 5 comma 5.", + ); + expect(line1Point2.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(line1Point2.getAttribute("aria-describedby")).toContain( + "-slope", + ); + + expect(line2Point1).toHaveAttribute( + "aria-label", + "Point 1 on line 2 at -5 comma -5.", + ); + // We don't know the exact ID because of React.useID(), but we can + // check the suffix. + expect(line2Point1.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(line2Point1.getAttribute("aria-describedby")).toContain( + "-slope", + ); + + expect(line2Grab).toHaveAttribute( + "aria-label", + "Line 2 from -5 comma -5 to 5 comma -5.", + ); + expect(line2Grab.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(line2Grab.getAttribute("aria-describedby")).toContain("-slope"); + + expect(line2Point2).toHaveAttribute( + "aria-label", + "Point 2 on line 2 at 5 comma -5.", + ); + expect(line2Point2.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + expect(line2Point2.getAttribute("aria-describedby")).toContain( + "-slope", + ); + }); + + test("points description should include points info", () => { + // Arrange + render( + , + ); + + // Act + const linearSystemGraph = screen.getByLabelText(overallGraphLabel); + + // Assert + expect(linearSystemGraph).toHaveTextContent( + "Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5.", + ); + expect(linearSystemGraph).toHaveTextContent( + "Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5.", + ); + }); + + // Test each line in the linear system graph separately. + describe.each` + lineSequence + ${1} + ${2} + `(`Line $lineSequence`, ({lineSequence}) => { + 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."} + ${"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."} + `( + "slope description should include slope info for $case", + ({coords, interceptDescription}) => { + // Arrange + const newCoords = [...baseLinearSystemState.coords]; + newCoords[lineSequence - 1] = coords; + + render( + , + ); + + // Act + const linearSystemGraph = + screen.getByLabelText(overallGraphLabel); + + // Assert + expect(linearSystemGraph).toHaveTextContent( + interceptDescription, + ); + }, + ); + + test.each` + case | coords | slopeDescription + ${"positive slope"} | ${[[1, 1], [3, 3]]} | ${`Its slope increases from left to right.`} + ${"negative slope"} | ${[[3, 3], [1, 6]]} | ${`Its slope decreases from left to right.`} + ${"horizontal line"} | ${[[1, 1], [3, 1]]} | ${`Its slope is zero.`} + ${"vertical line"} | ${[[1, 1], [1, 3]]} | ${`Its slope is undefined.`} + `( + "slope description should include slope info for $case", + ({coords, slopeDescription}) => { + // Arrange + const newCoords = [...baseLinearSystemState.coords]; + newCoords[lineSequence - 1] = coords; + + render( + , + ); + + // Act + const linearSystemGraph = + screen.getByLabelText(overallGraphLabel); + + // Assert + expect(linearSystemGraph).toHaveTextContent(slopeDescription); + }, + ); + + test("aria label reflects updated values", async () => { + // Arrange + const newCoords = [...baseLinearSystemState.coords]; + newCoords[lineSequence - 1] = [ + [-2, 3], + [3, 3], + ]; + + // Act + render( + , + ); + + const interactiveElements = screen.getAllByRole("button"); + + // Get interactive elements for this line. + const point1 = interactiveElements[0 + (lineSequence - 1) * 3]; + const grabHandle = interactiveElements[1 + (lineSequence - 1) * 3]; + const point2 = interactiveElements[2 + (lineSequence - 1) * 3]; + + // Assert + // Check updated aria-label for the linear graph. + expect(point1).toHaveAttribute( + "aria-label", + `Point 1 on line ${lineSequence} at -2 comma 3.`, + ); + expect(grabHandle).toHaveAttribute( + "aria-label", + `Line ${lineSequence} from -2 comma 3 to 3 comma 3.`, + ); + expect(point2).toHaveAttribute( + "aria-label", + `Point 2 on line ${lineSequence} at 3 comma 3.`, + ); + }); + + test.each` + elementName | index + ${"point1"} | ${0} + ${"grabHandle"} | ${1} + ${"point2"} | ${2} + `( + "Should update the aria-live when $elementName is moved", + async ({index}) => { + // Arrange + render( + , + ); + const interactiveElements = screen.getAllByRole("button"); + const [point1, grabHandle, point2] = interactiveElements; + const movingElement = interactiveElements[index]; + + // Act - Move the element + movingElement.focus(); + await userEvent.keyboard("{ArrowRight}"); + + const expectedAriaLive = ["off", "off", "off"]; + expectedAriaLive[index] = "polite"; + + // Assert + expect(point1).toHaveAttribute( + "aria-live", + expectedAriaLive[0], + ); + expect(grabHandle).toHaveAttribute( + "aria-live", + expectedAriaLive[1], + ); + expect(point2).toHaveAttribute( + "aria-live", + expectedAriaLive[2], + ); + }, + ); + }); +}); 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 05ca19d704..0cd1d04c64 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -1,8 +1,11 @@ import * as React from "react"; +import {usePerseusI18n} from "../../../components/i18n-context"; import {actions} from "../reducer/interactive-graph-action"; import {MovableLine} from "./components/movable-line"; +import {srFormatNumber} from "./screenreader-text"; +import {getInterceptStringForLine, getSlopeStringForLine} from "./utils"; import type { MafsGraphProps, @@ -28,12 +31,70 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { const {dispatch} = props; const {coords: lines} = props.graphState; + const {strings, locale} = usePerseusI18n(); + const id = React.useId(); + + const linesAriaInfo = lines.map((line, i) => { + return { + pointsDescriptionId: id + `-line${i + 1}-points`, + interceptDescriptionId: id + `-line${i + 1}-intercept`, + slopeDescriptionId: id + `-line${i + 1}-slope`, + pointsDescription: strings.srLinearSystemPoints({ + lineSequence: 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), + }), + interceptDescription: getInterceptStringForLine( + line, + strings, + locale, + ), + slopeDescription: getSlopeStringForLine(line, strings), + }; + }); + return ( - <> + + `${pointsDescriptionId} ${interceptDescriptionId} ${slopeDescriptionId}`, + ) + .join(" ")} + > {lines?.map((line, i) => ( { dispatch(actions.linearSystem.moveLine(i, delta)); }} @@ -56,7 +117,40 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { color="var(--movable-line-stroke-color)" /> ))} - ; - + {linesAriaInfo.map( + ({ + pointsDescriptionId, + interceptDescriptionId, + slopeDescriptionId, + pointsDescription, + interceptDescription, + slopeDescription, + }) => ( + <> + + {pointsDescription} + + + {interceptDescription} + + + {slopeDescription} + + + ), + )} + ); }; diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx index ccee745c4f..4cc8eff7c0 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx @@ -5,6 +5,7 @@ import {actions} from "../reducer/interactive-graph-action"; import {MovableLine} from "./components/movable-line"; import {srFormatNumber} from "./screenreader-text"; +import {getInterceptStringForLine, getSlopeStringForLine} from "./utils"; import type { MafsGraphProps, @@ -49,53 +50,14 @@ const LinearGraph = (props: LinearGraphProps, key: number) => { point2X: srFormatNumber(line[1][0], locale), point2Y: srFormatNumber(line[1][1], locale), }); - - // Slope description - const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); - let slopeString = ""; - if (slope === Infinity || slope === -Infinity) { - slopeString = strings.srLinearGraphSlopeVertical; - } else if (slope === 0) { - slopeString = strings.srLinearGraphSlopeHorizontal; - } else { - slopeString = - slope > 0 - ? strings.srLinearGraphSlopeIncreasing - : strings.srLinearGraphSlopeDecreasing; - } - - // Intersection description - const xIntercept = (0 - line[0][1]) / slope + line[0][0]; - const yIntercept = line[0][1] - slope * line[0][0]; - const hasXIntercept = xIntercept !== Infinity && xIntercept !== -Infinity; - const hasYIntercept = yIntercept !== Infinity && yIntercept !== -Infinity; - let interceptString; - if (hasXIntercept && hasYIntercept) { - // Describe both intercepts in the same sentence. - interceptString = - xIntercept === 0 && yIntercept === 0 - ? strings.srLinearGraphOriginIntercept - : strings.srLinearGraphBothIntercepts({ - xIntercept: srFormatNumber(xIntercept, locale), - yIntercept: srFormatNumber(yIntercept, locale), - }); - } else { - // Describe only one intercept. - interceptString = hasXIntercept - ? strings.srLinearGraphXOnlyIntercept({ - xIntercept: srFormatNumber(xIntercept, locale), - }) - : strings.srLinearGraphYOnlyIntercept({ - yIntercept: srFormatNumber(yIntercept, locale), - }); - } + const slopeString = getSlopeStringForLine(line, strings); + const interceptString = getInterceptStringForLine(line, strings, locale); // Linear graphs only have one line // (LEMS-2050): Update the reducer so that we have a separate action for moving one line // and another action for moving multiple lines return ( ): Array { return returnArray; } + +export function getSlopeStringForLine(line: PairOfPoints, strings): string { + const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); + if (slope === Infinity || slope === -Infinity) { + return strings.srLinearGraphSlopeVertical; + } + + if (slope === 0) { + return strings.srLinearGraphSlopeHorizontal; + } + + return slope > 0 + ? strings.srLinearGraphSlopeIncreasing + : strings.srLinearGraphSlopeDecreasing; +} + +export function getInterceptStringForLine( + line: PairOfPoints, + strings, + locale, +): string { + const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); + const xIntercept = (0 - line[0][1]) / slope + line[0][0]; + const yIntercept = line[0][1] - slope * line[0][0]; + const hasXIntercept = xIntercept !== Infinity && xIntercept !== -Infinity; + const hasYIntercept = yIntercept !== Infinity && yIntercept !== -Infinity; + + if (hasXIntercept && hasYIntercept) { + // Describe both intercepts in the same sentence. + return xIntercept === 0 && yIntercept === 0 + ? strings.srLinearGraphOriginIntercept + : strings.srLinearGraphBothIntercepts({ + xIntercept: srFormatNumber(xIntercept, locale), + yIntercept: srFormatNumber(yIntercept, locale), + }); + } + + // Describe only one intercept. + return hasXIntercept + ? strings.srLinearGraphXOnlyIntercept({ + xIntercept: srFormatNumber(xIntercept, locale), + }) + : strings.srLinearGraphYOnlyIntercept({ + yIntercept: srFormatNumber(yIntercept, locale), + }); +} diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx index d9b4f0d03f..8a831af94c 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx @@ -249,10 +249,10 @@ describe("MafsGraph", () => { />, ); - expectLabelInDoc("Point 1 at 0 comma 0"); - expectLabelInDoc("Point 2 at -7 comma 0.5"); - expectLabelInDoc("Point 1 at 1 comma 1"); - expectLabelInDoc("Point 2 at 7 comma 0.5"); + expectLabelInDoc("Point 1 on line 1 at 0 comma 0."); + expectLabelInDoc("Point 2 on line 1 at -7 comma 0.5."); + expectLabelInDoc("Point 1 on line 2 at 1 comma 1."); + expectLabelInDoc("Point 2 on line 2 at 7 comma 0.5."); }); it("renders ARIA labels for each point (ray)", () => { From 0fbbbfa8e2c3002c929a9a0e4a17d3b16bc108d3 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 18 Dec 2024 14:02:07 -0800 Subject: [PATCH 04/18] docs(changeset): [SR] Linear System - add screen reader support for Linear System interactive graph --- .changeset/tidy-baboons-tie.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tidy-baboons-tie.md diff --git a/.changeset/tidy-baboons-tie.md b/.changeset/tidy-baboons-tie.md new file mode 100644 index 0000000000..20d14a409b --- /dev/null +++ b/.changeset/tidy-baboons-tie.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +[SR] Linear System - add screen reader support for Linear System interactive graph From 193ee2a3876fe727c06e80c1654633d81d4d0f1a Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 18 Dec 2024 16:48:27 -0800 Subject: [PATCH 05/18] fix misunderstanding from linear PR. update tests --- .../graphs/linear-system.test.tsx | 133 ++++-------------- .../interactive-graphs/graphs/linear.tsx | 1 - 2 files changed, 27 insertions(+), 107 deletions(-) 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 d0b2183c34..e6dfa9bc94 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 @@ -64,112 +64,6 @@ describe("Linear System graph screen reader", () => { ); }); - test("should have aria labels and describedbys for both points and grab handle on the line", () => { - // Arrange - render( - , - ); - - // Act - // Moveable elements: point 1, grab handle, point 2 - const movableElements = screen.getAllByRole("button"); - const [ - line1Point1, - line1Grab, - line1Point2, - line2Point1, - line2Grab, - line2Point2, - ] = movableElements; - - // Assert - // Check aria-label and describedby on interactive elements. - // (The actual description text is tested separately below.) - expect(line1Point1).toHaveAttribute( - "aria-label", - "Point 1 on line 1 at -5 comma 5.", - ); - // We don't know the exact ID because of React.useID(), but we can - // check the suffix. - expect(line1Point1.getAttribute("aria-describedby")).toContain( - "-intercept", - ); - expect(line1Point1.getAttribute("aria-describedby")).toContain( - "-slope", - ); - - expect(line1Grab).toHaveAttribute( - "aria-label", - "Line 1 from -5 comma 5 to 5 comma 5.", - ); - expect(line1Grab.getAttribute("aria-describedby")).toContain( - "-intercept", - ); - expect(line1Grab.getAttribute("aria-describedby")).toContain("-slope"); - - expect(line1Point2).toHaveAttribute( - "aria-label", - "Point 2 on line 1 at 5 comma 5.", - ); - expect(line1Point2.getAttribute("aria-describedby")).toContain( - "-intercept", - ); - expect(line1Point2.getAttribute("aria-describedby")).toContain( - "-slope", - ); - - expect(line2Point1).toHaveAttribute( - "aria-label", - "Point 1 on line 2 at -5 comma -5.", - ); - // We don't know the exact ID because of React.useID(), but we can - // check the suffix. - expect(line2Point1.getAttribute("aria-describedby")).toContain( - "-intercept", - ); - expect(line2Point1.getAttribute("aria-describedby")).toContain( - "-slope", - ); - - expect(line2Grab).toHaveAttribute( - "aria-label", - "Line 2 from -5 comma -5 to 5 comma -5.", - ); - expect(line2Grab.getAttribute("aria-describedby")).toContain( - "-intercept", - ); - expect(line2Grab.getAttribute("aria-describedby")).toContain("-slope"); - - expect(line2Point2).toHaveAttribute( - "aria-label", - "Point 2 on line 2 at 5 comma -5.", - ); - expect(line2Point2.getAttribute("aria-describedby")).toContain( - "-intercept", - ); - expect(line2Point2.getAttribute("aria-describedby")).toContain( - "-slope", - ); - }); - - test("points description should include points info", () => { - // Arrange - render( - , - ); - - // Act - const linearSystemGraph = screen.getByLabelText(overallGraphLabel); - - // Assert - expect(linearSystemGraph).toHaveTextContent( - "Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5.", - ); - expect(linearSystemGraph).toHaveTextContent( - "Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5.", - ); - }); - // Test each line in the linear system graph separately. describe.each` lineSequence @@ -285,6 +179,33 @@ describe("Linear System graph screen reader", () => { ); }); + test.each` + element | index + ${"point1"} | ${0} + ${"grabHandle"} | ${1} + ${"point2"} | ${2} + `("should have describedby on all interactive elements", ({index}) => { + // Arrange + render( + , + ); + + // Act + const interactiveElements = screen.getAllByRole("button"); + const element = interactiveElements[index + (lineSequence - 1) * 3]; + + // Assert + expect(element.getAttribute("aria-describedby")).toContain( + "-slope", + ); + expect(element.getAttribute("aria-describedby")).toContain( + "-intercept", + ); + }); + test.each` elementName | index ${"point1"} | ${0} diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx index e3110b2253..4cc8eff7c0 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx @@ -61,7 +61,6 @@ const LinearGraph = (props: LinearGraphProps, key: number) => { // Outer line minimal description aria-label={strings.srLinearGraph} aria-describedby={`${pointsDescriptionId} ${interceptDescriptionId} ${slopeDescriptionId}`} - role="figure" > Date: Wed, 8 Jan 2025 14:49:57 -0800 Subject: [PATCH 06/18] remove contexts from strings --- packages/perseus/src/strings.ts | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index dd593d5105..b234bb5ac8 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -517,24 +517,12 @@ export const strings: { srAngleGraphAriaDescription: "The angle measure is %(angleMeasure)s degrees with a vertex at %(vertexX)s comma %(vertexY)s, a point on the starting side at %(startingSideX)s comma %(startingSideY)s and a point on the ending side at %(endingSideX)s comma %(endingSideY)s", srLinearSystemGraph: "Two lines on a coordinate plane.", - srLinearSystemPoints: { - context: - "Additional information about the points for a specific line within the linear system graph.", - message: - "Line %(lineSequence)s has two points, point 1 at %(point1X)s comma %(point1Y)s and point 2 at %(point2X)s comma %(point2Y)s.", - }, - srLinearSystemPoint: { - context: - "Screenreader-accessible description of a point on a line within a linear system graph.", - message: - "Point %(pointSequence)s on line %(lineSequence)s at %(x)s comma %(y)s.", - }, - srLinearSystemGrabHandle: { - context: - "Screenreader-only label on the grab handle for a line within a linear system graph.", - message: - "Line %(lineSequence)s from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", - }, + srLinearSystemPoints: + "Line %(lineSequence)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 %(lineSequence)s at %(x)s comma %(y)s.", + srLinearSystemGrabHandle: + "Line %(lineSequence)s from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", // The above strings are used for interactive graph SR descriptions. }; From 7d96885d5e33daa1ddbb6e466cff07980ca55184 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 8 Jan 2025 15:44:08 -0800 Subject: [PATCH 07/18] Add full graph description of all interactive elements --- .../perseus/src/components/i18n-context.tsx | 2 +- .../graphs/linear-system.test.tsx | 47 +++++++++++++++++++ .../graphs/linear-system.tsx | 47 ++++++++++++++++++- 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/packages/perseus/src/components/i18n-context.tsx b/packages/perseus/src/components/i18n-context.tsx index 91726b214a..14a696402b 100644 --- a/packages/perseus/src/components/i18n-context.tsx +++ b/packages/perseus/src/components/i18n-context.tsx @@ -10,7 +10,7 @@ import {mockStrings} from "../strings"; import type {PerseusStrings} from "../strings"; -type I18nContextType = { +export type I18nContextType = { strings: PerseusStrings; locale: string; }; 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 e6dfa9bc94..d69fbe4a1d 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 @@ -5,9 +5,12 @@ import * as React from "react"; import {Dependencies} from "@khanacademy/perseus"; import {testDependencies} from "../../../../../../testing/test-dependencies"; +import {mockPerseusI18nContext} from "../../../components/i18n-context"; import {MafsGraph} from "../mafs-graph"; import {getBaseMafsGraphPropsForTests} from "../utils"; +import {describeLinearSystemGraph} from "./linear-system"; + import type {InteractiveGraphState} from "../types"; import type {UserEvent} from "@testing-library/user-event"; @@ -249,3 +252,47 @@ describe("Linear System graph screen reader", () => { ); }); }); + +describe(describeLinearSystemGraph, () => { + test("describes a default linear system graph", () => { + // Arrange + + // Act + const linearSystemGraphDescription = describeLinearSystemGraph( + baseLinearSystemState, + mockPerseusI18nContext, + ); + + // Assert + expect(linearSystemGraphDescription).toEqual( + "Interactive elements: Two lines on a coordinate plane. Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5. Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5.", + ); + }); + + test("describes a linear system graph with updated points", () => { + // Arrange + + // Act + const linearSystemGraphDescription = describeLinearSystemGraph( + { + ...baseLinearSystemState, + coords: [ + [ + [-2, 3], + [3, 3], + ], + [ + [-2, -3], + [3, -3], + ], + ], + }, + mockPerseusI18nContext, + ); + + // Assert + expect(linearSystemGraphDescription).toEqual( + "Interactive elements: Two lines on a coordinate plane. Line 1 has two points, point 1 at -2 comma 3 and point 2 at 3 comma 3. Line 2 has two points, point 1 at -2 comma -3 and point 2 at 3 comma -3.", + ); + }); +}); 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 0cd1d04c64..2a85c542f6 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -7,6 +7,7 @@ import {MovableLine} from "./components/movable-line"; import {srFormatNumber} from "./screenreader-text"; import {getInterceptStringForLine, getSlopeStringForLine} from "./utils"; +import type {I18nContextType} from "../../../components/i18n-context"; import type { MafsGraphProps, LinearSystemGraphState, @@ -21,7 +22,9 @@ export function renderLinearSystemGraph( ): InteractiveGraphElementSuite { return { graph: , - interactiveElementsDescription: null, + interactiveElementsDescription: ( + + ), }; } @@ -154,3 +157,45 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { ); }; + +function LinearSystemGraphDescription({ + state, +}: { + state: LinearSystemGraphState; +}) { + // The reason that LinearSystemGraphDescription is a component (rather + // than a function that returns a string) is because it needs to use a + // hook: `usePerseusI18n`. + const i18n = usePerseusI18n(); + + return describeLinearSystemGraph(state, i18n); +} + +// Exported for testing +export function describeLinearSystemGraph( + state: LinearSystemGraphState, + i18n: I18nContextType, +): string { + const {strings, locale} = i18n; + const {coords: lines} = state; + + const graphDescription = strings.srLinearSystemGraph; + + const lineDescriptions = lines.map((line, i) => { + const point1 = line[0]; + const point2 = line[1]; + return strings.srLinearSystemPoints({ + lineSequence: i + 1, + point1X: srFormatNumber(point1[0], locale), + point1Y: srFormatNumber(point1[1], locale), + point2X: srFormatNumber(point2[0], locale), + point2Y: srFormatNumber(point2[1], locale), + }); + }); + + const allDescriptions = [graphDescription, ...lineDescriptions]; + + return strings.srInteractiveElements({ + elements: allDescriptions.join(" "), + }); +} From 9ac97821b99dd1eb72468b10ae77dba3c24735f4 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Thu, 9 Jan 2025 10:37:55 -0800 Subject: [PATCH 08/18] Rename lineSequence --> lineNumber --- packages/perseus/src/strings.ts | 36 ++++++++----------- .../graphs/linear-system.test.tsx | 24 ++++++------- .../graphs/linear-system.tsx | 10 +++--- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index b234bb5ac8..a27cc36c3e 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -260,37 +260,37 @@ export type PerseusStrings = { }) => string; srLinearSystemGraph: string; srLinearSystemPoints: ({ - lineSequence, + lineNumber, point1X, point1Y, point2X, point2Y, }: { - lineSequence: number; + lineNumber: number; point1X: string; point1Y: string; point2X: string; point2Y: string; }) => string; srLinearSystemPoint({ - lineSequence, + lineNumber, pointSequence, x, y, }: { - lineSequence: number; + lineNumber: number; pointSequence: number; x: string; y: string; }): string; srLinearSystemGrabHandle: ({ - lineSequence, + lineNumber, point1X, point1Y, point2X, point2Y, }: { - lineSequence: number; + lineNumber: number; point1X: string; point1Y: string; point2X: string; @@ -518,11 +518,11 @@ export const strings: { "The angle measure is %(angleMeasure)s degrees with a vertex at %(vertexX)s comma %(vertexY)s, a point on the starting side at %(startingSideX)s comma %(startingSideY)s and a point on the ending side at %(endingSideX)s comma %(endingSideY)s", srLinearSystemGraph: "Two lines on a coordinate plane.", srLinearSystemPoints: - "Line %(lineSequence)s has two points, point 1 at %(point1X)s comma %(point1Y)s and point 2 at %(point2X)s comma %(point2Y)s.", + "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 %(lineSequence)s at %(x)s comma %(y)s.", + "Point %(pointSequence)s on line %(lineNumber)s at %(x)s comma %(y)s.", srLinearSystemGrabHandle: - "Line %(lineSequence)s from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", + "Line %(lineNumber)s from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", // The above strings are used for interactive graph SR descriptions. }; @@ -741,23 +741,17 @@ export const mockStrings: PerseusStrings = { }) => `The angle measure is ${angleMeasure} degrees with a vertex at ${vertexX} comma ${vertexY}, a point on the starting side at ${startingSideX} comma ${startingSideY} and a point on the ending side at ${endingSideX} comma ${endingSideY}.`, srLinearSystemGraph: "Two lines on a coordinate plane.", - srLinearSystemPoints: ({ - lineSequence, - point1X, - point1Y, - point2X, - point2Y, - }) => - `Line ${lineSequence} has two points, point 1 at ${point1X} comma ${point1Y} and point 2 at ${point2X} comma ${point2Y}.`, - srLinearSystemPoint: ({lineSequence, pointSequence, x, y}) => - `Point ${pointSequence} on line ${lineSequence} at ${x} comma ${y}.`, + srLinearSystemPoints: ({lineNumber, point1X, point1Y, point2X, point2Y}) => + `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: ({ - lineSequence, + lineNumber, point1X, point1Y, point2X, point2Y, }) => - `Line ${lineSequence} from ${point1X} comma ${point1Y} to ${point2X} comma ${point2Y}.`, + `Line ${lineNumber} from ${point1X} comma ${point1Y} to ${point2X} comma ${point2Y}.`, // The above strings are used for interactive graph SR descriptions. }; 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 d69fbe4a1d..31780461b8 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 @@ -69,10 +69,10 @@ describe("Linear System graph screen reader", () => { // Test each line in the linear system graph separately. describe.each` - lineSequence + lineNumber ${1} ${2} - `(`Line $lineSequence`, ({lineSequence}) => { + `(`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."} @@ -84,7 +84,7 @@ describe("Linear System graph screen reader", () => { ({coords, interceptDescription}) => { // Arrange const newCoords = [...baseLinearSystemState.coords]; - newCoords[lineSequence - 1] = coords; + newCoords[lineNumber - 1] = coords; render( { ({coords, slopeDescription}) => { // Arrange const newCoords = [...baseLinearSystemState.coords]; - newCoords[lineSequence - 1] = coords; + newCoords[lineNumber - 1] = coords; render( { test("aria label reflects updated values", async () => { // Arrange const newCoords = [...baseLinearSystemState.coords]; - newCoords[lineSequence - 1] = [ + newCoords[lineNumber - 1] = [ [-2, 3], [3, 3], ]; @@ -162,23 +162,23 @@ describe("Linear System graph screen reader", () => { const interactiveElements = screen.getAllByRole("button"); // Get interactive elements for this line. - const point1 = interactiveElements[0 + (lineSequence - 1) * 3]; - const grabHandle = interactiveElements[1 + (lineSequence - 1) * 3]; - const point2 = interactiveElements[2 + (lineSequence - 1) * 3]; + const point1 = interactiveElements[0 + (lineNumber - 1) * 3]; + const grabHandle = interactiveElements[1 + (lineNumber - 1) * 3]; + const point2 = interactiveElements[2 + (lineNumber - 1) * 3]; // Assert // Check updated aria-label for the linear graph. expect(point1).toHaveAttribute( "aria-label", - `Point 1 on line ${lineSequence} at -2 comma 3.`, + `Point 1 on line ${lineNumber} at -2 comma 3.`, ); expect(grabHandle).toHaveAttribute( "aria-label", - `Line ${lineSequence} from -2 comma 3 to 3 comma 3.`, + `Line ${lineNumber} from -2 comma 3 to 3 comma 3.`, ); expect(point2).toHaveAttribute( "aria-label", - `Point 2 on line ${lineSequence} at 3 comma 3.`, + `Point 2 on line ${lineNumber} at 3 comma 3.`, ); }); @@ -198,7 +198,7 @@ describe("Linear System graph screen reader", () => { // Act const interactiveElements = screen.getAllByRole("button"); - const element = interactiveElements[index + (lineSequence - 1) * 3]; + const element = interactiveElements[index + (lineNumber - 1) * 3]; // Assert expect(element.getAttribute("aria-describedby")).toContain( 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 2a85c542f6..4d3834641b 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -43,7 +43,7 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { interceptDescriptionId: id + `-line${i + 1}-intercept`, slopeDescriptionId: id + `-line${i + 1}-slope`, pointsDescription: strings.srLinearSystemPoints({ - lineSequence: i + 1, + lineNumber: i + 1, point1X: srFormatNumber(line[0][0], locale), point1Y: srFormatNumber(line[0][1], locale), point2X: srFormatNumber(line[1][0], locale), @@ -78,19 +78,19 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { points={line} ariaLabels={{ point1AriaLabel: strings.srLinearSystemPoint({ - lineSequence: i + 1, + lineNumber: i + 1, pointSequence: 1, x: srFormatNumber(line[0][0], locale), y: srFormatNumber(line[0][1], locale), }), point2AriaLabel: strings.srLinearSystemPoint({ - lineSequence: i + 1, + lineNumber: i + 1, pointSequence: 2, x: srFormatNumber(line[1][0], locale), y: srFormatNumber(line[1][1], locale), }), grabHandleAriaLabel: strings.srLinearSystemGrabHandle({ - lineSequence: i + 1, + lineNumber: i + 1, point1X: srFormatNumber(line[0][0], locale), point1Y: srFormatNumber(line[0][1], locale), point2X: srFormatNumber(line[1][0], locale), @@ -185,7 +185,7 @@ export function describeLinearSystemGraph( const point1 = line[0]; const point2 = line[1]; return strings.srLinearSystemPoints({ - lineSequence: i + 1, + lineNumber: i + 1, point1X: srFormatNumber(point1[0], locale), point1Y: srFormatNumber(point1[1], locale), point2X: srFormatNumber(point2[0], locale), From 5b48e40b60792fe46049668eb77ddb7eeed427f1 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Thu, 9 Jan 2025 13:01:18 -0800 Subject: [PATCH 09/18] Create custom matcher for aria descriptions --- config/test/custom-matchers.ts | 46 +++++++++++++++++++ .../graphs/linear-system.test.tsx | 19 ++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/config/test/custom-matchers.ts b/config/test/custom-matchers.ts index afddd66c24..9238620c8e 100644 --- a/config/test/custom-matchers.ts +++ b/config/test/custom-matchers.ts @@ -15,6 +15,10 @@ declare global { toHaveInvalidInput(message?: string | null): R; toHaveBeenAnsweredIncorrectly(): R; toBeHighlighted(): R; + toHaveAriaDescription( + expected: string, + options?: {repetitionNumber?: number}, + ): R; } } } @@ -151,4 +155,46 @@ expect.extend({ message: () => `Element does not appear to be a part of a widget`, }; }, + + toHaveAriaDescription( + el: HTMLElement, + expected: string, + options?: {repetitionNumber?: number}, + ) { + // Get the IDs from the element's aria-describedby attribute + const describedByIds = + el.getAttribute("aria-describedby")?.split(" ") ?? []; + + // Get the elements with the IDs + const describedByElements = describedByIds.map((id) => + document.getElementById(id), + ); + + // Get the text content of the elements + const describedByContents = describedByElements.map((el) => + el?.textContent?.trim(), + ); + + if (options?.repetitionNumber) { + // Check if the expected description shows up the same + // amount of times as the repetition number. + const count = describedByContents.filter( + (desc) => desc === expected, + ).length; + if (count === options.repetitionNumber) { + return {pass: true, message: () => ""}; + } + } + + // Check if the expected description is in the list of descriptions + if (describedByContents.includes(expected)) { + return {pass: true, message: () => ""}; + } + + return { + pass: false, + message: () => + `Element does not have the expected aria description. Expected: "${expected}". Found: "${describedByContents.join('",\n"')}"`, + }; + }, }); 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 31780461b8..97e0a63458 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 @@ -61,9 +61,22 @@ describe("Linear System graph screen reader", () => { // Assert expect(linearSystemGraph).toBeInTheDocument(); - expect(linearSystemGraph).toHaveAttribute( - "aria-describedby", - ":r1:-line1-points :r1:-line1-intercept :r1:-line1-slope :r1:-line2-points :r1:-line2-intercept :r1:-line2-slope", + expect(linearSystemGraph).toHaveAriaDescription( + "Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5.", + ); + expect(linearSystemGraph).toHaveAriaDescription( + "The line crosses the Y-axis at 0 comma 5.", + ); + expect(linearSystemGraph).toHaveAriaDescription("Its slope is zero.", { + // The slope is zero for both lines, so it shows up twice in the + // aria descriptions. + repetitionNumber: 2, + }); + expect(linearSystemGraph).toHaveAriaDescription( + "Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5.", + ); + expect(linearSystemGraph).toHaveAriaDescription( + "The line crosses the Y-axis at 0 comma -5.", ); }); From 24f71a132b53d789214d49e31f13c7298a32d927 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Thu, 9 Jan 2025 13:04:15 -0800 Subject: [PATCH 10/18] Include ID in template string --- .../src/widgets/interactive-graphs/graphs/linear-system.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 4d3834641b..6ee62a441d 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -39,9 +39,9 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { const linesAriaInfo = lines.map((line, i) => { return { - pointsDescriptionId: id + `-line${i + 1}-points`, - interceptDescriptionId: id + `-line${i + 1}-intercept`, - slopeDescriptionId: id + `-line${i + 1}-slope`, + pointsDescriptionId: `${id}-line${i + 1}-points`, + interceptDescriptionId: `${id}-line${i + 1}-intercept`, + slopeDescriptionId: `${id}-line${i + 1}-slope`, pointsDescription: strings.srLinearSystemPoints({ lineNumber: i + 1, point1X: srFormatNumber(line[0][0], locale), From 8669b75e3df0d3389c8338742ee5629ee8c04a20 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Thu, 9 Jan 2025 13:09:46 -0800 Subject: [PATCH 11/18] Use srOnly style instead of invalid display hidden --- .../src/widgets/interactive-graphs/graphs/angle.tsx | 3 ++- .../src/widgets/interactive-graphs/graphs/circle.tsx | 5 +++-- .../widgets/interactive-graphs/graphs/linear-system.tsx | 7 ++++--- .../src/widgets/interactive-graphs/graphs/linear.tsx | 7 ++++--- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx index 0ecb3f8e88..62afcaca5f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx @@ -2,6 +2,7 @@ import {vec} from "mafs"; import * as React from "react"; import {usePerseusI18n} from "../../../components/i18n-context"; +import a11y from "../../../util/a11y"; import {X, Y, calculateAngleInDegrees, getClockwiseAngle, polar} from "../math"; import {findIntersectionOfRays} from "../math/geometry"; import {actions} from "../reducer/interactive-graph-action"; @@ -215,7 +216,7 @@ function AngleGraph(props: AngleGraphProps) { } ariaLabel={initialSideAriaLabel} /> - + {wholeAngleDescription} diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx index d68849c674..7cc5dbed95 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import {useRef} from "react"; import {usePerseusI18n} from "../../../components/i18n-context"; +import a11y from "../../../util/a11y"; import {snap, X, Y} from "../math"; import {actions} from "../reducer/interactive-graph-action"; import {getRadius} from "../reducer/interactive-graph-state"; @@ -115,10 +116,10 @@ function CircleGraph(props: CircleGraphProps) { /> {/* Hidden elements to provide the descriptions for the circle and radius point's `aria-describedby` properties. */} - + {circleRadiusDescription} - + {circleOuterPointsDescription} 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 6ee62a441d..31325c54c6 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import {usePerseusI18n} from "../../../components/i18n-context"; +import a11y from "../../../util/a11y"; import {actions} from "../reducer/interactive-graph-action"; import {MovableLine} from "./components/movable-line"; @@ -133,21 +134,21 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { {pointsDescription} {interceptDescription} {slopeDescription} diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx index 4cc8eff7c0..f0b0f429df 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import {usePerseusI18n} from "../../../components/i18n-context"; +import a11y from "../../../util/a11y"; import {actions} from "../reducer/interactive-graph-action"; import {MovableLine} from "./components/movable-line"; @@ -86,13 +87,13 @@ const LinearGraph = (props: LinearGraphProps, key: number) => { /> {/* Hidden elements to provide the descriptions for the circle and radius point's `aria-describedby` properties. */} - + {linearGraphPointsDescription} - + {interceptString} - + {slopeString} From ed8ccbd8a3c01dbbad8b3f571c4ae1d49471ea2d Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Thu, 9 Jan 2025 13:16:37 -0800 Subject: [PATCH 12/18] use isFinite --- .../perseus/src/widgets/interactive-graphs/graphs/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts index 72bf8ef0c0..19febe5dd3 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts @@ -68,7 +68,7 @@ export function getArrayWithoutDuplicates(array: Array): Array { export function getSlopeStringForLine(line: PairOfPoints, strings): string { const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); - if (slope === Infinity || slope === -Infinity) { + if (!Number.isFinite(slope)) { return strings.srLinearGraphSlopeVertical; } @@ -89,8 +89,8 @@ export function getInterceptStringForLine( const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); const xIntercept = (0 - line[0][1]) / slope + line[0][0]; const yIntercept = line[0][1] - slope * line[0][0]; - const hasXIntercept = xIntercept !== Infinity && xIntercept !== -Infinity; - const hasYIntercept = yIntercept !== Infinity && yIntercept !== -Infinity; + const hasXIntercept = Number.isFinite(xIntercept); + const hasYIntercept = Number.isFinite(yIntercept); if (hasXIntercept && hasYIntercept) { // Describe both intercepts in the same sentence. From f8c8c118fa79e78196213b2a9bd320e0b8bf15d5 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 14 Jan 2025 21:55:42 -0800 Subject: [PATCH 13/18] handle SR for lines overlapping axes --- .../interactive-graphs/graphs/linear-system.test.tsx | 4 ++++ .../src/widgets/interactive-graphs/graphs/utils.ts | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) 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 97e0a63458..24d8214b4d 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 @@ -92,6 +92,8 @@ describe("Linear System graph screen reader", () => { ${"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."} + ${"overlaps x-axis"} | ${[[5, 0], [2, 0]]} | ${"The line crosses the Y-axis at 0 comma 0."} `( "slope description should include slope info for $case", ({coords, interceptDescription}) => { @@ -126,6 +128,8 @@ describe("Linear System graph screen reader", () => { ${"negative slope"} | ${[[3, 3], [1, 6]]} | ${`Its slope decreases from left to right.`} ${"horizontal line"} | ${[[1, 1], [3, 1]]} | ${`Its slope is zero.`} ${"vertical line"} | ${[[1, 1], [1, 3]]} | ${`Its slope is undefined.`} + ${"overlaps x-axis"} | ${[[1, 0], [3, 0]]} | ${`Its slope is zero.`} + ${"overlaps y-axis"} | ${[[0, 1], [0, 3]]} | ${`Its slope is undefined.`} `( "slope description should include slope info for $case", ({coords, slopeDescription}) => { diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts index 19febe5dd3..508de156af 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts @@ -89,8 +89,13 @@ export function getInterceptStringForLine( const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); const xIntercept = (0 - line[0][1]) / slope + line[0][0]; const yIntercept = line[0][1] - slope * line[0][0]; - const hasXIntercept = Number.isFinite(xIntercept); - const hasYIntercept = Number.isFinite(yIntercept); + + // Check if the line fully overlaps with an axis. + const overlapsXAxis = line[0][1] === 0 && line[1][1] === 0; + const overlapsYAxis = line[0][0] === 0 && line[1][0] === 0; + + const hasXIntercept = Number.isFinite(xIntercept) && !overlapsXAxis; + const hasYIntercept = Number.isFinite(yIntercept) && !overlapsYAxis; if (hasXIntercept && hasYIntercept) { // Describe both intercepts in the same sentence. From 1dcf3fd37e1f563584525eab24c863eadfd9856c Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 14 Jan 2025 22:40:06 -0800 Subject: [PATCH 14/18] Use existing aria description matcher rather than creating a new one --- config/test/custom-matchers.ts | 46 ------------------- .../graphs/linear-system.test.tsx | 18 +------- 2 files changed, 2 insertions(+), 62 deletions(-) diff --git a/config/test/custom-matchers.ts b/config/test/custom-matchers.ts index 9238620c8e..afddd66c24 100644 --- a/config/test/custom-matchers.ts +++ b/config/test/custom-matchers.ts @@ -15,10 +15,6 @@ declare global { toHaveInvalidInput(message?: string | null): R; toHaveBeenAnsweredIncorrectly(): R; toBeHighlighted(): R; - toHaveAriaDescription( - expected: string, - options?: {repetitionNumber?: number}, - ): R; } } } @@ -155,46 +151,4 @@ expect.extend({ message: () => `Element does not appear to be a part of a widget`, }; }, - - toHaveAriaDescription( - el: HTMLElement, - expected: string, - options?: {repetitionNumber?: number}, - ) { - // Get the IDs from the element's aria-describedby attribute - const describedByIds = - el.getAttribute("aria-describedby")?.split(" ") ?? []; - - // Get the elements with the IDs - const describedByElements = describedByIds.map((id) => - document.getElementById(id), - ); - - // Get the text content of the elements - const describedByContents = describedByElements.map((el) => - el?.textContent?.trim(), - ); - - if (options?.repetitionNumber) { - // Check if the expected description shows up the same - // amount of times as the repetition number. - const count = describedByContents.filter( - (desc) => desc === expected, - ).length; - if (count === options.repetitionNumber) { - return {pass: true, message: () => ""}; - } - } - - // Check if the expected description is in the list of descriptions - if (describedByContents.includes(expected)) { - return {pass: true, message: () => ""}; - } - - return { - pass: false, - message: () => - `Element does not have the expected aria description. Expected: "${expected}". Found: "${describedByContents.join('",\n"')}"`, - }; - }, }); 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 24d8214b4d..c54f7cae82 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 @@ -61,22 +61,8 @@ describe("Linear System graph screen reader", () => { // Assert expect(linearSystemGraph).toBeInTheDocument(); - expect(linearSystemGraph).toHaveAriaDescription( - "Line 1 has two points, point 1 at -5 comma 5 and point 2 at 5 comma 5.", - ); - expect(linearSystemGraph).toHaveAriaDescription( - "The line crosses the Y-axis at 0 comma 5.", - ); - expect(linearSystemGraph).toHaveAriaDescription("Its slope is zero.", { - // The slope is zero for both lines, so it shows up twice in the - // aria descriptions. - repetitionNumber: 2, - }); - expect(linearSystemGraph).toHaveAriaDescription( - "Line 2 has two points, point 1 at -5 comma -5 and point 2 at 5 comma -5.", - ); - expect(linearSystemGraph).toHaveAriaDescription( - "The line crosses the Y-axis at 0 comma -5.", + expect(linearSystemGraph).toHaveAccessibleDescription( + "Line 1 has two points, ponit 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.", ); }); From d64ac0d70dd9136f1f80faddcd87c190bbcacdae Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 14 Jan 2025 22:41:56 -0800 Subject: [PATCH 15/18] correct string in test --- .../widgets/interactive-graphs/graphs/linear-system.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c54f7cae82..2bfe75efed 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, ponit 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.", + "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.", ); }); From 4468c3e867170eb30535d3105930dad49cc617be Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 15 Jan 2025 13:15:23 -0800 Subject: [PATCH 16/18] Add types --- .../src/widgets/interactive-graphs/graphs/utils.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts index 508de156af..9efe55b13a 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts @@ -2,6 +2,7 @@ import {srFormatNumber} from "./screenreader-text"; import type {PairOfPoints} from "../types"; import type {Coord} from "@khanacademy/perseus"; +import type {PerseusStrings} from "@khanacademy/perseus/strings"; import type {Interval, vec} from "mafs"; /** @@ -66,7 +67,10 @@ export function getArrayWithoutDuplicates(array: Array): Array { return returnArray; } -export function getSlopeStringForLine(line: PairOfPoints, strings): string { +export function getSlopeStringForLine( + line: PairOfPoints, + strings: PerseusStrings, +): string { const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); if (!Number.isFinite(slope)) { return strings.srLinearGraphSlopeVertical; @@ -83,8 +87,8 @@ export function getSlopeStringForLine(line: PairOfPoints, strings): string { export function getInterceptStringForLine( line: PairOfPoints, - strings, - locale, + strings: PerseusStrings, + locale: string, ): string { const slope = (line[1][1] - line[0][1]) / (line[1][0] - line[0][0]); const xIntercept = (0 - line[0][1]) / slope + line[0][0]; From e4a7322680a5676a1c77cef01a0b2b32cb7cec27 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 15 Jan 2025 13:21:25 -0800 Subject: [PATCH 17/18] fix import --- packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts index 9efe55b13a..c023e5d7f6 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts @@ -1,8 +1,8 @@ import {srFormatNumber} from "./screenreader-text"; +import type {PerseusStrings} from "../../../strings"; import type {PairOfPoints} from "../types"; import type {Coord} from "@khanacademy/perseus"; -import type {PerseusStrings} from "@khanacademy/perseus/strings"; import type {Interval, vec} from "mafs"; /** From 11f2a4aa0d1571630f807647c38f7d39c4f77b5d Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 15 Jan 2025 13:29:08 -0800 Subject: [PATCH 18/18] Update grab handle description to match SRUX doc --- packages/perseus/src/strings.ts | 23 ------------------- .../graphs/linear-system.test.tsx | 2 +- .../graphs/linear-system.tsx | 8 +------ 3 files changed, 2 insertions(+), 31 deletions(-) diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index a27cc36c3e..0723bdec92 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -283,19 +283,6 @@ 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; // The above strings are used for interactive graph SR descriptions. }; @@ -521,8 +508,6 @@ 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 from %(point1X)s comma %(point1Y)s to %(point2X)s comma %(point2Y)s.", // The above strings are used for interactive graph SR descriptions. }; @@ -745,13 +730,5 @@ 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} from ${point1X} comma ${point1Y} to ${point2X} comma ${point2Y}.`, // The above strings are used for interactive graph SR descriptions. }; 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 2bfe75efed..2bda019d36 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 @@ -177,7 +177,7 @@ describe("Linear System graph screen reader", () => { ); expect(grabHandle).toHaveAttribute( "aria-label", - `Line ${lineNumber} from -2 comma 3 to 3 comma 3.`, + `The line crosses the Y-axis at 0 comma 3. Its slope is zero.`, ); expect(point2).toHaveAttribute( "aria-label", 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 31325c54c6..d6a38cf885 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.tsx @@ -90,13 +90,7 @@ const LinearSystemGraph = (props: LinearSystemGraphProps) => { x: srFormatNumber(line[1][0], locale), y: srFormatNumber(line[1][1], locale), }), - 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), - }), + grabHandleAriaLabel: `${linesAriaInfo[i].interceptDescription} ${linesAriaInfo[i].slopeDescription}`, }} ariaDescribedBy={`${linesAriaInfo[i].interceptDescriptionId} ${linesAriaInfo[i].slopeDescriptionId}`} onMoveLine={(delta: vec.Vector2) => {