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

Expose a way to get user input from ServerItemRenderer (#1753) #1753

Merged
merged 17 commits into from
Nov 18, 2024
Merged
6 changes: 6 additions & 0 deletions .changeset/rotten-starfishes-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": major
"@khanacademy/perseus-dev-ui": patch
---

Change ServerItemRenderer scoring APIs to externalize scoring
11 changes: 10 additions & 1 deletion dev/flipbook.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {useEffect, useMemo, useReducer, useRef, useState} from "react";

import {Renderer} from "../packages/perseus/src";
import {SvgImage} from "../packages/perseus/src/components";
import {scorePerseusItem} from "../packages/perseus/src/renderer-util";
import {mockStrings} from "../packages/perseus/src/strings";
import {isCorrect} from "../packages/perseus/src/util";
import {trueForAllMafsSupportedGraphTypes} from "../packages/perseus/src/widgets/interactive-graphs/mafs-supported-graph-types";
Expand Down Expand Up @@ -319,7 +320,15 @@ function GradableRenderer(props: QuestionRendererProps) {
leftContent={
<Button
onClick={() => {
setScore(rendererRef.current?.score());
if (rendererRef.current) {
const score = scorePerseusItem(
question,
rendererRef.current.getUserInputMap(),
mockStrings,
"en",
);
setScore(score);
}
clearScoreTimeout.set();
}}
>
Expand Down
12 changes: 4 additions & 8 deletions packages/perseus/src/components/__tests__/sorter.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,13 @@ describe("sorter widget", () => {
const {renderer} = renderQuestion(question1, apiOptions);
const sorter = renderer.findWidgets("sorter 1")[0];

// Act
// Put the options in the correct order

["$0.005$ kilograms", "$15$ grams", "$55$ grams"].forEach((option) => {
act(() => sorter.moveOptionToIndex(option, 3));
});
// Act
renderer.guessAndScore();

// assert
// Assert
expect(renderer).toHaveBeenAnsweredCorrectly();
});
it("can be answered incorrectly", () => {
Expand All @@ -95,17 +93,15 @@ describe("sorter widget", () => {
const {renderer} = renderQuestion(question1, apiOptions);
const sorter = renderer.findWidgets("sorter 1")[0];

// Act
// Put the options in the reverse order
["$0.005$ kilograms", "$15$ grams", "$55$ grams"].forEach(
(option, index) => {
act(() => sorter.moveOptionToIndex(option, 0));
},
);

// Act
renderer.guessAndScore();

// assert
// Assert
expect(renderer).toHaveBeenAnsweredIncorrectly();
});
});
2 changes: 2 additions & 0 deletions packages/perseus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export {default as ServerItemRenderer} from "./server-item-renderer";
export {default as HintsRenderer} from "./hints-renderer";
export {default as HintRenderer} from "./hint-renderer";
export {default as Renderer} from "./renderer";
export {scorePerseusItem} from "./renderer-util";

/**
* Widgets
Expand Down Expand Up @@ -226,6 +227,7 @@ export type {
PerseusWidgetsMap,
MultiItem,
} from "./perseus-types";
export type {UserInputMap} from "./validation.types";
export type {Coord} from "./interactive2/types";
export type {MarkerType} from "./widgets/label-image/types";

Expand Down
51 changes: 50 additions & 1 deletion packages/perseus/src/renderer-util.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import {emptyWidgetsFunctional, scoreWidgetsFunctional} from "./renderer-util";
import {screen} from "@testing-library/react";
import {userEvent as userEventLib} from "@testing-library/user-event";

import {
emptyWidgetsFunctional,
scorePerseusItem,
scoreWidgetsFunctional,
} from "./renderer-util";
import {mockStrings} from "./strings";
import {registerAllWidgetsForTesting} from "./util/register-all-widgets-for-testing";
import {renderQuestion} from "./widgets/__testutils__/renderQuestion";
import {question1} from "./widgets/group/group.testdata";

import type {DropdownWidget, PerseusWidgetsMap} from "./perseus-types";
import type {UserInputMap} from "./validation.types";
import type {UserEvent} from "@testing-library/user-event";

const testDropdownWidget: DropdownWidget = {
type: "dropdown",
Expand Down Expand Up @@ -469,3 +479,42 @@ describe("scoreWidgetsFunctional", () => {
expect(result["group 1"]).toHaveBeenAnsweredIncorrectly();
});
});

describe("scorePerseusItem", () => {
let userEvent: UserEvent;
beforeEach(() => {
userEvent = userEventLib.setup({
advanceTimers: jest.advanceTimersByTime,
});
});

it("should return score from contained Renderer", async () => {
// Arrange
const {renderer} = renderQuestion(question1);
// Answer all widgets correctly
await userEvent.click(screen.getAllByRole("radio")[4]);
// Note(jeremy): If we don't tab away from the radio button in this
// test, it seems like the userEvent typing doesn't land in the first
// text field.
await userEvent.tab();
await userEvent.type(
screen.getByRole("textbox", {name: /nearest ten/}),
"230",
);
await userEvent.type(
screen.getByRole("textbox", {name: /nearest hundred/}),
"200",
);
const userInput = renderer.getUserInputMap();
const score = scorePerseusItem(question1, userInput, mockStrings, "en");
handeyeco marked this conversation as resolved.
Show resolved Hide resolved

// Assert
expect(score).toHaveBeenAnsweredCorrectly();
expect(score).toEqual({
earned: 3,
message: null,
total: 3,
type: "points",
});
});
});
31 changes: 28 additions & 3 deletions packages/perseus/src/renderer-util.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import Util from "./util";
import {getWidgetIdsFromContent} from "./widget-type-utils";
import {getWidgetScorer} from "./widgets";

import type {PerseusWidgetsMap} from "./perseus-types";
import type {PerseusRenderer, PerseusWidgetsMap} from "./perseus-types";
import type {PerseusStrings} from "./strings";
import type {PerseusScore} from "./types";
import type {UserInput, UserInputMap} from "./validation.types";
Expand Down Expand Up @@ -50,11 +51,35 @@ export function emptyWidgetsFunctional(
});
}

// TODO: combine scorePerseusItem with scoreWidgetsFunctional
// once scorePerseusItem is the only one calling scoreWidgetsFunctional
export function scorePerseusItem(
perseusRenderData: PerseusRenderer,
userInputMap: UserInputMap,
// TODO(LEMS-2461,LEMS-2391): these probably
// need to be removed before we move this to the server
strings: PerseusStrings,
locale: string,
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
): PerseusScore {
// There seems to be a chance that PerseusRenderer.widgets might include
// widget data for widgets that are not in PerseusRenderer.content,
// so this checks that the widgets are being used before scoring them
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
const usedWidgetIds = getWidgetIdsFromContent(perseusRenderData.content);
const scores = scoreWidgetsFunctional(
perseusRenderData.widgets,
usedWidgetIds,
userInputMap,
strings,
locale,
);
return Util.flattenScores(scores);
}

export function scoreWidgetsFunctional(
widgets: PerseusWidgetsMap,
// This is a port of old code, I'm not sure why
// we need widgetIds vs the keys of the widgets object
widgetIds: Array<string>,
widgetIds: ReadonlyArray<string>,
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
userInputMap: UserInputMap,
strings: PerseusStrings,
locale: string,
Expand All @@ -79,7 +104,7 @@ export function scoreWidgetsFunctional(
if (widget.type === "group") {
const scores = scoreWidgetsFunctional(
widget.options.widgets,
Object.keys(widget.options.widgets),
getWidgetIdsFromContent(widget.options.content),
userInputMap[id] as UserInputMap,
strings,
locale,
Expand Down
38 changes: 17 additions & 21 deletions packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ class Renderer
// /cry(aria)
this._foundTextNodes = true;

if (_.contains(this.widgetIds, node.id)) {
if (this.widgetIds.includes(node.id)) {
// We don't want to render a duplicate widget key/ref,
// as this causes problems with react (for obvious
// reasons). Instead we just notify the
Expand Down Expand Up @@ -1510,15 +1510,15 @@ class Renderer

getInputPaths: () => ReadonlyArray<FocusPath> = () => {
const inputPaths: Array<FocusPath> = [];
_.each(this.widgetIds, (widgetId: string) => {
this.widgetIds.forEach((widgetId: string) => {
const widget = this.getWidgetInstance(widgetId);
if (widget && widget.getInputPaths) {
// Grab all input paths and add widgetID to the front
const widgetInputPaths = widget.getInputPaths();
// Prefix paths with their widgetID and add to collective
// list of paths.
// @ts-expect-error - TS2345 - Argument of type '(inputPath: string) => void' is not assignable to parameter of type 'CollectionIterator<FocusPath, void, readonly FocusPath[]>'.
_.each(widgetInputPaths, (inputPath: string) => {
widgetInputPaths.forEach((inputPath: string) => {
const relativeInputPath = [widgetId].concat(inputPath);
inputPaths.push(relativeInputPath);
});
Expand Down Expand Up @@ -1742,46 +1742,42 @@ class Renderer
}

/**
* Returns an object mapping from widget ID to perseus-style score.
* The keys of this object are the values of the array returned
* from `getWidgetIds`.
* Scores the content.
*
* @deprecated use scorePerseusItem
*/
scoreWidgets(): {[widgetId: string]: PerseusScore} {
return scoreWidgetsFunctional(
score(): PerseusScore {
const scores = scoreWidgetsFunctional(
this.state.widgetInfo,
this.widgetIds,
this.getUserInputMap(),
this.props.strings,
this.context.locale,
);
}

/**
* Grades the content.
*/
score(): PerseusScore {
const scores = this.scoreWidgets();
const combinedScore = Util.flattenScores(scores);
return combinedScore;
}

guessAndScore(): [UserInputArray, PerseusScore] {
/**
* @deprecated use scorePerseusItem
*/
guessAndScore: () => [UserInputArray, PerseusScore] = () => {
const totalGuess = this.getUserInput();
const totalScore = this.score();

return [totalGuess, totalScore];
}
};

examples: () => ReadonlyArray<string> | null | undefined = () => {
const widgetIds = this.widgetIds;
const examples = _.compact(
_.map(widgetIds, (widgetId) => {
const examples = widgetIds
.map((widgetId) => {
const widget = this.getWidgetInstance(widgetId);
return widget != null && widget.examples
? widget.examples()
: null;
}),
);
})
.filter(Boolean);

// no widgets with examples
if (!examples.length) {
Expand Down
Loading
Loading