Skip to content

Commit

Permalink
Input Number Deprecation and Conversion into Numeric Input (#1731)
Browse files Browse the repository at this point in the history 
## Summary:
This PR contains all the work necessary to deprecate our Input Number Widget, and convert it into the Numeric Input widget (sans actually deleting the Input Number Files, which will be handled in LEMS-2411).

The changes include: 

- Using replaceWidget/Editor to change the rendering of Input Number widgets to Numeric Input 
- New conversion util functions to automatically convert the json on the Editor pages for pre-existing Input Numbers in content, as an effort to avoid a backfill. 
- Cleanup of MANY tests across the repo that used Input Number for their test data 
- New PropUpgrades function for Numeric Input that converts Input Number widgetOptions into Numeric Input's format.

NOTE: There is a temporary wrapper div around the Numeric Input component in order to help QA be able to determine that the widget has been converted, as InputNumber/NumericInput are virtually identical otherwise. This will be removed prior to landing the ticket, and after QA testing has completed.  

Issue: LEMS-2408 & LEMS-2409

## Test plan:
- Extensive Manual Testing 
- New Jest Tests 
- QA Testing Round

Author: SonicScrewdriver

Reviewers: SonicScrewdriver, mark-fitzgerald, #perseus

Required Reviewers:

Approved By: mark-fitzgerald

Checks: ✅ Publish npm snapshot (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), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1731
  • Loading branch information
@SonicScrewdriver
SonicScrewdriver authored Nov 14, 2024
1 parent 8eb1ff5 commit 27126aa
Show file tree
Hide file tree
Showing 43 changed files with 2,782 additions and 2,476 deletions.
6 changes: 6 additions & 0 deletions .changeset/rude-adults-lie.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

Conversion of Input Number to Numeric Input
40 changes: 38 additions & 2 deletions packages/perseus-editor/src/__stories__/article-editor.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,50 @@ import {useRef, useState} from "react";
import ArticleEditor from "../article-editor";
import {registerAllWidgetsAndEditorsForTesting} from "../util/register-all-widgets-and-editors-for-testing";

import type {PerseusRenderer} from "@khanacademy/perseus";

registerAllWidgetsAndEditorsForTesting();

export default {
title: "PerseusEditor/ArticleEditor",
};

const articleSectionWithInputNumber: PerseusRenderer = {
content:
"### Practice Problem\n\n$8\\cdot(11i+2)=$ [[☃ input-number 1]]. Also [[☃ input-number 2]] \n*.*",
images: {},
widgets: {
"input-number 1": {
type: "input-number",
graded: true,
alignment: "default",
options: {
maxError: 0.1,
inexact: false,
value: 0.4,
simplify: "optional",
answerType: "rational",
size: "normal",
},
version: {major: 1, minor: 0},
},
"input-number 2": {
type: "input-number",
graded: true,
alignment: "default",
options: {
maxError: 0.1,
inexact: false,
value: 0.5,
simplify: "optional",
answerType: "rational",
size: "normal",
},
version: {major: 1, minor: 0},
},
},
};
export const Base = (): React.ReactElement => {
const [state, setState] = useState();
const [state, setState] = useState(articleSectionWithInputNumber);
const articleEditorRef = useRef();

function handleChange(value) {
Expand Down
38 changes: 38 additions & 0 deletions packages/perseus-editor/src/__stories__/editor-page.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,50 @@ import {registerAllWidgetsAndEditorsForTesting} from "../util/register-all-widge

import EditorPageWithStorybookPreview from "./editor-page-with-storybook-preview";

import type {InputNumberWidget, PerseusRenderer} from "@khanacademy/perseus";

registerAllWidgetsAndEditorsForTesting(); // SIDE_EFFECTY!!!! :cry:

export default {
title: "PerseusEditor/EditorPage",
};

const question1: PerseusRenderer = {
content:
"Denis baked a peach pie and cut it into $3$ equal-sized pieces. Denis's dad eats $1$ section of the pie. \n\n**What fraction of the pie did Denis's dad eat?** \n![](https://ka-perseus-graphie.s3.amazonaws.com/74a2b7583a2c26ebfb3ad714e29867541253fc97.png) \n[[\u2603 input-number 1]] \n\n\n\n",
images: {
"https://ka-perseus-graphie.s3.amazonaws.com/74a2b7583a2c26ebfb3ad714e29867541253fc97.png":
{
width: 200,
height: 200,
},
},
widgets: {
"input-number 1": {
version: {
major: 0,
minor: 0,
},
type: "input-number",
graded: true,
alignment: "default",
options: {
maxError: 0.1,
inexact: false,
value: 0.5,
simplify: "optional",
answerType: "rational",
size: "normal",
},
} as InputNumberWidget,
},
};

export const Demo = (): React.ReactElement => {
return <EditorPageWithStorybookPreview />;
};

// Used to test that Input Numbers are being automatically converted to Numeric Inputs
export const InputNumberDemo = (): React.ReactElement => {
return <EditorPageWithStorybookPreview question={question1} />;
};
37 changes: 22 additions & 15 deletions packages/perseus-editor/src/__tests__/traversal.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,31 @@ const missingOptions = {
const clonedMissingOptions = JSON.parse(JSON.stringify(missingOptions));

const sampleOptions = {
content: "[[☃ input-number 1]]",
content: "[[☃ numeric-input 1]]",
images: {},
widgets: {
"input-number 1": {
type: "input-number",
"numeric-input 1": {
type: "numeric-input",
graded: true,
static: false,
options: {
value: "0",
simplify: "required",
answers: [
{
value: 0,
status: "correct",
message: "",
simplify: "required",
strict: true,
maxError: 0.1,
},
],
size: "normal",
inexact: false,
maxError: 0.1,
answerType: "number",
coefficient: false,
labelText: "",
rightAlign: false,
},
static: false,
version: {
major: 0,
major: 1,
minor: 0,
},
alignment: "default",
Expand Down Expand Up @@ -258,7 +265,7 @@ describe("Traversal", () => {
readContent = content;
});

expect(readContent).toBe("[[☃ input-number 1]]");
expect(readContent).toBe("[[☃ numeric-input 1]]");
assertNonMutative();
});

Expand All @@ -280,7 +287,7 @@ describe("Traversal", () => {
widgetMap[widgetInfo.type] = (widgetMap[widgetInfo.type] || 0) + 1;
});
expect(widgetMap).toEqual({
"input-number": 1,
"numeric-input": 1,
});
assertNonMutative();
});
Expand All @@ -294,9 +301,9 @@ describe("Traversal", () => {
expect(newOptions).toEqual(
_.extend({}, sampleOptions, {
widgets: {
"input-number 1": _.extend(
"numeric-input 1": _.extend(
{},
sampleOptions.widgets["input-number 1"],
sampleOptions.widgets["numeric-input 1"],
{graded: false},
),
},
Expand All @@ -312,7 +319,7 @@ describe("Traversal", () => {
});
});
expect(newOptions.content).toBe(
"[[☃ input-number 1]]\n\nnew content!",
"[[☃ numeric-input 1]]\n\nnew content!",
);
expect(newOptions.widgets).toEqual(sampleOptions.widgets);
assertNonMutative();
Expand Down
59 changes: 32 additions & 27 deletions packages/perseus-editor/src/article-editor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ import {
iconCircleArrowUp,
iconPlus,
} from "./styles/icon-paths";
import {convertDeprecatedWidgets} from "./util/deprecated-widgets/modernize-widgets-utils";

import type {APIOptions, Changeable, ImageUploader} from "@khanacademy/perseus";
import type {
APIOptions,
Changeable,
ImageUploader,
PerseusRenderer,
} from "@khanacademy/perseus";

const {HUD, InlineIcon} = components;

type RendererProps = {
content?: string;
widgets?: any;
images?: any;
};

type JsonType = RendererProps | ReadonlyArray<RendererProps>;
type JsonType = PerseusRenderer | PerseusRenderer[];
type DefaultProps = {
contentPaths?: ReadonlyArray<string>;
json: JsonType;
Expand All @@ -50,20 +50,27 @@ type Props = DefaultProps & {

type State = {
highlightLint: boolean;
json: JsonType;
};
export default class ArticleEditor extends React.Component<Props, State> {
static defaultProps: DefaultProps = {
contentPaths: [],
json: [{}],
json: [],
mode: "edit",
screen: "desktop",
sectionImageUploadGenerator: () => <span />,
useNewStyles: false,
};

state: State = {
highlightLint: true,
};
constructor(props: Props) {
super(props);
this.state = {
highlightLint: true,
json: Array.isArray(props.json)
? props.json.map(convertDeprecatedWidgets)
: convertDeprecatedWidgets(props.json as PerseusRenderer),
};
}

componentDidMount() {
this._updatePreviewFrames();
Expand Down Expand Up @@ -95,7 +102,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
}
}

_apiOptionsForSection(section: RendererProps, sectionIndex: number): any {
_apiOptionsForSection(section: PerseusRenderer, sectionIndex: number): any {
// eslint-disable-next-line react/no-string-refs
const editor = this.refs[`editor${sectionIndex}`];
return {
Expand All @@ -120,10 +127,13 @@ export default class ArticleEditor extends React.Component<Props, State> {
};
}

_sections(): ReadonlyArray<RendererProps> {
return Array.isArray(this.props.json)
? this.props.json
: [this.props.json];
_sections(): PerseusRenderer[] {
const sections = Array.isArray(this.state.json)
? this.state.json
: [this.state.json];
return sections.filter(
(section): section is PerseusRenderer => section !== null,
);
}

_renderEditor(): React.ReactElement<React.ComponentProps<"div">> {
Expand Down Expand Up @@ -297,13 +307,13 @@ export default class ArticleEditor extends React.Component<Props, State> {
this.props.onChange({json: newJson});
};

_handleEditorChange: (i: number, newProps: RendererProps) => void = (
_handleEditorChange: (i: number, newProps: PerseusRenderer) => void = (
i,
newProps,
) => {
const sections = _.clone(this._sections());
// @ts-expect-error - TS2542 - Index signature in type 'readonly RendererProps[]' only permits reading.
sections[i] = _.extend({}, sections[i], newProps);
this.setState({json: sections});
this.props.onChange({json: sections});
};

Expand All @@ -313,9 +323,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
}
const sections = _.clone(this._sections());
const section = sections[i];
// @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'?
sections.splice(i, 1);
// @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'?
sections.splice(i - 1, 0, section);
this.props.onChange({
json: sections,
Expand All @@ -328,9 +336,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
return;
}
const section = sections[i];
// @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'?
sections.splice(i, 1);
// @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'?
sections.splice(i + 1, 0, section);
this.props.onChange({
json: sections,
Expand Down Expand Up @@ -362,7 +368,6 @@ export default class ArticleEditor extends React.Component<Props, State> {

_handleRemoveSection(i: number) {
const sections = _.clone(this._sections());
// @ts-expect-error - TS2551 - Property 'splice' does not exist on type 'readonly RendererProps[]'. Did you mean 'slice'?
sections.splice(i, 1);
this.props.onChange({
json: sections,
Expand All @@ -378,7 +383,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
});
}
if (this.props.mode === "preview" || this.props.mode === "json") {
return this.props.json;
return this.state.json;
}
throw new PerseusError(
"Could not serialize; mode " + this.props.mode + " not found",
Expand All @@ -392,7 +397,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
*
* This function can currently only be called in edit mode.
*/
getSaveWarnings(): ReadonlyArray<RendererProps> {
getSaveWarnings(): ReadonlyArray<PerseusRenderer> {
if (this.props.mode !== "edit") {
// TODO(joshuan): We should be able to get save warnings in
// preview mode.
Expand Down Expand Up @@ -427,7 +432,7 @@ export default class ArticleEditor extends React.Component<Props, State> {
<JsonEditor
multiLine={true}
onChange={this._handleJsonChange}
value={this.props.json}
value={this.state.json}
/>
</div>
)}
Expand Down
Loading

0 comments on commit 27126aa

Please sign in to comment.