From 3e48b2c26fbd1f649ce3878468e06043b88b0949 Mon Sep 17 00:00:00 2001 From: Matthew Date: Tue, 22 Oct 2024 16:27:33 -0500 Subject: [PATCH] reviewModeRubric prework: limit reviewModeRubric uses (#1741) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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: https://github.com/Khan/perseus/pull/1741 --- .changeset/six-clocks-buy.md | 6 ++++++ .../src/widgets/input-number-editor.tsx | 20 +++++++++--------- .../src/widgets/radio/editor.tsx | 1 + packages/perseus/src/renderer.tsx | 1 + packages/perseus/src/types.ts | 3 ++- .../widgets/expression/expression.stories.tsx | 19 ++--------------- packages/perseus/src/widgets/group/group.tsx | 2 +- .../src/widgets/input-number/input-number.tsx | 6 +----- .../numeric-input/numeric-input.stories.tsx | 7 ------- .../passage/__tests__/passage.test.tsx | 4 +--- .../perseus/src/widgets/passage/passage.tsx | 2 +- .../radio/__stories__/base-radio.stories.tsx | 21 +------------------ .../perseus/src/widgets/radio/base-radio.tsx | 5 +++-- .../src/widgets/radio/radio-component.tsx | 5 ++--- 14 files changed, 32 insertions(+), 70 deletions(-) create mode 100644 .changeset/six-clocks-buy.md diff --git a/.changeset/six-clocks-buy.md b/.changeset/six-clocks-buy.md new file mode 100644 index 0000000000..c8479199b9 --- /dev/null +++ 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 index 6e99f299f5..8cc8ba78c1 100644 --- 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["simplify"]; - size: PropsFor["size"]; - inexact: PropsFor["reviewModeRubric"]["inexact"]; - maxError: PropsFor< - typeof InputNumber.widget - >["reviewModeRubric"]["maxError"]; - answerType: PropsFor["answerType"]; - rightAlign: PropsFor["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 index ab597477ed..231bdfd13b 100644 --- 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 { 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 index c0981f856d..167f17c236 100644 --- a/packages/perseus/src/renderer.tsx +++ b/packages/perseus/src/renderer.tsx @@ -608,6 +608,7 @@ class Renderer extends React.Component { 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 index bfa3815d9f..59d81ee91c 100644 --- 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; - 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 index 6bb70aa05b..ed07491861 100644 --- 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, @@ -91,8 +76,8 @@ export const DesktopKitchenSink = (args: StoryArgs): React.ReactElement => { static={false} trackInteraction={() => {}} widgetId="expression" - reviewModeRubric={reviewModeRubric} keypadConfiguration={keypadConfiguration} + reviewMode={false} /> ); diff --git a/packages/perseus/src/widgets/group/group.tsx b/packages/perseus/src/widgets/group/group.tsx index d150ad025b..964598b64b 100644 --- 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 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 index 4b8a5d916f..ec06ef2e61 100644 --- 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 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 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 index 9e35f2b4ae..ec1678b1cf 100644 --- 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 index e78bcacc34..fd3026773d 100644 --- 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 index 8b21b6f65a..8121bc5692 100644 --- 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 ( { 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 ; @@ -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 ; diff --git a/packages/perseus/src/widgets/radio/base-radio.tsx b/packages/perseus/src/widgets/radio/base-radio.tsx index e69003b50c..5a9daf88b1 100644 --- 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 index 821e9af822..3d5817c532 100644 --- 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 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 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}