Add readOnly mode to interactive graphs to prevent keyboard interacti…

…ons when question marked correct (#1487) ## Summary: Widgets are placed into a `readOnly` mode when they are part of an exercise that was answered correctly. This is to prevent a state where the question is marked correct, but the user has modified the graph to an incorrect state. Mouse interactions are prevented by adding an invisible div over the widget, but this does not prevent keyboard interactions. This change adds a `readOnly` property to graphs that reduces the tabIndex to -1 in instances where `readOnly` mode is active (when the question was answered correctly). Issue: LEMS-2175 ## Test plan: - Confirm all tests pass - Checkout the branch associated with this PR. Run `yarn storybook` - Confirm the graph in the readOnly mode story is non-interactive: http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--polygon-with-mafs-read-only - Compare the interactivity with the corresponding story not in readOnly mode: http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--polygon-with-mafs - Use the PR npm snapshot to test this in webapp with a ZND to confirm when a graph related exercise is answered correctly that the graph cannot be interacted with either via mouse or keyboard (don't see a snapshot, so maybe this isn't possible) Author: Myranae Reviewers: benchristel, Myranae, jeremywiebe, nishasy Required Reviewers: Approved By: benchristel, jeremywiebe Checks: ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1487
Khan · Aug 8, 2024 · 53031f8 · 53031f8
1 parent de13fd3
commit 53031f8
Show file tree

Hide file tree

Showing 14 changed files with 87 additions and 31 deletions.
diff --git a/.changeset/short-paws-suffer.md b/.changeset/short-paws-suffer.md
@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": minor
+---
+
+Make graphs non-interactive via keyboard when their question has been answered correctly
diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts
@@ -200,6 +200,10 @@ export type APIOptions = Readonly<{
     ) => unknown;
     GroupMetadataEditor?: React.ComponentType<StubTagEditorType>;
     showAlignmentOptions?: boolean;
+    /**
+     * A boolean that indicates whether the associated problem has been
+     * answered correctly and should no longer be interactive.
+     */
     readOnly?: boolean;
     answerableCallback?: (arg1: boolean) => unknown;
     getAnotherHint?: () => unknown;

diff --git a/packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx b/packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx
@@ -22,17 +22,17 @@ import {
     segmentWithLockedPolygons,
 } from "../__testdata__/interactive-graph.testdata";
 
+import type {APIOptions} from "@khanacademy/perseus";
+
 export default {
     title: "Perseus/Widgets/Interactive Graph",
 };
 
-const mafsOptions = {
-    apiOptions: {
-        flags: {
-            mafs: {
-                segment: true,
-                polygon: true,
-            },
+const enableMafs: APIOptions = {
+    flags: {
+        mafs: {
+            segment: true,
+            polygon: true,
         },
     },
 };
@@ -68,7 +68,19 @@ export const Polygon = (args: StoryArgs): React.ReactElement => (
 );
 
 export const PolygonWithMafs = (args: StoryArgs): React.ReactElement => (
-    <RendererWithDebugUI {...mafsOptions} question={polygonQuestion} />
+    <RendererWithDebugUI
+        apiOptions={{...enableMafs}}
+        question={polygonQuestion}
+    />
+);
+
+export const PolygonWithMafsReadOnly = (
+    args: StoryArgs,
+): React.ReactElement => (
+    <RendererWithDebugUI
+        apiOptions={{...enableMafs, readOnly: true}}
+        question={polygonQuestion}
+    />
 );
 
 export const Ray = (args: StoryArgs): React.ReactElement => (
@@ -83,7 +95,7 @@ export const SegmentWithMafsAndLockedPoints = (
     args: StoryArgs,
 ): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithLockedPointsQuestion}
     />
 );
@@ -92,46 +104,49 @@ export const SegmentWithMafsAndLockedLines = (
     args: StoryArgs,
 ): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithLockedLineQuestion}
     />
 );
 
 export const AllLockedLineSegments = (args: StoryArgs): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithAllLockedLineSegmentVariations}
     />
 );
 
 export const AllLockedLines = (args: StoryArgs): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithAllLockedLineVariations}
     />
 );
 
 export const AllLockedRays = (args: StoryArgs): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithAllLockedRayVariations}
     />
 );
 
 export const LockedVector = (args: StoryArgs): React.ReactElement => (
-    <RendererWithDebugUI {...mafsOptions} question={segmentWithLockedVectors} />
+    <RendererWithDebugUI
+        apiOptions={{...enableMafs}}
+        question={segmentWithLockedVectors}
+    />
 );
 
 export const LockedEllipse = (args: StoryArgs): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithLockedEllipses}
     />
 );
 
 export const LockedPolygon = (args: StoryArgs): React.ReactElement => (
     <RendererWithDebugUI
-        {...mafsOptions}
+        apiOptions={{...enableMafs}}
         question={segmentWithLockedPolygons}
     />
 );

diff --git a/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx b/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx
@@ -265,6 +265,26 @@ describe("a mafs graph", () => {
                     {timeout: 5000},
                 );
             });
+
+            it("is marked invalid when readOnly set to true", async () => {
+                const {renderer} = renderQuestion(question, {
+                    ...apiOptions,
+                    readOnly: true,
+                });
+
+                await userEvent.tab();
+
+                // Act
+                await userEvent.keyboard("{arrowup}{arrowdown}");
+
+                // Assert
+                await waitFor(
+                    () => {
+                        expect(renderer).toHaveInvalidInput();
+                    },
+                    {timeout: 5000},
+                );
+            });
         },
     );
 

diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx
@@ -1827,6 +1827,7 @@ class InteractiveGraph extends React.Component<Props, State> {
                     box={box}
                     showTooltips={!!this.props.showTooltips}
                     hintMode={this.props.hintMode}
+                    readOnly={this.props.apiOptions?.readOnly}
                 />
             );
         }

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx
@@ -46,7 +46,7 @@ function MovableCircle(props: {
     onMove: (newCenter: vec.Vector2) => unknown;
 }) {
     const {center, radius, onMove} = props;
-    const {snapStep, hintMode} = useGraphConfig();
+    const {snapStep, disableKeyboardInteraction} = useGraphConfig();
 
     const draggableRef = useRef<SVGGElement>(null);
 
@@ -63,7 +63,7 @@ function MovableCircle(props: {
     return (
         <g
             ref={draggableRef}
-            tabIndex={hintMode ? -1 : 0}
+            tabIndex={disableKeyboardInteraction ? -1 : 0}
             className={`movable-circle ${dragging ? "movable-circle--dragging" : ""}`}
         >
             <ellipse

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx
@@ -78,7 +78,7 @@ function useControlPoint(
     color: string | undefined,
     onMovePoint: (newPoint: vec.Vector2) => unknown,
 ) {
-    const {snapStep, hintMode} = useGraphConfig();
+    const {snapStep, disableKeyboardInteraction} = useGraphConfig();
     const [focused, setFocused] = useState(false);
     const keyboardHandleRef = useRef<SVGGElement>(null);
     useDraggable({
@@ -100,7 +100,7 @@ function useControlPoint(
         <g
             data-testid="movable-point__focusable-handle"
             className="movable-point__focusable-handle"
-            tabIndex={hintMode ? -1 : 0}
+            tabIndex={disableKeyboardInteraction ? -1 : 0}
             ref={keyboardHandleRef}
             onFocus={() => setFocused(true)}
             onBlur={() => setFocused(false)}
@@ -142,8 +142,12 @@ export const Line = (props: LineProps) => {
     const {start, end, onMove, extend, stroke = defaultStroke} = props;
 
     const [startPtPx, endPtPx] = useTransformVectorsToPixels(start, end);
-    const {range, graphDimensionsInPixels, snapStep, hintMode} =
-        useGraphConfig();
+    const {
+        range,
+        graphDimensionsInPixels,
+        snapStep,
+        disableKeyboardInteraction,
+    } = useGraphConfig();
 
     let startExtend: vec.Vector2 | undefined = undefined;
     let endExtend: vec.Vector2 | undefined = undefined;
@@ -172,7 +176,7 @@ export const Line = (props: LineProps) => {
         <>
             <g
                 ref={line}
-                tabIndex={hintMode ? -1 : 0}
+                tabIndex={disableKeyboardInteraction ? -1 : 0}
                 className="movable-line"
                 data-testid="movable-line"
                 style={{cursor: dragging ? "grabbing" : "grab"}}

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point-view.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point-view.tsx
@@ -38,7 +38,8 @@ const hitboxSizePx = 48;
 // the description of https://github.com/Khan/perseus/pull/1240
 export const MovablePointView = forwardRef(
     (props: Props, hitboxRef: ForwardedRef<SVGGElement>) => {
-        const {range, markings, showTooltips, hintMode} = useGraphConfig();
+        const {range, markings, showTooltips, disableKeyboardInteraction} =
+            useGraphConfig();
         const {
             point,
             color = WBColor.blue,
@@ -90,7 +91,9 @@ export const MovablePointView = forwardRef(
                 className={pointClasses}
                 style={{"--movable-point-color": color, cursor} as any}
                 data-testid="movable-point"
-                tabIndex={hintMode ? -1 : tabIndex(focusBehavior)}
+                tabIndex={
+                    disableKeyboardInteraction ? -1 : tabIndex(focusBehavior)
+                }
             >
                 <circle
                     className="movable-point-hitbox"

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx
@@ -25,7 +25,7 @@ export const PolygonGraph = (props: Props) => {
     const {dispatch} = props;
     const {coords, showAngles, showSides, range, snapStep, snapTo} =
         props.graphState;
-    const {hintMode} = useGraphConfig();
+    const {disableKeyboardInteraction} = useGraphConfig();
 
     // TODO(benchristel): can the default set of points be removed here? I don't
     // think coords can be null.
@@ -103,7 +103,7 @@ export const PolygonGraph = (props: Props) => {
                 color="transparent"
                 svgPolygonProps={{
                     ref,
-                    tabIndex: hintMode ? -1 : 0,
+                    tabIndex: disableKeyboardInteraction ? -1 : 0,
                     strokeWidth: TARGET_SIZE,
                     style: {
                         cursor: dragging ? "grabbing" : "grab",

diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx
@@ -26,6 +26,7 @@ function getBaseMafsGraphProps(): MafsGraphProps {
         showTooltips: false,
         showProtractor: false,
         hintMode: false,
+        readOnly: false,
         labels: ["x", "y"],
         dispatch: () => {},
         state: {

diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx
@@ -46,10 +46,11 @@ export type MafsGraphProps = {
     state: InteractiveGraphState;
     dispatch: React.Dispatch<InteractiveGraphAction>;
     hintMode: boolean;
+    readOnly: boolean;
 };
 
 export const MafsGraph = (props: MafsGraphProps) => {
-    const {state, dispatch, labels, hintMode} = props;
+    const {state, dispatch, labels, hintMode, readOnly} = props;
     const [width, height] = props.box;
     const tickStep = props.step as vec.Vector2;
     return (
@@ -65,7 +66,7 @@ export const MafsGraph = (props: MafsGraphProps) => {
                 width,
                 height,
                 labels,
-                hintMode,
+                disableKeyboardInteraction: hintMode || readOnly,
             }}
         >
             <View

diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/use-graph-config.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/use-graph-config.ts
@@ -15,7 +15,7 @@ export type GraphConfig = {
     width: number; // pixels
     height: number; // pixels
     labels: readonly string[];
-    hintMode?: boolean;
+    disableKeyboardInteraction?: boolean;
 };
 
 const defaultGraphConfig: GraphConfig = {
@@ -32,7 +32,7 @@ const defaultGraphConfig: GraphConfig = {
     width: 0,
     height: 0,
     labels: [],
-    hintMode: false,
+    disableKeyboardInteraction: false,
 };
 
 export const GraphConfigContext =

diff --git a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx
@@ -26,6 +26,7 @@ function getBaseStatefulMafsGraphProps(): StatefulMafsGraphProps {
         showTooltips: false,
         showProtractor: false,
         hintMode: false,
+        readOnly: false,
         labels: ["x", "y"],
         graph: {type: "segment"},
     };

diff --git a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx
@@ -32,6 +32,7 @@ export type StatefulMafsGraphProps = {
     showProtractor: boolean;
     labels: InteractiveGraphProps["labels"];
     hintMode: boolean;
+    readOnly: boolean;
 };
 
 // Rather than be tightly bound to how data was structured in