Skip to content

Commit 53031f8

Browse files
authored
Add readOnly mode to interactive graphs to prevent keyboard interactions 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
1 parent de13fd3 commit 53031f8

14 files changed

+87
-31
lines changed

.changeset/short-paws-suffer.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/perseus": minor
3+
---
4+
5+
Make graphs non-interactive via keyboard when their question has been answered correctly

packages/perseus/src/types.ts

+4
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,10 @@ export type APIOptions = Readonly<{
200200
) => unknown;
201201
GroupMetadataEditor?: React.ComponentType<StubTagEditorType>;
202202
showAlignmentOptions?: boolean;
203+
/**
204+
* A boolean that indicates whether the associated problem has been
205+
* answered correctly and should no longer be interactive.
206+
*/
203207
readOnly?: boolean;
204208
answerableCallback?: (arg1: boolean) => unknown;
205209
getAnotherHint?: () => unknown;

packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx

+31-16
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,17 @@ import {
2222
segmentWithLockedPolygons,
2323
} from "../__testdata__/interactive-graph.testdata";
2424

25+
import type {APIOptions} from "@khanacademy/perseus";
26+
2527
export default {
2628
title: "Perseus/Widgets/Interactive Graph",
2729
};
2830

29-
const mafsOptions = {
30-
apiOptions: {
31-
flags: {
32-
mafs: {
33-
segment: true,
34-
polygon: true,
35-
},
31+
const enableMafs: APIOptions = {
32+
flags: {
33+
mafs: {
34+
segment: true,
35+
polygon: true,
3636
},
3737
},
3838
};
@@ -68,7 +68,19 @@ export const Polygon = (args: StoryArgs): React.ReactElement => (
6868
);
6969

7070
export const PolygonWithMafs = (args: StoryArgs): React.ReactElement => (
71-
<RendererWithDebugUI {...mafsOptions} question={polygonQuestion} />
71+
<RendererWithDebugUI
72+
apiOptions={{...enableMafs}}
73+
question={polygonQuestion}
74+
/>
75+
);
76+
77+
export const PolygonWithMafsReadOnly = (
78+
args: StoryArgs,
79+
): React.ReactElement => (
80+
<RendererWithDebugUI
81+
apiOptions={{...enableMafs, readOnly: true}}
82+
question={polygonQuestion}
83+
/>
7284
);
7385

7486
export const Ray = (args: StoryArgs): React.ReactElement => (
@@ -83,7 +95,7 @@ export const SegmentWithMafsAndLockedPoints = (
8395
args: StoryArgs,
8496
): React.ReactElement => (
8597
<RendererWithDebugUI
86-
{...mafsOptions}
98+
apiOptions={{...enableMafs}}
8799
question={segmentWithLockedPointsQuestion}
88100
/>
89101
);
@@ -92,46 +104,49 @@ export const SegmentWithMafsAndLockedLines = (
92104
args: StoryArgs,
93105
): React.ReactElement => (
94106
<RendererWithDebugUI
95-
{...mafsOptions}
107+
apiOptions={{...enableMafs}}
96108
question={segmentWithLockedLineQuestion}
97109
/>
98110
);
99111

100112
export const AllLockedLineSegments = (args: StoryArgs): React.ReactElement => (
101113
<RendererWithDebugUI
102-
{...mafsOptions}
114+
apiOptions={{...enableMafs}}
103115
question={segmentWithAllLockedLineSegmentVariations}
104116
/>
105117
);
106118

107119
export const AllLockedLines = (args: StoryArgs): React.ReactElement => (
108120
<RendererWithDebugUI
109-
{...mafsOptions}
121+
apiOptions={{...enableMafs}}
110122
question={segmentWithAllLockedLineVariations}
111123
/>
112124
);
113125

114126
export const AllLockedRays = (args: StoryArgs): React.ReactElement => (
115127
<RendererWithDebugUI
116-
{...mafsOptions}
128+
apiOptions={{...enableMafs}}
117129
question={segmentWithAllLockedRayVariations}
118130
/>
119131
);
120132

121133
export const LockedVector = (args: StoryArgs): React.ReactElement => (
122-
<RendererWithDebugUI {...mafsOptions} question={segmentWithLockedVectors} />
134+
<RendererWithDebugUI
135+
apiOptions={{...enableMafs}}
136+
question={segmentWithLockedVectors}
137+
/>
123138
);
124139

125140
export const LockedEllipse = (args: StoryArgs): React.ReactElement => (
126141
<RendererWithDebugUI
127-
{...mafsOptions}
142+
apiOptions={{...enableMafs}}
128143
question={segmentWithLockedEllipses}
129144
/>
130145
);
131146

132147
export const LockedPolygon = (args: StoryArgs): React.ReactElement => (
133148
<RendererWithDebugUI
134-
{...mafsOptions}
149+
apiOptions={{...enableMafs}}
135150
question={segmentWithLockedPolygons}
136151
/>
137152
);

packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx

+20
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,26 @@ describe("a mafs graph", () => {
265265
{timeout: 5000},
266266
);
267267
});
268+
269+
it("is marked invalid when readOnly set to true", async () => {
270+
const {renderer} = renderQuestion(question, {
271+
...apiOptions,
272+
readOnly: true,
273+
});
274+
275+
await userEvent.tab();
276+
277+
// Act
278+
await userEvent.keyboard("{arrowup}{arrowdown}");
279+
280+
// Assert
281+
await waitFor(
282+
() => {
283+
expect(renderer).toHaveInvalidInput();
284+
},
285+
{timeout: 5000},
286+
);
287+
});
268288
},
269289
);
270290

packages/perseus/src/widgets/interactive-graph.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -1827,6 +1827,7 @@ class InteractiveGraph extends React.Component<Props, State> {
18271827
box={box}
18281828
showTooltips={!!this.props.showTooltips}
18291829
hintMode={this.props.hintMode}
1830+
readOnly={this.props.apiOptions?.readOnly}
18301831
/>
18311832
);
18321833
}

packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ function MovableCircle(props: {
4646
onMove: (newCenter: vec.Vector2) => unknown;
4747
}) {
4848
const {center, radius, onMove} = props;
49-
const {snapStep, hintMode} = useGraphConfig();
49+
const {snapStep, disableKeyboardInteraction} = useGraphConfig();
5050

5151
const draggableRef = useRef<SVGGElement>(null);
5252

@@ -63,7 +63,7 @@ function MovableCircle(props: {
6363
return (
6464
<g
6565
ref={draggableRef}
66-
tabIndex={hintMode ? -1 : 0}
66+
tabIndex={disableKeyboardInteraction ? -1 : 0}
6767
className={`movable-circle ${dragging ? "movable-circle--dragging" : ""}`}
6868
>
6969
<ellipse

packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx

+9-5
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ function useControlPoint(
7878
color: string | undefined,
7979
onMovePoint: (newPoint: vec.Vector2) => unknown,
8080
) {
81-
const {snapStep, hintMode} = useGraphConfig();
81+
const {snapStep, disableKeyboardInteraction} = useGraphConfig();
8282
const [focused, setFocused] = useState(false);
8383
const keyboardHandleRef = useRef<SVGGElement>(null);
8484
useDraggable({
@@ -100,7 +100,7 @@ function useControlPoint(
100100
<g
101101
data-testid="movable-point__focusable-handle"
102102
className="movable-point__focusable-handle"
103-
tabIndex={hintMode ? -1 : 0}
103+
tabIndex={disableKeyboardInteraction ? -1 : 0}
104104
ref={keyboardHandleRef}
105105
onFocus={() => setFocused(true)}
106106
onBlur={() => setFocused(false)}
@@ -142,8 +142,12 @@ export const Line = (props: LineProps) => {
142142
const {start, end, onMove, extend, stroke = defaultStroke} = props;
143143

144144
const [startPtPx, endPtPx] = useTransformVectorsToPixels(start, end);
145-
const {range, graphDimensionsInPixels, snapStep, hintMode} =
146-
useGraphConfig();
145+
const {
146+
range,
147+
graphDimensionsInPixels,
148+
snapStep,
149+
disableKeyboardInteraction,
150+
} = useGraphConfig();
147151

148152
let startExtend: vec.Vector2 | undefined = undefined;
149153
let endExtend: vec.Vector2 | undefined = undefined;
@@ -172,7 +176,7 @@ export const Line = (props: LineProps) => {
172176
<>
173177
<g
174178
ref={line}
175-
tabIndex={hintMode ? -1 : 0}
179+
tabIndex={disableKeyboardInteraction ? -1 : 0}
176180
className="movable-line"
177181
data-testid="movable-line"
178182
style={{cursor: dragging ? "grabbing" : "grab"}}

packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point-view.tsx

+5-2
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ const hitboxSizePx = 48;
3838
// the description of https://github.com/Khan/perseus/pull/1240
3939
export const MovablePointView = forwardRef(
4040
(props: Props, hitboxRef: ForwardedRef<SVGGElement>) => {
41-
const {range, markings, showTooltips, hintMode} = useGraphConfig();
41+
const {range, markings, showTooltips, disableKeyboardInteraction} =
42+
useGraphConfig();
4243
const {
4344
point,
4445
color = WBColor.blue,
@@ -90,7 +91,9 @@ export const MovablePointView = forwardRef(
9091
className={pointClasses}
9192
style={{"--movable-point-color": color, cursor} as any}
9293
data-testid="movable-point"
93-
tabIndex={hintMode ? -1 : tabIndex(focusBehavior)}
94+
tabIndex={
95+
disableKeyboardInteraction ? -1 : tabIndex(focusBehavior)
96+
}
9497
>
9598
<circle
9699
className="movable-point-hitbox"

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export const PolygonGraph = (props: Props) => {
2525
const {dispatch} = props;
2626
const {coords, showAngles, showSides, range, snapStep, snapTo} =
2727
props.graphState;
28-
const {hintMode} = useGraphConfig();
28+
const {disableKeyboardInteraction} = useGraphConfig();
2929

3030
// TODO(benchristel): can the default set of points be removed here? I don't
3131
// think coords can be null.
@@ -103,7 +103,7 @@ export const PolygonGraph = (props: Props) => {
103103
color="transparent"
104104
svgPolygonProps={{
105105
ref,
106-
tabIndex: hintMode ? -1 : 0,
106+
tabIndex: disableKeyboardInteraction ? -1 : 0,
107107
strokeWidth: TARGET_SIZE,
108108
style: {
109109
cursor: dragging ? "grabbing" : "grab",

packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ function getBaseMafsGraphProps(): MafsGraphProps {
2626
showTooltips: false,
2727
showProtractor: false,
2828
hintMode: false,
29+
readOnly: false,
2930
labels: ["x", "y"],
3031
dispatch: () => {},
3132
state: {

packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ export type MafsGraphProps = {
4646
state: InteractiveGraphState;
4747
dispatch: React.Dispatch<InteractiveGraphAction>;
4848
hintMode: boolean;
49+
readOnly: boolean;
4950
};
5051

5152
export const MafsGraph = (props: MafsGraphProps) => {
52-
const {state, dispatch, labels, hintMode} = props;
53+
const {state, dispatch, labels, hintMode, readOnly} = props;
5354
const [width, height] = props.box;
5455
const tickStep = props.step as vec.Vector2;
5556
return (
@@ -65,7 +66,7 @@ export const MafsGraph = (props: MafsGraphProps) => {
6566
width,
6667
height,
6768
labels,
68-
hintMode,
69+
disableKeyboardInteraction: hintMode || readOnly,
6970
}}
7071
>
7172
<View

packages/perseus/src/widgets/interactive-graphs/reducer/use-graph-config.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export type GraphConfig = {
1515
width: number; // pixels
1616
height: number; // pixels
1717
labels: readonly string[];
18-
hintMode?: boolean;
18+
disableKeyboardInteraction?: boolean;
1919
};
2020

2121
const defaultGraphConfig: GraphConfig = {
@@ -32,7 +32,7 @@ const defaultGraphConfig: GraphConfig = {
3232
width: 0,
3333
height: 0,
3434
labels: [],
35-
hintMode: false,
35+
disableKeyboardInteraction: false,
3636
};
3737

3838
export const GraphConfigContext =

packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ function getBaseStatefulMafsGraphProps(): StatefulMafsGraphProps {
2626
showTooltips: false,
2727
showProtractor: false,
2828
hintMode: false,
29+
readOnly: false,
2930
labels: ["x", "y"],
3031
graph: {type: "segment"},
3132
};

packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export type StatefulMafsGraphProps = {
3232
showProtractor: boolean;
3333
labels: InteractiveGraphProps["labels"];
3434
hintMode: boolean;
35+
readOnly: boolean;
3536
};
3637

3738
// Rather than be tightly bound to how data was structured in

0 commit comments

Comments
 (0)