Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(LEMS-1733): adds aria labels to line segment #2032

Merged
merged 25 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
776c38b
docs(changeset): adds aria labels to line segment
anakaren-rojas Dec 18, 2024
bf27d16
add aria labels to line segment
anakaren-rojas Dec 18, 2024
76391ec
use line segment label
anakaren-rojas Dec 18, 2024
25b8121
Merge branch 'main' into LEMS-1733-add-aria-label-to-line-segment
anakaren-rojas Dec 19, 2024
de68801
remove role from segment
anakaren-rojas Dec 19, 2024
0616ef8
Merge branch 'main' into LEMS-1733-add-aria-label-to-line-segment
anakaren-rojas Dec 19, 2024
08fd6bf
add description for multiple segments
anakaren-rojas Dec 20, 2024
a4f033a
update strings
anakaren-rojas Dec 20, 2024
67b3bd7
add periods at the end of sentences
anakaren-rojas Dec 23, 2024
06114d8
update strings to include multiples and add desc for whole graph
anakaren-rojas Dec 23, 2024
1f6b8b4
move function outside of component
anakaren-rojas Dec 27, 2024
822097d
add unique id to the descriptions
anakaren-rojas Dec 27, 2024
4981fdc
remove unique id from aria described by key
anakaren-rojas Dec 27, 2024
bacaad4
add missing id
anakaren-rojas Dec 27, 2024
717bd34
readd key
anakaren-rojas Dec 27, 2024
42d5288
remove context and message objects
anakaren-rojas Jan 7, 2025
ec9d977
Merge branch 'main' into LEMS-1733-add-aria-label-to-line-segment
anakaren-rojas Jan 7, 2025
072225a
lint fix
anakaren-rojas Jan 7, 2025
1206856
Merge branch 'main' into LEMS-1733-add-aria-label-to-line-segment
anakaren-rojas Jan 8, 2025
d070c09
add placeholder for grab handle aria label
anakaren-rojas Jan 15, 2025
9221928
Merge branch 'main' into LEMS-1733-add-aria-label-to-line-segment
nishasy Jan 21, 2025
2adacae
Remove unused string. Make the description the label.
nishasy Jan 21, 2025
97a9970
Add grab handle string
nishasy Jan 21, 2025
4235cd7
Separate length into description
nishasy Jan 21, 2025
30061d3
Describe interactive elements in outer graph
nishasy Jan 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fuzzy-cheetahs-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

adds aria labels to line segment
74 changes: 74 additions & 0 deletions packages/perseus/src/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,48 @@ export type PerseusStrings = {
endingSideX: string;
endingSideY: string;
}) => string;
srSingleSegmentGraphAriaLabel: string;
srMultipleSegmentGraphAriaLabel: ({
countOfSegments,
}: {
countOfSegments: number;
}) => string;
srIndividualSegmentAriaLabel: string;
srIndividualSegmentAriaDescription: ({
point1X,
point1Y,
point2X,
point2Y,
length,
indexOfSegment,
}: {
point1X: string;
point1Y: string;
point2X: string;
point2Y: string;
length: string;
indexOfSegment: number;
}) => string;
srSingleSegmentGraphEndpointAriaLabel: ({
endpointNumber,
x,
y,
}: {
endpointNumber: number;
x: string;
y: string;
}) => string;
srMultipleSegmentGraphEndpointAriaLabel: ({
endpointNumber,
x,
y,
indexOfSegment,
}: {
endpointNumber: number;
x: string;
y: string;
indexOfSegment: number;
}) => string;
// The above strings are used for interactive graph SR descriptions.
};

Expand Down Expand Up @@ -478,6 +520,16 @@ export const strings: {
srAngleGraphAriaLabel: "An angle on a coordinate plane.",
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",
srSingleSegmentGraphAriaLabel: "A line segment on a coordinate plane.",
srMultipleSegmentGraphAriaLabel:
"%(countOfSegments)s segments on a coordinate plane.",
srIndividualSegmentAriaLabel: "PLACEHOLDER: PLEASE UPDATE ME",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nishasy FYI - I added a placeholder for inner aria label, which should be updated when the design copy is finalized

srIndividualSegmentAriaDescription:
"Segment %(indexOfSegment)s: Endpoint 1 at %(point1X)s comma %(point1Y)s. Endpoint 2 %(point2X)s comma %(point2Y)s. Segment length %(length)s units.",
srSingleSegmentGraphEndpointAriaLabel:
"Endpoint %(endpointNumber)s at %(x)s comma %(y)s.",
srMultipleSegmentGraphEndpointAriaLabel:
"Endpoint %(endpointNumber)s on segment %(indexOfSegment)s at %(x)s comma %(y)s.",
// The above strings are used for interactive graph SR descriptions.
};

Expand Down Expand Up @@ -695,5 +747,27 @@ export const mockStrings: PerseusStrings = {
endingSideY,
}) =>
`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}.`,
srSingleSegmentGraphAriaLabel: "A line segment on a coordinate plane.",
srMultipleSegmentGraphAriaLabel: ({countOfSegments}) =>
`${countOfSegments} segments on a coordinate plane.`,
srIndividualSegmentAriaLabel: "PLACEHOLDER: PLEASE UPDATE ME",
srIndividualSegmentAriaDescription: ({
point1X,
point1Y,
point2X,
point2Y,
length,
indexOfSegment,
}) =>
`Segment ${indexOfSegment}: Endpoint 1 at ${point1X} comma ${point1Y}. Endpoint 2 at ${point2X} comma ${point2Y}. Segment length ${length} units.`,
srSingleSegmentGraphEndpointAriaLabel: ({endpointNumber, x, y}) =>
`Endpoint ${endpointNumber} at ${x} comma ${y}.`,
srMultipleSegmentGraphEndpointAriaLabel: ({
endpointNumber,
x,
y,
indexOfSegment,
}) =>
`Endpoint ${endpointNumber} on segment ${indexOfSegment} at ${x} comma ${y}.`,
// The above strings are used for interactive graph SR descriptions.
};
147 changes: 124 additions & 23 deletions packages/perseus/src/widgets/interactive-graphs/graphs/segment.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import {point as kpoint} from "@khanacademy/kmath";
import * as React from "react";

import {usePerseusI18n} from "../../../components/i18n-context";
import {X, Y} from "../math";
import {actions} from "../reducer/interactive-graph-action";

import {MovableLine} from "./components/movable-line";
import {srFormatNumber} from "./screenreader-text";

import type {
Dispatch,
InteractiveGraphElementSuite,
MafsGraphProps,
PairOfPoints,
SegmentGraphState,
} from "../types";
import type {vec} from "mafs";
Expand All @@ -24,33 +29,129 @@ export function renderSegmentGraph(

type SegmentProps = MafsGraphProps<SegmentGraphState>;

const SegmentGraph = (props: SegmentProps) => {
const {dispatch} = props;
const {coords: segments} = props.graphState;
const SegmentGraph = ({dispatch, graphState}: SegmentProps) => {
const {coords: segments} = graphState;
const {strings, locale} = usePerseusI18n();
const segmentUniqueId = React.useId();
function getWholeSegmentGraphAriaLabel(): string {
return segments?.length > 1
? strings.srMultipleSegmentGraphAriaLabel({
countOfSegments: segments.length,
})
: strings.srSingleSegmentGraphAriaLabel;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (Optional): Since this component is not using stuff inside the component (everything it needs is passed into the parameters), you can move it outside the component, maybe at the top of this file. That way it won't re-initialize on every render.

This won't really make a difference in performance in this particular case, so it's really up to you, but I believe that's considered good practice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I meant to write "Since this function is not using stuff inside the component"


const wholeSegmentGraphAriaLabel = getWholeSegmentGraphAriaLabel();
// STRING FOR THIS SHOULD BE UPDATED PRIOR TO BEING TRANSLATED
const individualSegmentAriaLabel = strings.srIndividualSegmentAriaLabel;

function getIndividualSegmentAriaDescription(
segment: PairOfPoints,
index: number,
) {
return strings.srIndividualSegmentAriaDescription({
point1X: srFormatNumber(segment[0][X], locale),
point1Y: srFormatNumber(segment[0][Y], locale),
point2X: srFormatNumber(segment[1][X], locale),
point2Y: srFormatNumber(segment[1][Y], locale),
length: srFormatNumber(getLengthOfSegment(segment), locale),
indexOfSegment: index + 1,
});
anakaren-rojas marked this conversation as resolved.
Show resolved Hide resolved
}

function getWholeSegmentGraphAriaDescription() {
let description = `${wholeSegmentGraphAriaLabel} `;

segments.forEach((segment, index) => {
description +=
getIndividualSegmentAriaDescription(segment, index) + " ";
});

return description;
}

function formatSegment(
endpointNumber: number,
x: number,
y: number,
index: number,
) {
const segObj = {
endpointNumber: endpointNumber,
x: srFormatNumber(x, locale),
y: srFormatNumber(y, locale),
};

return segments.length > 1
? strings.srMultipleSegmentGraphEndpointAriaLabel({
...segObj,
indexOfSegment: index,
})
: strings.srSingleSegmentGraphEndpointAriaLabel(segObj);
}

return (
<>
<g
aria-label={wholeSegmentGraphAriaLabel}
aria-describedby={`wholeSegmentGraphAriaDescription-${segmentUniqueId}`}
>
{segments?.map((segment, i) => (
<MovableLine
key={i}
points={segment}
onMoveLine={(delta: vec.Vector2) => {
dispatch(actions.segment.moveLine(i, delta));
}}
onMovePoint={(
endpointIndex: number,
destination: vec.Vector2,
) => {
dispatch(
actions.segment.movePointInFigure(
i,
endpointIndex,
destination,
<g
aria-label={individualSegmentAriaLabel}
aria-describedby={`segment-description-${segmentUniqueId}-${i}`}
key={`${segmentUniqueId}-${i}`}
>
<MovableLine
key={i}
points={segment}
onMoveLine={(delta: vec.Vector2) => {
dispatch(actions.segment.moveLine(i, delta));
}}
onMovePoint={(
endpointIndex: number,
destination: vec.Vector2,
) => {
dispatch(
actions.segment.movePointInFigure(
i,
endpointIndex,
destination,
),
);
}}
ariaLabels={{
point1AriaLabel: formatSegment(
1,
segment[0][X],
segment[0][Y],
i + 1,
),
point2AriaLabel: formatSegment(
2,
segment[1][X],
segment[1][Y],
i + 1,
),
);
}}
/>
}}
/>
<g
id={`segment-description-${segmentUniqueId}-${i}`}
style={{display: "hidden"}}
>
{getIndividualSegmentAriaDescription(segment, i)}
</g>
</g>
))}
</>
<g
style={{display: "hidden"}}
id={`wholeSegmentGraphAriaDescription-${segmentUniqueId}`}
>
{getWholeSegmentGraphAriaDescription()}
</g>
</g>
);
};

function getLengthOfSegment(segment: PairOfPoints) {
return kpoint.distanceToPoint(...segment);
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ describe("MafsGraph", () => {
/>,
);

expectLabelInDoc("Point 1 at 0 comma 0");
expectLabelInDoc("Point 2 at -7 comma 0.5");
expectLabelInDoc("Endpoint 1 at 0 comma 0.");
expectLabelInDoc("Endpoint 2 at -7 comma 0.5.");
});

it("renders ARIA labels for each point (multiple segments)", () => {
Expand Down Expand Up @@ -187,10 +187,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("Endpoint 1 on segment 1 at 0 comma 0.");
expectLabelInDoc("Endpoint 2 on segment 1 at -7 comma 0.5.");
expectLabelInDoc("Endpoint 1 on segment 2 at 1 comma 1.");
expectLabelInDoc("Endpoint 2 on segment 2 at 7 comma 0.5.");
});

it("renders ARIA labels for each point (linear)", () => {
Expand Down
Loading