Remove score prop from ServerItemRenderer

.changeset/light-drinks-kneel.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": major
+---
+
+Remove `score` prop from ServerItemRenderer

packages/perseus/src/__docs__/renderers-overview.mdx

@@ -85,7 +85,6 @@ Perseus renderers accept a variety of props to configure their behavior. The mos
 | \`widgets\` | \`PerseusWidgetsMap\` | Object mapping widget IDs to their configuration data |
 | \`images\` | \`{[key: string]: any}\` | Images to be used in the content |
 | \`problemNum\` | \`number\` | The problem number (for exercises with multiple parts) |
-| \`questionCompleted\` | \`boolean\` | Whether the question has been answered correctly |
 | \`showSolutions\` | \`"all" \| "selected" \| "none"\` | Controls display of widget solutions and rationales |
 | \`linterContext\` | \`LinterContextProps\` | Options for the Perseus linter, including whether to highlight lint warnings |
     `}

packages/perseus/src/mixins/widget-prop-denylist.ts

@@ -25,7 +25,6 @@ const denylist = [
     "problemNum",
     "apiOptions",
     "widgetIsOpen",
-    "questionCompleted",
     "findWidgets",
     // added by src/editor.jsx, for widgets removing themselves
     // this is soooo not the right place for this, but alas.

packages/perseus/src/renderer.tsx

@@ -123,14 +123,12 @@ type Props = Partial<React.ContextType<typeof DependenciesContext>> & {
     apiOptions?: APIOptions;
     alwaysUpdate?: boolean;
     findExternalWidgets: any;
-    highlightedWidgets?: ReadonlyArray<any>;
     images: PerseusRenderer["images"];
     keypadElement?: KeypadAPI | null;
-    onInteractWithWidget: (id: string) => void;
     onRender: (node?: any) => void;
     problemNum?: number;
-    questionCompleted?: boolean;
     reviewMode?: boolean | null | undefined;
+    highlightEmptyWidgets?: boolean;
     /**
      * If set to "all", all rationales or solutions will be shown. If set to
      * "selected", soltions will only be shown for selected choices. If set to
@@ -173,12 +171,9 @@ type DefaultProps = Required<
         | "alwaysUpdate"
         | "content"
         | "findExternalWidgets"
-        | "highlightedWidgets"
         | "images"
         | "linterContext"
-        | "onInteractWithWidget"
         | "onRender"
-        | "questionCompleted"
         | "showSolutions"
         | "reviewMode"
         | "widgets"
@@ -253,14 +248,11 @@ class Renderer
         content: "",
         widgets: {},
         images: {},
-        highlightedWidgets: [],
-        questionCompleted: false,
         showSolutions: "none",
         // onRender may be called multiple times per render, for example
         // if there are multiple images or TeX pieces within `content`.
         // It is a good idea to debounce any functions passed here.
         onRender: noopOnRender,
-        onInteractWithWidget: function () {},
         findExternalWidgets: () => [],
         alwaysUpdate: false,
         reviewMode: false,
@@ -304,6 +296,8 @@ class Renderer
                 this.handletranslationLintErrors,
             );
         }
+
+        this.props.apiOptions?.answerableCallback?.(this._isAnswerable());
     }
 
     UNSAFE_componentWillReceiveProps(nextProps: Props) {
@@ -358,6 +352,13 @@ class Renderer
                 this.handletranslationLintErrors,
             );
         }
+
+        if (
+            this.props.userInput &&
+            !_.isEqual(this.props.userInput, prevProps.userInput)
+        ) {
+            this.props.apiOptions?.answerableCallback?.(this._isAnswerable());
+        }
     }
 
     componentWillUnmount() {
@@ -431,6 +432,13 @@ class Renderer
         );
     };
 
+    _isAnswerable(): boolean {
+        if (this.props.userInput) {
+            return this.emptyWidgets().length === 0;
+        }
+        return false;
+    }
+
     renderWidget: (
         impliedType: string,
         id: string,
@@ -448,11 +456,11 @@ class Renderer
 
         if (widgetInfo) {
             const type = (widgetInfo && widgetInfo.type) || impliedType;
-            const shouldHighlight = _.contains(
-                // @ts-expect-error - TS2345 - Argument of type 'readonly any[] | undefined' is not assignable to parameter of type 'Collection<any>'.
-                this.props.highlightedWidgets,
-                id,
-            );
+
+            let shouldHighlight = false;
+            if (this.props.highlightEmptyWidgets && this.props.userInput) {
+                shouldHighlight = this.emptyWidgets().includes(id);
+            }
 
             // By this point we should have no duplicates, which are
             // filtered out in this.render(), so we shouldn't have to
@@ -537,7 +545,6 @@ class Renderer
             problemNum: this.props.problemNum,
             apiOptions: this.getApiOptions(),
             keypadElement: this.props.keypadElement,
-            questionCompleted: this.props.questionCompleted,
             showSolutions: this.props.showSolutions,
             onFocus: _.partial(this._onWidgetFocus, widgetId),
             onBlur: _.partial(this._onWidgetBlur, widgetId),
@@ -561,7 +568,7 @@ class Renderer
                     newUserInput,
                     widgetsEmpty,
                 );
-                this.props.onInteractWithWidget(widgetId);
+                this.props.apiOptions?.interactionCallback?.(updatedUserInput);
             },
             trackInteraction: interactionTracker.track,
         };
@@ -1442,35 +1449,6 @@ class Renderer
         );
     }
 
-    handleStateUpdate(id: string, cb: () => boolean, silent?: boolean) {
-        // Wait until all components have rendered. In React 16 setState
-        // callback fires immediately after this componentDidUpdate, and
-        // there is no guarantee that parent/siblings components have
-        // finished rendering.
-        // TODO(jeff, CP-3128): Use Wonder Blocks Timing API
-        // eslint-disable-next-line no-restricted-syntax
-        setTimeout(() => {
-            // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-            const cbResult = cb && cb();
-            if (!silent) {
-                this.props.onInteractWithWidget(id);
-            }
-            if (cbResult !== false) {
-                // TODO(jack): For some reason, some widgets don't always
-                // end up in refs here, which is repro-able if you make an
-                // [[ orderer 1 ]] and copy-paste this, then change it to
-                // be an [[ orderer 2 ]]. The resulting Renderer ends up
-                // with an "orderer 2" ref but not an "orderer 1" ref.
-                // @_@??
-                // TODO(jack): Figure out why this is happening and fix it
-                // As far as I can tell, this is only an issue in the
-                // editor-page, so doing this shouldn't break clients
-                // hopefully
-                this._setCurrentFocus([id]);
-            }
-        }, 0);
-    }
-
     /**
      * Returns an array of the widget `.getUserInput()` results
      *

packages/perseus/src/server-item-renderer.tsx

@@ -37,7 +37,6 @@ import type {
     ShowSolutions,
     KeypadContextRendererInterface,
     RendererInterface,
-    KEScore,
     UserInputArray,
     UserInputMap,
 } from "@khanacademy/perseus-core";
@@ -46,9 +45,9 @@ import type {PropsFor} from "@khanacademy/wonder-blocks-core";
 type OwnProps = {
     hintsVisible?: number;
     item: PerseusItem;
-    score?: KEScore | null;
     problemNum?: number;
     reviewMode?: boolean;
+    highlightEmptyWidgets?: boolean;
     keypadElement?: KeypadAPI | null | undefined;
     dependencies: PerseusDependenciesV2;
     showSolutions?: ShowSolutions;
@@ -65,19 +64,6 @@ type DefaultProps = Required<
 >;
 
 type State = {
-    /**
-     * questionCompleted is used to signal that a learner has attempted
-     * the exercise. This is used when widgets want to show things like
-     * rationale or partial correctness.
-     */
-    questionCompleted: boolean;
-    /**
-     * As far as I can tell, this is used to highlight empty widgets
-     * after a learner has clicked the "check" button. I don't think this could
-     * still be used though, because the "check" button is disabled while there
-     * are empty widgets.
-     */
-    questionHighlightedWidgets: ReadonlyArray<string>;
     /**
      * Keeps track of whether each asset (SvgImage or TeX) rendered by
      * the questionRenderer has finished loading or rendering.
@@ -124,8 +110,6 @@ export class ServerItemRenderer
         super(props);
 
         this.state = {
-            questionCompleted: false,
-            questionHighlightedWidgets: [],
             assetStatuses: {},
         };
         this._fullyRendered = false;
@@ -145,32 +129,8 @@ export class ServerItemRenderer
         this.maybeCallOnRendered();
     }
 
-    // eslint-disable-next-line react/no-unsafe
-    UNSAFE_componentWillReceiveProps(nextProps: Props) {
-        this.setState({
-            questionHighlightedWidgets: [],
-        });
-    }
-
-    componentDidUpdate(prevProps: Props, prevState: State) {
-        const answerableCallback = this.props.apiOptions.answerableCallback;
-        if (answerableCallback != null) {
-            const isAnswerable =
-                this.questionRenderer.emptyWidgets().length === 0;
-            answerableCallback(isAnswerable);
-        }
-
+    componentDidUpdate() {
         this.maybeCallOnRendered();
-
-        if (this.props.score && this.props.score !== prevProps.score) {
-            const emptyQuestionAreaWidgets =
-                this.questionRenderer.emptyWidgets();
-
-            this.setState({
-                questionCompleted: this.props.score.correct,
-                questionHighlightedWidgets: emptyQuestionAreaWidgets,
-            });
-        }
     }
 
     componentWillUnmount() {
@@ -304,28 +264,6 @@ export class ServerItemRenderer
         return questionAreaInputPaths;
     }
 
-    handleInteractWithWidget: (widgetId: string) => void = (widgetId) => {
-        const withRemoved = _.difference(
-            this.state.questionHighlightedWidgets,
-            [widgetId],
-        );
-
-        this.setState(
-            {
-                questionCompleted: false,
-                questionHighlightedWidgets: withRemoved,
-            },
-            () => {
-                // Call the interactionCallback, if it exists,
-                // with the current user input data
-                // (in the setState callback to avoid stale state)
-                this.props.apiOptions?.interactionCallback?.(
-                    this.questionRenderer.getUserInputMap(),
-                );
-            },
-        );
-    };
-
     focus(): boolean | null | undefined {
         return this.questionRenderer.focus();
     }
@@ -422,14 +360,7 @@ export class ServerItemRenderer
                             <Renderer
                                 keypadElement={this.props.keypadElement}
                                 problemNum={this.props.problemNum}
-                                onInteractWithWidget={
-                                    this.handleInteractWithWidget
-                                }
-                                highlightedWidgets={
-                                    this.state.questionHighlightedWidgets
-                                }
                                 apiOptions={apiOptions}
-                                questionCompleted={this.state.questionCompleted}
                                 reviewMode={this.props.reviewMode}
                                 showSolutions={this.props.showSolutions}
                                 ref={(elem) => {
@@ -457,7 +388,6 @@ export class ServerItemRenderer
                                         userInput,
                                         widgetsEmpty,
                                     );
-                                    this.handleInteractWithWidget(id);
                                 }}
                                 initializeUserInput={initializeUserInput}
                             />

packages/perseus/src/types.ts

@@ -543,11 +543,7 @@ type UniversalWidgetProps<TUserInput = Empty, TrackingExtraArgs = Empty> = {
     problemNum: number | null | undefined;
     apiOptions: APIOptionsWithDefaults;
     keypadElement?: any;
-    /**
-     * questionCompleted is used to signal that a learner has attempted
-     * the exercise. This is used when widgets want to show things like
-     * rationale or partial correctness.
-     */
+    // TODO(LEMS-3783): remove uses of `questionCompleted`
     questionCompleted?: boolean;
     onFocus: (blurPath: FocusPath) => void;
     onBlur: (blurPath: FocusPath) => void;

packages/perseus/src/widgets/group/group.test.tsx

@@ -120,31 +120,6 @@ describe("group widget", () => {
         });
     });
 
-    it("should call onInteractWithWidget when internal widget interacted with", async () => {
-        // Arrange
-        const onInteractWithWidget = jest.fn();
-
-        renderQuestion(
-            question1,
-            {},
-            {
-                onInteractWithWidget,
-            },
-        );
-
-        // Act
-        await userEvent.type(
-            screen.getByRole("textbox", {
-                name: "value rounded to the nearest ten",
-            }),
-            "99",
-        );
-
-        // Assert
-        // NOTE: The numeric-input that we typed into is in the second group.
-        expect(onInteractWithWidget).toHaveBeenCalledWith("group 2");
-    });
-
     it("should return contained renderer's getUserInput", async () => {
         // Arrange
         const {renderer} = renderQuestion(question1);

packages/perseus/src/widgets/radio/__tests__/base-radio.testdata.ts

@@ -14,6 +14,7 @@ export function generateChoice(options: Partial<ChoiceType>): ChoiceType {
         isNoneOfTheAbove: false,
         highlighted: false,
         previouslyAnswered: false,
+        // TODO(LEMS-3783): remove uses of `revealNoneOfTheAbove`
         revealNoneOfTheAbove: false,
         disabled: false,
     } as const;

packages/perseus/src/widgets/radio/__tests__/multiple-choice-component.test.tsx

@@ -25,6 +25,7 @@ const baseChoiceValues = {
     correct: false,
     isNoneOfTheAbove: false,
     previouslyAnswered: false,
+    // TODO(LEMS-3783): remove uses of `revealNoneOfTheAbove`
     revealNoneOfTheAbove: false,
     disabled: false,
 };

packages/perseus/src/widgets/radio/base-radio.tsx

@@ -36,6 +36,7 @@ export type ChoiceType = {
     isNoneOfTheAbove: boolean;
     highlighted: boolean;
     previouslyAnswered: boolean;
+    // TODO(LEMS-3783): remove uses of `revealNoneOfTheAbove`
     revealNoneOfTheAbove: boolean;
     disabled: boolean;
 };
@@ -264,6 +265,7 @@ const BaseRadio = function ({
                     if (choice.isNoneOfTheAbove) {
                         Element = ChoiceNoneAbove;
                         _.extend(elementProps, {
+                            // TODO(LEMS-3783): remove uses of `revealNoneOfTheAbove`
                             showContent: choice.revealNoneOfTheAbove,
                         });
                     }