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

Input Number Deprecation and Conversion into Numeric Input #1731

Merged
merged 34 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ffa8953
Initial Prototype for testing on webapp
SonicScrewdriver Oct 7, 2024
d37c982
only register the widget once in the list
SonicScrewdriver Oct 7, 2024
222fa9e
Undoing some of the initial test changes and bringing the widget back…
SonicScrewdriver Oct 8, 2024
3de001e
Returning the export for temporary linter appeasement
SonicScrewdriver Oct 8, 2024
575f433
Missed an export
SonicScrewdriver Oct 8, 2024
34a6ad3
Further linter appeasements
SonicScrewdriver Oct 8, 2024
89cc6bc
Bringing 'er around again
SonicScrewdriver Oct 8, 2024
4f83433
Fixing up the editor side of things.
SonicScrewdriver Oct 9, 2024
9306c0e
Added tests, moved logic to utils folder, fixed prop injection bug, a…
SonicScrewdriver Oct 30, 2024
453c5e5
Snapshots + minor clean up
SonicScrewdriver Oct 30, 2024
e631e78
Seems the Article Editor uses a different component
SonicScrewdriver Oct 30, 2024
20717fe
Missed a spot
SonicScrewdriver Oct 30, 2024
99d6fdf
Article works but it's ugly. Also cleaned up the utils
SonicScrewdriver Oct 31, 2024
22c07ea
Could we possibly export the convert util to webapp to improve this?
SonicScrewdriver Oct 31, 2024
05aa578
changeset
SonicScrewdriver Oct 31, 2024
9e73b3a
Now we're scoping nested widgets properly!
SonicScrewdriver Nov 1, 2024
b55035e
Cleaning up some earlier changes, better organization of the moderniz…
SonicScrewdriver Nov 5, 2024
cbeeb24
MASSIVE cleanup
SonicScrewdriver Nov 5, 2024
6dcfe33
Removing input-number test rather than updating.
SonicScrewdriver Nov 5, 2024
641cdad
Removing old snapshot
SonicScrewdriver Nov 6, 2024
efa04eb
Fixing the state for articles
SonicScrewdriver Nov 6, 2024
3063116
Fixing keys
SonicScrewdriver Nov 6, 2024
717e7d9
Updating class
SonicScrewdriver Nov 6, 2024
9b7b956
Fixing a bug
SonicScrewdriver Nov 7, 2024
0f8e4d0
Fixing tests after fixing bug
SonicScrewdriver Nov 7, 2024
987f34f
Addressing PR Feedback + Bugfix for LEMS-2613
SonicScrewdriver Nov 13, 2024
142cc3e
Undoing export
SonicScrewdriver Nov 13, 2024
fa335c0
Removing new code added for deprecated widget to unblock PR
SonicScrewdriver Nov 13, 2024
31d1397
Undoing this one file but keeping the tests deleted.
SonicScrewdriver Nov 13, 2024
079fcfa
Addressing PR Feedback + Fixing merge conflict with deleted input-num…
SonicScrewdriver Nov 14, 2024
3cccf4e
Removing this sweet lil' guy now that QA testing is complete
SonicScrewdriver Nov 14, 2024
c4e93af
Whoops this is not necessary any more
SonicScrewdriver Nov 14, 2024
1fc4ea3
Updated snapshots
SonicScrewdriver Nov 14, 2024
3f29d8a
Whoops, we missed bringing over the logic upgrades for answerforms.
SonicScrewdriver Nov 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/rude-adults-lie.md
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
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
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
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
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,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Perhaps this could use nullish coalescence?

    _sections(): PerseusRenderer[] {
        return Array.isArray(this.state.json)
            ? (this.state.json ?? [])
            : [this.state.json];
    };

}

_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
Loading