Skip to content

Commit

Permalink
feat(LEMS-2664): update use control point and movable point with opti…
Browse files Browse the repository at this point in the history
…onal props (#1906)

## Summary:
- Updates _MovablePoint_ component and _useControlPoint_ method to include optional props for `aria label`, `aria described by`, `aria live`, and `sequence number`
- Sets default for aria live to `polite`; based on design doc, only the removal of a point or trying to move a point of out bounds warrants the use of `assertive`
- Set default for sequence number - if aria label and sequence number are not provided - sequence will default to 1 
- Creates logic for optional aria label - custom aria label will take precedence over sequence number

Issue: LEMS-2664

## Test plan:
- Using [chromatic](https://650db21c3f5d1b2f13c02952-jhskbowqxd.chromatic.com/?path=/docs/perseuseditor-editorpage--docs) link, selecting an interactive graph point reads out the point's coordinates 

## Screen recording

https://github.com/user-attachments/assets/f2414756-b0bf-48bf-98ed-08f30c41d1a1

Author: anakaren-rojas

Reviewers: anakaren-rojas, nishasy, #perseus, benchristel, catandthemachines

Required Reviewers:

Approved By: nishasy

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

Pull Request URL: #1906
  • Loading branch information
anakaren-rojas authored Nov 26, 2024
1 parent 7f2866c commit 799ffe4
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 28 deletions.
5 changes: 5 additions & 0 deletions .changeset/lazy-geckos-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

update moveable point component and use control point method to have optional params
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ exports[`Rendering Does NOT render extensions of line when option is disabled 1`
>
<g
aria-label="Point 1 at -1 comma -1"
aria-live="assertive"
aria-live="polite"
class="movable-point__focusable-handle"
data-testid="movable-point__focusable-handle"
role="button"
Expand Down Expand Up @@ -61,7 +61,7 @@ exports[`Rendering Does NOT render extensions of line when option is disabled 1`
</g>
<g
aria-label="Point 2 at 1 comma 1"
aria-live="assertive"
aria-live="polite"
class="movable-point__focusable-handle"
data-testid="movable-point__focusable-handle"
role="button"
Expand Down Expand Up @@ -156,7 +156,7 @@ exports[`Rendering Does NOT render extensions of line when option is not provide
>
<g
aria-label="Point 1 at -1 comma -1"
aria-live="assertive"
aria-live="polite"
class="movable-point__focusable-handle"
data-testid="movable-point__focusable-handle"
role="button"
Expand Down Expand Up @@ -201,7 +201,7 @@ exports[`Rendering Does NOT render extensions of line when option is not provide
</g>
<g
aria-label="Point 2 at 1 comma 1"
aria-live="assertive"
aria-live="polite"
class="movable-point__focusable-handle"
data-testid="movable-point__focusable-handle"
role="button"
Expand Down Expand Up @@ -296,7 +296,7 @@ exports[`Rendering Does render extensions of line when option is enabled 1`] = `
>
<g
aria-label="Point 1 at -1 comma -1"
aria-live="assertive"
aria-live="polite"
class="movable-point__focusable-handle"
data-testid="movable-point__focusable-handle"
role="button"
Expand Down Expand Up @@ -395,7 +395,7 @@ exports[`Rendering Does render extensions of line when option is enabled 1`] = `
</g>
<g
aria-label="Point 2 at 1 comma 1"
aria-live="assertive"
aria-live="polite"
class="movable-point__focusable-handle"
data-testid="movable-point__focusable-handle"
role="button"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,112 @@ describe("MovablePoint", () => {

expect(focusSpy).toHaveBeenCalledTimes(1);
});

describe("accessibility", () => {
it("uses the default sequence number when ariaLabel and sequence number are not provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint point={[0, 0]} />
</Mafs>,
);

expect(
screen.getByLabelText("Point 1 at 0 comma 0"),
).toBeInTheDocument();
});

it("uses sequence number when sequence is provided and aria label is not provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint point={[0, 0]} sequenceNumber={2} />
</Mafs>,
);

expect(
screen.getByLabelText("Point 2 at 0 comma 0"),
).toBeInTheDocument();
});

it("uses the ariaLabel when both sequence and ariaLabel are provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint
point={[0, 0]}
sequenceNumber={1}
ariaLabel="Aria Label being used instead of sequence number"
/>
</Mafs>,
);

expect(
screen.getByLabelText(
"Aria Label being used instead of sequence number",
),
).toBeInTheDocument();
});

it("uses the ariaLabel when only ariaLabel is provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint
point={[0, 0]}
ariaLabel="Custom aria label"
/>
</Mafs>,
);

expect(
screen.getByLabelText("Custom aria label"),
).toBeInTheDocument();
});

it("uses the ariaDescribedBy when provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint
point={[0, 0]}
ariaDescribedBy="description"
/>
<p id="description">Aria is described by me</p>
</Mafs>,
);

const pointElement = screen.getByRole("button", {
name: "Point 1 at 0 comma 0",
});
expect(pointElement).toHaveAttribute(
"aria-describedby",
"description",
);

const descriptionElement = screen.getByText(
"Aria is described by me",
);
expect(descriptionElement).toBeInTheDocument();
});

it("uses the ariaLive when provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint point={[0, 0]} ariaLive="assertive" />
</Mafs>,
);

expect(
screen.getByLabelText("Point 1 at 0 comma 0"),
).toHaveAttribute("aria-live", "assertive");
});

it("uses the default ariaLive when not provided", () => {
render(
<Mafs width={200} height={200}>
<MovablePoint point={[0, 0]} />
</Mafs>,
);

expect(
screen.getByLabelText("Point 1 at 0 comma 0"),
).toHaveAttribute("aria-live", "polite");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@ import * as React from "react";
import {useControlPoint} from "./use-control-point";

import type {CSSCursor} from "./css-cursor";
import type {AriaLive} from "../../types";
import type {KeyboardMovementConstraint} from "../use-draggable";
import type {vec} from "mafs";

type Props = {
point: vec.Vector2;
ariaDescribedBy?: string;
ariaLabel?: string;
ariaLive?: AriaLive;
color?: string;
constrain?: KeyboardMovementConstraint;
cursor?: CSSCursor | undefined;
/**
* Represents where this point stands in the overall point sequence.
* This is used to provide screen readers with context about the point.
Expand All @@ -16,14 +23,11 @@ type Props = {
* Note: This number is 1-indexed, and should restart from 1 for each
* interactive figure on the graph.
*/
sequenceNumber: number;
onMove?: (newPoint: vec.Vector2) => unknown;
sequenceNumber?: number;
onBlur?: (event: React.FocusEvent) => unknown;
onClick?: () => unknown;
color?: string;
cursor?: CSSCursor | undefined;
constrain?: KeyboardMovementConstraint;
onFocus?: ((event: React.FocusEvent) => unknown) | undefined;
onBlur?: ((event: React.FocusEvent) => unknown) | undefined;
onFocus?: (event: React.FocusEvent) => unknown;
onMove?: (newPoint: vec.Vector2) => unknown;
};

export const MovablePoint = React.forwardRef(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,20 @@ import {useDraggable} from "../use-draggable";
import {MovablePointView} from "./movable-point-view";

import type {CSSCursor} from "./css-cursor";
import type {AriaLive} from "../../types";
import type {KeyboardMovementConstraint} from "../use-draggable";
import type {vec} from "mafs";

type Params = {
point: vec.Vector2;
ariaDescribedBy?: string;
ariaLabel?: string;
ariaLive?: AriaLive;
color?: string | undefined;
constrain?: KeyboardMovementConstraint;
cursor?: CSSCursor | undefined;
// The focusableHandle element is assigned to the forwarded ref.
forwardedRef?: React.ForwardedRef<SVGGElement | null> | undefined;
/**
* Represents where this point stands in the overall point sequence.
* This is used to provide screen readers with context about the point.
Expand All @@ -25,14 +32,11 @@ type Params = {
* Note: This number is 1-indexed, and should restart from 1 for each
* interactive figure on the graph.
*/
sequenceNumber: number;
constrain?: KeyboardMovementConstraint;
sequenceNumber?: number;
onMove?: ((newPoint: vec.Vector2) => unknown) | undefined;
onClick?: (() => unknown) | undefined;
onFocus?: ((event: React.FocusEvent) => unknown) | undefined;
onBlur?: ((event: React.FocusEvent) => unknown) | undefined;
// The focusableHandle element is assigned to the forwarded ref.
forwardedRef?: React.ForwardedRef<SVGGElement | null> | undefined;
};

type Return = {
Expand All @@ -46,15 +50,18 @@ export function useControlPoint(params: Params): Return {
const {snapStep, disableKeyboardInteraction} = useGraphConfig();
const {
point,
sequenceNumber,
ariaDescribedBy,
ariaLabel,
ariaLive = "polite",
color,
cursor,
constrain = (p) => snap(snapStep, p),
cursor,
forwardedRef = noop,
sequenceNumber = 1,
onMove = noop,
onClick = noop,
onFocus = noop,
onBlur = noop,
forwardedRef = noop,
} = params;

const {strings, locale} = usePerseusI18n();
Expand All @@ -76,6 +83,15 @@ export function useControlPoint(params: Params): Return {
constrainKeyboardMovement: constrain,
});

// if custom aria label is not provided, will use default of sequence number and point coordinates
const pointAriaLabel =
ariaLabel ||
strings.srPointAtCoordinates({
num: sequenceNumber,
x: srFormatNumber(point[X], locale),
y: srFormatNumber(point[Y], locale),
});

useLayoutEffect(() => {
setForwardedRef(forwardedRef, focusableHandleRef.current);
}, [forwardedRef]);
Expand All @@ -87,14 +103,9 @@ export function useControlPoint(params: Params): Return {
tabIndex={disableKeyboardInteraction ? -1 : 0}
ref={focusableHandleRef}
role="button"
aria-label={strings.srPointAtCoordinates({
num: sequenceNumber,
x: srFormatNumber(point[X], locale),
y: srFormatNumber(point[Y], locale),
})}
// aria-live="assertive" causes the new location of the point to be
// announced immediately on move.
aria-live="assertive"
aria-describedby={ariaDescribedBy}
aria-label={pointAriaLabel}
aria-live={ariaLive}
onFocus={(event) => {
onFocus(event);
setFocused(true);
Expand Down
2 changes: 2 additions & 0 deletions packages/perseus/src/widgets/interactive-graphs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,5 @@ export type GraphDimensions = {
width: number; // pixels
height: number; // pixels
};

export type AriaLive = "off" | "assertive" | "polite" | undefined;

0 comments on commit 799ffe4

Please sign in to comment.