From 53031f8df120a7ea15b6d82e5a94988d9281a9b3 Mon Sep 17 00:00:00 2001 From: Tamara <60857422+Myranae@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:53:08 -0500 Subject: [PATCH] Add readOnly mode to interactive graphs to prevent keyboard interactions when question marked correct (#1487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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: https://github.com/Khan/perseus/pull/1487 --- .changeset/short-paws-suffer.md | 5 ++ packages/perseus/src/types.ts | 4 ++ .../__stories__/interactive-graph.stories.tsx | 47 ++++++++++++------- .../__tests__/interactive-graph.test.tsx | 20 ++++++++ .../perseus/src/widgets/interactive-graph.tsx | 1 + .../interactive-graphs/graphs/circle.tsx | 4 +- .../graphs/components/movable-line.tsx | 14 ++++-- .../graphs/components/movable-point-view.tsx | 7 ++- .../interactive-graphs/graphs/polygon.tsx | 4 +- .../interactive-graphs/mafs-graph.test.tsx | 1 + .../widgets/interactive-graphs/mafs-graph.tsx | 5 +- .../reducer/use-graph-config.ts | 4 +- .../stateful-mafs-graph.test.tsx | 1 + .../stateful-mafs-graph.tsx | 1 + 14 files changed, 87 insertions(+), 31 deletions(-) create mode 100644 .changeset/short-paws-suffer.md diff --git a/.changeset/short-paws-suffer.md b/.changeset/short-paws-suffer.md new file mode 100644 index 0000000000..8a7f37404d --- /dev/null +++ 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 index 0555392292..0ec64276b9 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -200,6 +200,10 @@ export type APIOptions = Readonly<{ ) => unknown; GroupMetadataEditor?: React.ComponentType; 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 index 2ee0c46466..77d9f2119d 100644 --- 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 => ( - + +); + +export const PolygonWithMafsReadOnly = ( + args: StoryArgs, +): React.ReactElement => ( + ); export const Ray = (args: StoryArgs): React.ReactElement => ( @@ -83,7 +95,7 @@ export const SegmentWithMafsAndLockedPoints = ( args: StoryArgs, ): React.ReactElement => ( ); @@ -92,46 +104,49 @@ export const SegmentWithMafsAndLockedLines = ( args: StoryArgs, ): React.ReactElement => ( ); export const AllLockedLineSegments = (args: StoryArgs): React.ReactElement => ( ); export const AllLockedLines = (args: StoryArgs): React.ReactElement => ( ); export const AllLockedRays = (args: StoryArgs): React.ReactElement => ( ); export const LockedVector = (args: StoryArgs): React.ReactElement => ( - + ); export const LockedEllipse = (args: StoryArgs): React.ReactElement => ( ); export const LockedPolygon = (args: StoryArgs): React.ReactElement => ( ); diff --git a/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx b/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx index 38da22e1b7..8c3a2a0eb2 100644 --- 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 index 0b647e3dc8..1a72c41aa8 100644 --- 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 { 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 index 5ee56870e5..8f4e220ba2 100644 --- 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(null); @@ -63,7 +63,7 @@ function MovableCircle(props: { return ( unknown, ) { - const {snapStep, hintMode} = useGraphConfig(); + const {snapStep, disableKeyboardInteraction} = useGraphConfig(); const [focused, setFocused] = useState(false); const keyboardHandleRef = useRef(null); useDraggable({ @@ -100,7 +100,7 @@ function useControlPoint( 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) => { <> ) => { - 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) + } > { 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 index 5d5b6eba94..75aa4905aa 100644 --- 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 index d800975eb0..3e70522859 100644 --- 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; 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, }} >