make sequenceNumber optional

Khan · Nov 26, 2024 · 8bcc3b7 · 8bcc3b7
1 parent 059baef
commit 8bcc3b7
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 28 deletions.
diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx
@@ -287,10 +287,10 @@ describe("MovablePoint", () => {
     });
 
     describe("accessibility", () => {
-        it("uses the default ariaLabel when not provided", () => {
+        it("uses the default sequence number when ariaLabel and sequence number are not provided", () => {
             render(
                 <Mafs width={200} height={200}>
-                    <MovablePoint point={[0, 0]} sequenceNumber={1} />
+                    <MovablePoint point={[0, 0]} />
                 </Mafs>,
             );
 
@@ -299,12 +299,41 @@ describe("MovablePoint", () => {
             ).toBeInTheDocument();
         });
 
-        it("uses the ariaLabel when provided", () => {
+        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>,
@@ -320,7 +349,6 @@ describe("MovablePoint", () => {
                 <Mafs width={200} height={200}>
                     <MovablePoint
                         point={[0, 0]}
-                        sequenceNumber={1}
                         ariaDescribedBy="description"
                     />
                     <p id="description">Aria is described by me</p>
@@ -344,11 +372,7 @@ describe("MovablePoint", () => {
         it("uses the ariaLive when provided", () => {
             render(
                 <Mafs width={200} height={200}>
-                    <MovablePoint
-                        point={[0, 0]}
-                        sequenceNumber={1}
-                        ariaLive="assertive"
-                    />
+                    <MovablePoint point={[0, 0]} ariaLive="assertive" />
                 </Mafs>,
             );
 
@@ -360,7 +384,7 @@ describe("MovablePoint", () => {
         it("uses the default ariaLive when not provided", () => {
             render(
                 <Mafs width={200} height={200}>
-                    <MovablePoint point={[0, 0]} sequenceNumber={1} />
+                    <MovablePoint point={[0, 0]} />
                 </Mafs>,
             );
 

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx
@@ -9,6 +9,12 @@ 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.
@@ -17,13 +23,7 @@ type Props = {
      * Note: This number is 1-indexed, and should restart from 1 for each
      * interactive figure on the graph.
      */
-    sequenceNumber: number;
-    ariaDescribedBy?: string;
-    ariaLabel?: string;
-    ariaLive?: AriaLive;
-    color?: string;
-    constrain?: KeyboardMovementConstraint;
-    cursor?: CSSCursor | undefined;
+    sequenceNumber?: number;
     onBlur?: ((event: React.FocusEvent) => unknown) | undefined;
     onClick?: () => unknown;
     onFocus?: ((event: React.FocusEvent) => unknown) | undefined;

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/use-control-point.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/use-control-point.tsx
@@ -16,15 +16,6 @@ import type {vec} from "mafs";
 
 type Params = {
     point: vec.Vector2;
-    /**
-     * Represents where this point stands in the overall point sequence.
-     * This is used to provide screen readers with context about the point.
-     * Example: sequenceNumber={1} ==> "Point 1 at x comma y"
-     *
-     * Note: This number is 1-indexed, and should restart from 1 for each
-     * interactive figure on the graph.
-     */
-    sequenceNumber: number;
     ariaDescribedBy?: string;
     ariaLabel?: string;
     ariaLive?: AriaLive;
@@ -33,6 +24,15 @@ type Params = {
     constrain?: KeyboardMovementConstraint;
     // 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.
+     * Example: sequenceNumber={1} ==> "Point 1 at x comma y"
+     *
+     * 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) | undefined;
     onClick?: (() => unknown) | undefined;
     onFocus?: ((event: React.FocusEvent) => unknown) | undefined;
@@ -50,18 +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),
+        forwardedRef = noop,
+        sequenceNumber = 1,
         onMove = noop,
         onClick = noop,
         onFocus = noop,
         onBlur = noop,
-        forwardedRef = noop,
     } = params;
 
     const {strings, locale} = usePerseusI18n();