Skip to content

Commit

Permalink
Add Keyboard support for unlimited points (#1646)
Browse files Browse the repository at this point in the history
## Summary:
Adds keyboard support for unlimited point graphs.


https://github.com/user-attachments/assets/2ae468e1-82e0-4127-b371-bc4ad42a2ee3



- Adds a new overlay when the graph is focused that prompts the learner to enter keyboard mode when shift+return is pressed
- When in keyboard mode a button to "Add point" exists
- There is no way to leave keyboard mode for a graph once it is entered (this is by design)
- The delete button and the add point button are never shown at the same time

Figma: https://www.figma.com/design/JroPokeCLBv2VRIw3GPvY5/LEMS%3A-Interactive-Graph-Widget?node-id=2285-24101&node-type=frame&t=qhAvSlmYoTKgv1IN-0

Issue: [LEMS-1669](https://khanacademy.atlassian.net/browse/LEMS-1669)

## Test plan:

[LEMS-1669]: https://khanacademy.atlassian.net/browse/LEMS-1669?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Author: nicolecomputer

Reviewers: mark-fitzgerald, nicolecomputer, benchristel

Required Reviewers:

Approved By: mark-fitzgerald

Checks: ❌ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1646
  • Loading branch information
nicolecomputer committed Sep 23, 2024
1 parent 2950ec3 commit 7822ea6
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 37 deletions.
5 changes: 5 additions & 0 deletions .changeset/curvy-hats-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Keyboard support for unlimited point graphs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ describe("MafsGraph", () => {
focusedPointIndex: null,
hasBeenInteractedWith: true,
showRemovePointButton: false,
interactionMode: "mouse",
showKeyboardInteractionInvitation: false,
range: [
[-10, 10],
[-10, 10],
Expand Down Expand Up @@ -399,6 +401,8 @@ describe("MafsGraph", () => {
focusedPointIndex: 0,
hasBeenInteractedWith: true,
showRemovePointButton: true,
interactionMode: "mouse",
showKeyboardInteractionInvitation: false,
range: [
[-10, 10],
[-10, 10],
Expand Down Expand Up @@ -436,6 +440,8 @@ describe("MafsGraph", () => {
focusedPointIndex: 0,
hasBeenInteractedWith: true,
showRemovePointButton: true,
interactionMode: "mouse",
showKeyboardInteractionInvitation: false,
range: [
[-10, 10],
[-10, 10],
Expand Down
152 changes: 117 additions & 35 deletions packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*/
import Button from "@khanacademy/wonder-blocks-button";
import {View} from "@khanacademy/wonder-blocks-core";
import {LabelMedium} from "@khanacademy/wonder-blocks-typography";
import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core";
import {Mafs} from "mafs";
import * as React from "react";
Expand Down Expand Up @@ -138,17 +139,20 @@ export const MafsGraph = (props: MafsGraphProps) => {
height,
}}
onKeyUp={(event) => {
if (event.key === "Backspace") {
dispatch(actions.global.deleteIntent());
graphRef.current?.focus();
}
handleKeyboardEvent(event, state, dispatch);
}}
aria-label={fullGraphAriaLabel}
aria-describedby={
fullGraphAriaDescription ? descriptionId : undefined
}
ref={graphRef}
tabIndex={0}
onFocus={(event) => {
handleFocusEvent(event, state, dispatch);
}}
onBlur={(event) => {
handleBlurEvent(event, state, dispatch);
}}
>
{fullGraphAriaDescription && (
<View
Expand Down Expand Up @@ -262,6 +266,29 @@ export const MafsGraph = (props: MafsGraphProps) => {
</Mafs>
</View>
</View>
{state.type === "point" &&
state.showKeyboardInteractionInvitation && (
<View
style={{
textAlign: "center",
backgroundColor: "white",
border: "1px solid #21242C52",
padding: "16px 0",
boxShadow: "0px 8px 8px 0px #21242C14",

// This translates the box to the center of the
// graph Then backs it off by half of its
// overall height so it's perfectly centered
top: "50%",
transform: "translateY(-50%)",
}}
>
<LabelMedium>
Press <strong>Shift + Enter</strong> to
interact with the graph
</LabelMedium>
</View>
)}
</View>
{renderGraphControls({state, dispatch, width})}
</View>
Expand All @@ -273,50 +300,59 @@ const renderPointGraphControls = (props: {
state: PointGraphState;
dispatch: (action: InteractiveGraphAction) => unknown;
width: number;
}) => (
<View
style={{
flexDirection: "row",
width: props.width,
}}
>
{/* <Button
kind="secondary"
}) => {
const {interactionMode, showRemovePointButton, focusedPointIndex} =
props.state;
return (
<View
style={{
width: "100%",
marginLeft: "20px",
}}
tabIndex={0}
onClick={() => {
props.dispatch(actions.pointGraph.addPoint([0, 0]));
flexDirection: "row",
width: props.width,
}}
>
Add Point
</Button> */}
{props.state.showRemovePointButton &&
props.state.focusedPointIndex !== null && (
{interactionMode === "keyboard" && (
<Button
id={REMOVE_BUTTON_ID}
kind="secondary"
color="destructive"
tabIndex={-1}
style={{
width: "100%",
marginLeft: "20px",
}}
onClick={(event) => {
props.dispatch(
actions.pointGraph.removePoint(
props.state.focusedPointIndex!,
),
);
tabIndex={0}
onClick={() => {
props.dispatch(actions.pointGraph.addPoint([0, 0]));
}}
>
Remove Point
Add Point
</Button>
)}
</View>
);
{interactionMode === "mouse" &&
showRemovePointButton &&
focusedPointIndex !== null && (
<Button
id={REMOVE_BUTTON_ID}
kind="secondary"
color="destructive"
// This button is meant to be interacted with by the mouse only
// Never allow learners to tab to this button
tabIndex={-1}
style={{
width: "100%",
marginLeft: "20px",
}}
onClick={(event) => {
props.dispatch(
actions.pointGraph.removePoint(
props.state.focusedPointIndex!,
),
);
}}
>
Remove Point
</Button>
)}
</View>
);
};

const renderGraphControls = (props: {
state: InteractiveGraphState;
Expand All @@ -336,6 +372,52 @@ const renderGraphControls = (props: {
}
};

function handleFocusEvent(
event: React.FocusEvent,
state: InteractiveGraphState,
dispatch: (action: InteractiveGraphAction) => unknown,
) {
if (state.type === "point" && state.numPoints === "unlimited") {
if (
event.target.classList.contains("mafs-graph") &&
state.interactionMode === "mouse"
) {
dispatch(actions.global.changeKeyboardInvitationVisibility(true));
}
}
}

function handleBlurEvent(
event: React.FocusEvent,
state: InteractiveGraphState,
dispatch: (action: InteractiveGraphAction) => unknown,
) {
if (state.type === "point" && state.numPoints === "unlimited") {
dispatch(actions.global.changeKeyboardInvitationVisibility(false));
}
}

function handleKeyboardEvent(
event: React.KeyboardEvent,
state: InteractiveGraphState,
dispatch: (action: InteractiveGraphAction) => unknown,
) {
if (state.type === "point" && state.numPoints === "unlimited") {
if (event.key === "Backspace") {
dispatch(actions.global.deleteIntent());

// After removing a point blur
// It would be nice if this could focus on the graph but doing so
// would trigger the message to prompt a learner to enter keyboard mode
(document.activeElement as HTMLElement).blur();
} else if (event.shiftKey && event.key === "Enter") {
dispatch(actions.global.changeInteractionMode("keyboard"));
} else if (state.interactionMode === "keyboard" && event.key === "a") {
dispatch(actions.pointGraph.addPoint([0, 0]));
}
}
}

// Calculate the difference between the min and max values of a range
const getRangeDiff = (range: vec.Vector2) => {
const [min, max] = range;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ export function initializeGraphState(
numPoints: graph.numPoints || 0,
focusedPointIndex: null,
showRemovePointButton: false,
interactionMode: "mouse",
showKeyboardInteractionInvitation: false,
};
case "circle":
return {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {InitializeGraphStateParams} from "./initialize-graph-state";
import type {InteractionMode} from "../types";
import type {Interval, vec} from "mafs";

export type InteractiveGraphAction =
Expand All @@ -16,11 +17,15 @@ export type InteractiveGraphAction =
| FocusPoint
| BlurPoint
| DeleteIntent
| ClickPoint;
| ClickPoint
| ChangeInteractionMode
| ChangeKeyboardInvitationVisibility;

export const actions = {
global: {
deleteIntent,
changeInteractionMode,
changeKeyboardInvitationVisibility,
},
angle: {
movePoint,
Expand Down Expand Up @@ -150,6 +155,35 @@ function clickPoint(index: number): ClickPoint {
};
}

export const CHANGE_INTERACTION_MODE = "point-change-interaction-mode";

export interface ChangeInteractionMode {
type: typeof CHANGE_INTERACTION_MODE;
mode: InteractionMode;
}
function changeInteractionMode(mode: InteractionMode): ChangeInteractionMode {
return {
type: CHANGE_INTERACTION_MODE,
mode,
};
}

export const CHANGE_KEYBOARD_INVITATION_VISIBILITY =
"change-keyboard-interaction-invitation-visibility";

export interface ChangeKeyboardInvitationVisibility {
type: typeof CHANGE_KEYBOARD_INVITATION_VISIBILITY;
shouldShow: boolean;
}
function changeKeyboardInvitationVisibility(
shouldShow: boolean,
): ChangeKeyboardInvitationVisibility {
return {
type: CHANGE_KEYBOARD_INVITATION_VISIBILITY,
shouldShow,
};
}

export const MOVE_ALL = "move-all";
export interface MoveAll {
type: typeof MOVE_ALL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const baseSegmentGraphState: InteractiveGraphState = {
const basePointGraphState: InteractiveGraphState = {
hasBeenInteractedWith: false,
showRemovePointButton: false,
interactionMode: "mouse",
showKeyboardInteractionInvitation: false,
type: "point",
range: [
[-10, 10],
Expand All @@ -39,6 +41,8 @@ const basePointGraphState: InteractiveGraphState = {
const baseUnlimitedPointGraphState: PointGraphState = {
hasBeenInteractedWith: false,
showRemovePointButton: false,
interactionMode: "mouse",
showKeyboardInteractionInvitation: false,
type: "point",
numPoints: "unlimited",
range: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ import {
type BlurPoint,
CLICK_POINT,
type ClickPoint,
CHANGE_INTERACTION_MODE,
type ChangeInteractionMode,
CHANGE_KEYBOARD_INVITATION_VISIBILITY,
type ChangeKeyboardInvitationVisibility,
} from "./interactive-graph-action";

import type {Coord} from "../../../interactive2/types";
Expand Down Expand Up @@ -99,6 +103,10 @@ export function interactiveGraphReducer(
return doDeleteIntent(state, action);
case CLICK_POINT:
return doClickPoint(state, action);
case CHANGE_INTERACTION_MODE:
return doChangeInteractionMode(state, action);
case CHANGE_KEYBOARD_INVITATION_VISIBILITY:
return doChangeKeyboardInvitationVisibility(state, action);
default:
throw new UnreachableCaseError(action);
}
Expand Down Expand Up @@ -172,6 +180,48 @@ function doClickPoint(
return state;
}

function doChangeInteractionMode(
state: InteractiveGraphState,
action: ChangeInteractionMode,
): InteractiveGraphState {
if (state.type !== "point") {
return state;
}

if (state.numPoints === "unlimited") {
const nextKeyboardInvitation =
action.mode === "keyboard"
? false
: state.showKeyboardInteractionInvitation;
return {
...state,
interactionMode: action.mode,
showKeyboardInteractionInvitation: nextKeyboardInvitation,
};
}

return state;
}

function doChangeKeyboardInvitationVisibility(
state: InteractiveGraphState,
action: ChangeKeyboardInvitationVisibility,
): InteractiveGraphState {
if (state.type !== "point") {
return state;
}

if (state.numPoints === "unlimited") {
return {
...state,
showKeyboardInteractionInvitation: action.shouldShow,
hasBeenInteractedWith: true,
};
}

return state;
}

function doMovePointInFigure(
state: InteractiveGraphState,
action: MovePointInFigure,
Expand Down
Loading

0 comments on commit 7822ea6

Please sign in to comment.