Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add readOnly mode to interactive graphs to prevent keyboard interactions when question marked correct #1487

Merged
merged 10 commits into from
Aug 8, 2024
5 changes: 5 additions & 0 deletions .changeset/short-paws-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Make graphs non-interactive via keyboard when their question has been answered correctly
4 changes: 4 additions & 0 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,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.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting this!

readOnly?: boolean;
answerableCallback?: (arg1: boolean) => unknown;
getAnotherHint?: () => unknown;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
};
Expand Down Expand Up @@ -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 => (
Expand All @@ -83,7 +95,7 @@ export const SegmentWithMafsAndLockedPoints = (
args: StoryArgs,
): React.ReactElement => (
<RendererWithDebugUI
{...mafsOptions}
apiOptions={{...enableMafs}}
question={segmentWithLockedPointsQuestion}
/>
);
Expand All @@ -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}
/>
);
Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/widgets/interactive-graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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)}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ function getBaseMafsGraphProps(): MafsGraphProps {
showTooltips: false,
showProtractor: false,
hintMode: false,
readOnly: false,
labels: ["x", "y"],
dispatch: () => {},
state: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -65,7 +66,7 @@ export const MafsGraph = (props: MafsGraphProps) => {
width,
height,
labels,
hintMode,
disableKeyboardInteraction: hintMode || readOnly,
}}
>
<View
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export type GraphConfig = {
width: number; // pixels
height: number; // pixels
labels: readonly string[];
hintMode?: boolean;
disableKeyboardInteraction?: boolean;
};

const defaultGraphConfig: GraphConfig = {
Expand All @@ -32,7 +32,7 @@ const defaultGraphConfig: GraphConfig = {
width: 0,
height: 0,
labels: [],
hintMode: false,
disableKeyboardInteraction: false,
};

export const GraphConfigContext =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ function getBaseStatefulMafsGraphProps(): StatefulMafsGraphProps {
showTooltips: false,
showProtractor: false,
hintMode: false,
readOnly: false,
labels: ["x", "y"],
graph: {type: "segment"},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down