reviewModeRubric prework: limit reviewModeRubric uses (#1741)

## Summary: Not sure we'll want to merge this as it might be too risky/dirty, but I thought I'd try to remove as many instances as I could of `reviewModeRubric` without really changing logic to make it clearer where we're actually using it. Seems to just be radio. Issue: Prework for LEMS-2437 Author: handeyeco Reviewers: handeyeco, jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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: #1741
Khan · Oct 22, 2024 · 3e48b2c · 3e48b2c
1 parent ea7ede6
commit 3e48b2c
Show file tree

Hide file tree

Showing 14 changed files with 32 additions and 70 deletions.
diff --git a/.changeset/six-clocks-buy.md b/.changeset/six-clocks-buy.md
@@ -0,0 +1,6 @@
+---
+"@khanacademy/perseus": minor
+"@khanacademy/perseus-editor": patch
+---
+
+Scope reviewModeRubric to just the component that uses it (Radio)
diff --git a/packages/perseus-editor/src/widgets/input-number-editor.tsx b/packages/perseus-editor/src/widgets/input-number-editor.tsx
@@ -4,8 +4,10 @@ import _ from "underscore";
 
 import BlurInput from "../components/blur-input";
 
-import type {ParsedValue, InputNumber} from "@khanacademy/perseus";
-import type {PropsFor} from "@khanacademy/wonder-blocks-core";
+import type {
+    ParsedValue,
+    PerseusInputNumberWidgetOptions,
+} from "@khanacademy/perseus";
 
 const {InfoTip} = components;
 
@@ -46,14 +48,12 @@ const answerTypes = {
 
 type Props = {
     value: number;
-    simplify: PropsFor<typeof InputNumber.widget>["simplify"];
-    size: PropsFor<typeof InputNumber.widget>["size"];
-    inexact: PropsFor<typeof InputNumber.widget>["reviewModeRubric"]["inexact"];
-    maxError: PropsFor<
-        typeof InputNumber.widget
-    >["reviewModeRubric"]["maxError"];
-    answerType: PropsFor<typeof InputNumber.widget>["answerType"];
-    rightAlign: PropsFor<typeof InputNumber.widget>["rightAlign"];
+    simplify: PerseusInputNumberWidgetOptions["simplify"];
+    size: PerseusInputNumberWidgetOptions["size"];
+    inexact: PerseusInputNumberWidgetOptions["inexact"];
+    maxError: PerseusInputNumberWidgetOptions["maxError"];
+    answerType: PerseusInputNumberWidgetOptions["answerType"];
+    rightAlign: PerseusInputNumberWidgetOptions["rightAlign"];
     onChange: (arg1: {
         value?: ParsedValue | 0;
         simplify?: Props["simplify"];

diff --git a/packages/perseus-editor/src/widgets/radio/editor.tsx b/packages/perseus-editor/src/widgets/radio/editor.tsx
@@ -339,6 +339,7 @@ class RadioEditor extends React.Component<any> {
                     editMode={true}
                     labelWrap={false}
                     apiOptions={this.props.apiOptions}
+                    reviewMode={false}
                     choices={this.props.choices.map((choice, i) => {
                         return {
                             content: (

diff --git a/packages/perseus/src/renderer.tsx b/packages/perseus/src/renderer.tsx
@@ -608,6 +608,7 @@ class Renderer extends React.Component<Props, State> {
             onBlur: _.partial(this._onWidgetBlur, id),
             findWidgets: this.findWidgets,
             reviewModeRubric: reviewModeRubric,
+            reviewMode: this.props.reviewMode,
             onChange: (newProps, cb, silent = false) => {
                 this._setWidgetProps(id, newProps, cb, silent);
             },

diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts
@@ -671,7 +671,8 @@ export type WidgetProps<
     onFocus: (blurPath: FocusPath) => void;
     onBlur: (blurPath: FocusPath) => void;
     findWidgets: (criterion: FilterCriterion) => ReadonlyArray<Widget>;
-    reviewModeRubric: Rubric;
+    reviewModeRubric?: Rubric | null | undefined;
+    reviewMode: boolean;
     onChange: ChangeHandler;
     // This is slightly different from the `trackInteraction` function in
     // APIOptions. This provides the widget an easy way to notify the renderer

diff --git a/packages/perseus/src/widgets/expression/expression.stories.tsx b/packages/perseus/src/widgets/expression/expression.stories.tsx
@@ -9,7 +9,7 @@ import TestKeypadContextWrapper from "../__shared__/test-keypad-context-wrapper"
 import expressionExport from "./expression";
 import {expressionItem2, expressionItem3} from "./expression.testdata";
 
-import type {LegacyButtonSets, PerseusItem} from "../../perseus-types";
+import type {PerseusItem} from "../../perseus-types";
 import type {Keys as Key} from "@khanacademy/math-input";
 
 type StoryArgs = {
@@ -56,21 +56,6 @@ const WrappedKeypadContext = ({
 };
 
 export const DesktopKitchenSink = (args: StoryArgs): React.ReactElement => {
-    const reviewModeRubric = {
-        functions: ["f", "g", "h"],
-        times: true,
-        answerForms: [],
-        buttonSets: [
-            "basic",
-            "basic+div",
-            "trig",
-            "prealgebra",
-            "logarithms",
-            "basic relations",
-            "advanced relations",
-        ] as LegacyButtonSets,
-    };
-
     const keypadConfiguration = {
         keypadType: KeypadType.EXPRESSION,
         extraKeys: ["x", "y", "z"] as Array<Key>,
@@ -91,8 +76,8 @@ export const DesktopKitchenSink = (args: StoryArgs): React.ReactElement => {
                 static={false}
                 trackInteraction={() => {}}
                 widgetId="expression"
-                reviewModeRubric={reviewModeRubric}
                 keypadConfiguration={keypadConfiguration}
+                reviewMode={false}
             />
         </div>
     );

diff --git a/packages/perseus/src/widgets/group/group.tsx b/packages/perseus/src/widgets/group/group.tsx
@@ -170,7 +170,7 @@ class Group extends React.Component<Props> implements Widget {
                     ref={(ref) => (this.rendererRef = ref)}
                     apiOptions={apiOptions}
                     findExternalWidgets={this.props.findWidgets}
-                    reviewMode={!!this.props.reviewModeRubric}
+                    reviewMode={this.props.reviewMode}
                     onInteractWithWidget={onInteractWithWidget}
                     linterContext={this.props.linterContext}
                     strings={this.context.strings}

diff --git a/packages/perseus/src/widgets/input-number/input-number.tsx b/packages/perseus/src/widgets/input-number/input-number.tsx
@@ -203,10 +203,6 @@ class InputNumber extends React.Component<Props> implements Widget {
 
             return input;
         }
-        // HACK(johnsullivan): Create a function with shared logic between
-        // this and NumericInput.
-        // TODO(jeremy): Deprecate this widget and prefer numeric-input.
-        const rubric = this.props.reviewModeRubric;
 
         // Note: This is _very_ similar to what `numeric-input.jsx` does. If
         // you modify this, double-check if you also need to modify that
@@ -217,7 +213,7 @@ class InputNumber extends React.Component<Props> implements Widget {
             this.props.rightAlign ? styles.rightAlign : styles.leftAlign,
         ];
         // Unanswered
-        if (rubric && !this.props.currentValue) {
+        if (this.props.reviewMode && !this.props.currentValue) {
             inputStyles.push(styles.answerStateUnanswered);
         }
 

diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx
@@ -22,13 +22,6 @@ function generateProps(overwrite) {
         coefficient: false,
         currentValue: "",
         problemNum: 0,
-        reviewModeRubric: {
-            answers: [],
-            labelText: "",
-            size: "medium",
-            coefficient: false,
-            static: false,
-        },
         rightAlign: false,
         size: "normal",
         static: false,

diff --git a/packages/perseus/src/widgets/passage/__tests__/passage.test.tsx b/packages/perseus/src/widgets/passage/__tests__/passage.test.tsx
@@ -45,12 +45,10 @@ function renderPassage(
         onChange: () => {},
         onFocus: () => {},
         problemNum: 1,
-        reviewModeRubric: {
-            ...widgetPropsBase,
-        },
         static: true,
         trackInteraction: () => {},
         widgetId: "passage",
+        reviewMode: false,
     } as const;
 
     const extended = {...base, ...overwrite} as const;

diff --git a/packages/perseus/src/widgets/passage/passage.tsx b/packages/perseus/src/widgets/passage/passage.tsx
@@ -414,7 +414,7 @@ export class Passage
         const enabled = this.state.stylesAreApplied;
 
         // Highlights are read-only in review mode.
-        const editable = !this.props.reviewModeRubric;
+        const editable = !this.props.reviewMode;
         return (
             <HighlightableContent
                 editable={editable}

diff --git a/packages/perseus/src/widgets/radio/__stories__/base-radio.stories.tsx b/packages/perseus/src/widgets/radio/__stories__/base-radio.stories.tsx
@@ -26,6 +26,7 @@ export default {
 
 const defaultProps = {
     apiOptions: {},
+    reviewMode: false,
     choices: [
         generateChoice({
             content: "Content 1",
@@ -138,19 +139,9 @@ export const SingleKitchenSink = (args: StoryArgs): React.ReactElement => {
     choices[1].checked = true;
     choices[2].correct = true;
 
-    const rubricChoices = choices.map(({correct}) => ({
-        // note(matthew): reviewModeRubric.choices requires content,
-        // but I don't see how it's getting used and TypeScript gets mad
-        // when I use choice.content because it's not a string.
-        // reviewModeRubric could probably use a look over.
-        content: "",
-        correct,
-    }));
-
     const overwrittenProps = {
         ...defaultProps,
         multipleSelect: false,
-        reviewModeRubric: {choices: rubricChoices},
         choices,
     } as const;
     return <BaseRadio {...overwrittenProps} />;
@@ -176,20 +167,10 @@ export const MultipleKitchenSink = (args: StoryArgs): React.ReactElement => {
     choices[2].correct = true;
     choices[3].correct = true;
 
-    const rubricChoices = choices.map((c) => ({
-        // note(matthew): reviewModeRubric.choices requires content,
-        // but I don't see how it's getting used and TypeScript gets mad
-        // when I use choice.content because it's not a string.
-        // reviewModeRubric could probably use a look over.
-        content: "",
-        correct: c.correct,
-    }));
-
     const overwrittenProps = {
         ...defaultProps,
         multipleSelect: true,
         numCorrect: 2,
-        reviewModeRubric: {choices: rubricChoices},
         choices,
     } as const;
     return <BaseRadio {...overwrittenProps} />;

diff --git a/packages/perseus/src/widgets/radio/base-radio.tsx b/packages/perseus/src/widgets/radio/base-radio.tsx
@@ -56,7 +56,8 @@ type Props = {
     multipleSelect: boolean;
     // the logic checks whether this exists,
     // so it must be optional
-    reviewModeRubric?: PerseusRadioWidgetOptions;
+    reviewModeRubric?: PerseusRadioWidgetOptions | null;
+    reviewMode: boolean;
     // A callback indicating that this choice has changed. Its argument is
     // an object with two keys: `checked` and `crossedOut`. Each contains
     // an array of boolean values, specifying the new checked and
@@ -93,6 +94,7 @@ const BaseRadio = function (props: Props): React.ReactElement {
     const {
         apiOptions,
         reviewModeRubric,
+        reviewMode,
         choices,
         editMode,
         multipleSelect,
@@ -217,7 +219,6 @@ const BaseRadio = function (props: Props): React.ReactElement {
     });
 
     // some commonly used shorthands
-    const reviewMode = !!reviewModeRubric;
     const isMobile = apiOptions.isMobile;
 
     const firstChoiceHighlighted = choices[0].highlighted;

diff --git a/packages/perseus/src/widgets/radio/radio-component.tsx b/packages/perseus/src/widgets/radio/radio-component.tsx
@@ -434,9 +434,7 @@ class Radio extends React.Component<Props> implements Widget {
                     previouslyAnswered,
                 } = choiceStates[i];
 
-                const reviewChoice =
-                    this.props.reviewModeRubric &&
-                    this.props.reviewModeRubric.choices[i];
+                const reviewChoice = this.props.reviewModeRubric?.choices[i];
 
                 return {
                     content: this._renderRenderer(content),
@@ -480,6 +478,7 @@ class Radio extends React.Component<Props> implements Widget {
                 choices={choicesProp}
                 onChange={this.updateChoices}
                 reviewModeRubric={this.props.reviewModeRubric}
+                reviewMode={this.props.reviewMode}
                 deselectEnabled={this.props.deselectEnabled}
                 apiOptions={this.props.apiOptions}
                 isLastUsedWidget={this.props.isLastUsedWidget}