-
Notifications
You must be signed in to change notification settings - Fork 350
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
[SR] Add aria labels to interactive Circle graph #1928
Changes from 10 commits
8bf74c3
9fcc4cd
19a34b2
9c1d13b
4897d19
60e2a07
8282a65
e57d865
bb4a5c3
3d824ca
199146b
193facf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@khanacademy/perseus": patch | ||
--- | ||
|
||
[SR] Add screenreader support for circle graph |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import {render, screen} from "@testing-library/react"; | ||
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"; | ||
|
||
const baseMafsGraphProps = getBaseMafsGraphPropsForTests(); | ||
const baseCircleState: InteractiveGraphState = { | ||
type: "circle", | ||
center: [0, 0], | ||
radiusPoint: [1, 0], | ||
hasBeenInteractedWith: true, | ||
range: [ | ||
[-10, 10], | ||
[-10, 10], | ||
], | ||
snapStep: [1, 1], | ||
}; | ||
|
||
describe("Circle graph screen reader", () => { | ||
beforeEach(() => { | ||
jest.spyOn(Dependencies, "getDependencies").mockReturnValue( | ||
testDependencies, | ||
); | ||
}); | ||
|
||
test("should have aria label for circle graph", () => { | ||
// Arrange | ||
render(<MafsGraph {...baseMafsGraphProps} state={baseCircleState} />); | ||
|
||
// Act | ||
// eslint-disable-next-line testing-library/no-node-access | ||
const circleGraph = document.querySelector(".movable-circle"); | ||
const radiusPoint = screen.getByTestId( | ||
"movable-point__focusable-handle", | ||
); | ||
|
||
// Assert | ||
// Check aria-label, aria-describedby, and aria-live | ||
// for circle and radius point. | ||
expect(circleGraph).toHaveAttribute( | ||
"aria-label", | ||
"Circle. The center point is at 0 comma 0.", | ||
); | ||
expect(circleGraph).toHaveAttribute( | ||
"aria-describedby", | ||
// IDs for the radius and outer points hidden description elements | ||
":r1:-radius :r1:-outer-points", | ||
); | ||
expect(circleGraph).toHaveAttribute("aria-live", "polite"); | ||
|
||
expect(radiusPoint).toHaveAttribute( | ||
"aria-label", | ||
"Radius point at 1 comma 0. Circle radius is 1.", | ||
); | ||
expect(radiusPoint).toHaveAttribute( | ||
"aria-describedby", | ||
// ID for the outer points hidden description elements | ||
":r1:-outer-points", | ||
); | ||
// Radius point's aria-live is off by default so that it doesn't | ||
// override the circle's aria-live when the circle is moved (since | ||
// the radius point also moves when the circle moves). | ||
expect(radiusPoint).toHaveAttribute("aria-live", "off"); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import {vec} from "mafs"; | |
import * as React from "react"; | ||
import {useRef} from "react"; | ||
|
||
import {usePerseusI18n} from "../../../components/i18n-context"; | ||
import {snap, X, Y} from "../math"; | ||
import {actions} from "../reducer/interactive-graph-action"; | ||
import {getRadius} from "../reducer/interactive-graph-state"; | ||
|
@@ -37,31 +38,101 @@ function CircleGraph(props: CircleGraphProps) { | |
const {dispatch, graphState} = props; | ||
const {center, radiusPoint} = graphState; | ||
|
||
const {strings} = usePerseusI18n(); | ||
const [radiusPointAriaLive, setRadiusPointAriaLive] = React.useState< | ||
"off" | "polite" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd recommend using the aria live type in |
||
>("off"); | ||
|
||
const radius = getRadius(graphState); | ||
const id = React.useId(); | ||
const circleId = id + "-circle"; | ||
const radiusId = id + "-radius"; | ||
const outerPointsId = id + "-outer-points"; | ||
|
||
// Aria label strings | ||
const circleGraphAriaLabel = strings.circleGraphAriaLabel; | ||
const circleShapeAriaLabel = strings.circleShapeAriaLabel({ | ||
centerX: center[0], | ||
centerY: center[1], | ||
}); | ||
const circleRadiusPointAriaLabel = strings.circleRadiusPointAriaLabel({ | ||
radiusPointX: radiusPoint[0], | ||
radiusPointY: radiusPoint[1], | ||
}); | ||
const circleRadiusDescription = strings.circleRadiusDescription({ | ||
radius, | ||
}); | ||
const circleOuterPointsDescription = strings.circleOuterPointsDescription({ | ||
point1X: center[0] + radius, | ||
point1Y: center[1], | ||
point2X: center[0], | ||
point2Y: center[1] + radius, | ||
point3X: center[0] - radius, | ||
point3Y: center[1], | ||
point4X: center[0], | ||
point4Y: center[1] - radius, | ||
}); | ||
|
||
return ( | ||
<> | ||
<g | ||
// Outer circle minimal description | ||
aria-label={circleGraphAriaLabel} | ||
aria-describedby={`${circleId} ${radiusId} ${outerPointsId}`} | ||
> | ||
<MovableCircle | ||
id={circleId} | ||
// Focusable circle aria label reads with every update | ||
// because of the aria-live property in the circle <g>. | ||
ariaLabel={circleShapeAriaLabel} | ||
// Aria-describedby describes additional info on focus. | ||
ariaDescribedBy={`${radiusId} ${outerPointsId}`} | ||
center={center} | ||
radius={getRadius(graphState)} | ||
onMove={(c) => dispatch(actions.circle.moveCenter(c))} | ||
radius={radius} | ||
onMove={(c) => { | ||
setRadiusPointAriaLive("off"); | ||
dispatch(actions.circle.moveCenter(c)); | ||
}} | ||
/> | ||
<MovablePoint | ||
// Radius point aria label reads with every update. | ||
ariaLabel={`${circleRadiusPointAriaLabel} ${circleRadiusDescription}`} | ||
// Aria-describedby describes additional info on focus. | ||
ariaDescribedBy={`${outerPointsId}`} | ||
// The radius point's aria-live property is set to "off" when | ||
// the circle is moved, so that it doesn't override the circle's | ||
// aria-live (since the point is moved along with the circle). | ||
// When the radius point is moved, the aria-live is set to | ||
// "polite" so that the radius is read out. | ||
ariaLive={radiusPointAriaLive} | ||
point={radiusPoint} | ||
sequenceNumber={1} | ||
cursor="ew-resize" | ||
onMove={(newRadiusPoint) => { | ||
setRadiusPointAriaLive("polite"); | ||
dispatch(actions.circle.moveRadiusPoint(newRadiusPoint)); | ||
}} | ||
/> | ||
</> | ||
{/* Hidden elements to provide the descriptions for the | ||
circle and radius point's `aria-describedby` properties. */} | ||
<g id={radiusId} style={{display: "hidden"}}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I remember correctly, using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used |
||
{circleRadiusDescription} | ||
</g> | ||
<g id={outerPointsId} style={{display: "hidden"}}> | ||
{circleOuterPointsDescription} | ||
</g> | ||
</g> | ||
); | ||
} | ||
|
||
function MovableCircle(props: { | ||
id?: string; | ||
ariaLabel?: string; | ||
ariaDescribedBy?: string; | ||
center: vec.Vector2; | ||
radius: number; | ||
onMove: (newCenter: vec.Vector2) => unknown; | ||
}) { | ||
const {center, radius, onMove} = props; | ||
const {id, ariaLabel, ariaDescribedBy, center, radius, onMove} = props; | ||
const {snapStep, disableKeyboardInteraction} = useGraphConfig(); | ||
|
||
const draggableRef = useRef<SVGGElement>(null); | ||
|
@@ -78,7 +149,11 @@ function MovableCircle(props: { | |
|
||
return ( | ||
<g | ||
aria-label={ariaLabel} | ||
aria-describedby={ariaDescribedBy} | ||
aria-live="polite" | ||
ref={draggableRef} | ||
role="button" | ||
tabIndex={disableKeyboardInteraction ? -1 : 0} | ||
className={`movable-circle ${dragging ? "movable-circle--dragging" : ""}`} | ||
> | ||
|
@@ -90,6 +165,7 @@ function MovableCircle(props: { | |
ry={radiiPx[Y] + 3} | ||
/> | ||
<ellipse | ||
id={id} | ||
className="circle" | ||
cx={centerPx[X]} | ||
cy={centerPx[Y]} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I don't think I can test the changes to the aria properties on update with jest. I don't even think I can test that the radius point's aria live value state updates when the circle is moved.
Someone please tell me if I'm wrong. I'm open to suggestions for how to test this stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to use a command (can't remember it off the top of my head) where you fire
onMove
ormove
event and then you can useexpect(aria-live value in element to be "polite")
.It's worth a 15 min time box to see if this can be done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This article might have what you're looking for, the tool is called
fireEvent
:https://vinayak-hegde.medium.com/js-react-and-customevent-testing-using-jest-and-react-testing-library-abb247fd758a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests!