Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremywiebe committed Jul 25, 2024
1 parent 9e66968 commit 39e40f1
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 116 deletions.
84 changes: 14 additions & 70 deletions packages/perseus-editor/src/editor-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
ImageUploader,
Version,
PerseusItem,
PerseusRenderer,
} from "@khanacademy/perseus";
import type {KEScore} from "@khanacademy/perseus-core";

Expand All @@ -30,14 +31,14 @@ type Props = {
developerMode: boolean;
// Source HTML for the iframe to render
frameSource: string;
hints?: ReadonlyArray<Hint>; // related to the question,
hints: ReadonlyArray<Hint>; // related to the question,
// A function which takes a file object (guaranteed to be an image) and
// a callback, then calls the callback with the url where the image
// will be hosted. Image drag and drop is disabled when imageUploader
// is null.
imageUploader?: ImageUploader;
// Part of the question
itemDataVersion?: Version;
itemDataVersion: Version;
// The content ID of the AssessmentItem being edited.
itemId: string;
// Whether the question is displaying as JSON or if it is
Expand All @@ -49,7 +50,7 @@ type Props = {
onPreviewDeviceChange: (arg1: DeviceType) => unknown;
previewDevice: DeviceType;
// Initial value of the question being edited
question?: any;
question: PerseusRenderer;
// URL of the route to show on initial load of the preview frames.
previewURL: string;
};
Expand Down Expand Up @@ -82,14 +83,16 @@ class EditorPage extends React.Component<Props, State> {
super(props);

this.state = {
// @ts-expect-error - TS2322 - Type 'Pick<Readonly<Props> & Readonly<{ children?: ReactNode; }>, "hints" | "question" | "answerArea" | "itemDataVersion">' is not assignable to type 'PerseusJson'.
json: _.pick(
this.props,
"question",
"answerArea",
"hints",
"itemDataVersion",
),
json: {
question: this.props.question,
answerArea: this.props.answerArea,
hints: this.props.hints,
itemDataVersion: this.props.itemDataVersion,

// Deprecated keys
_multi: null,
answer: null,
},
gradeMessage: "",
wasAnswered: false,
highlightLint: true,
Expand All @@ -102,21 +105,6 @@ class EditorPage extends React.Component<Props, State> {
// TODO(scottgrant): This is a hack to remove the deprecated call to
// this.isMounted() but is still considered an anti-pattern.
this._isMounted = true;

this.updateRenderer();
}

componentDidUpdate() {
// NOTE: It is required to delay the preview update until after the
// current frame, to allow for ItemEditor to render its widgets.
// This then enables to serialize the widgets properties correctly,
// in order to send data to the preview iframe (IframeContentRenderer).
// Otherwise, widgets will render in an "empty" state in the preview.
// TODO(jeff, CP-3128): Use Wonder Blocks Timing API
// eslint-disable-next-line no-restricted-syntax
setTimeout(() => {
this.updateRenderer();
});
}

componentWillUnmount() {
Expand All @@ -136,50 +124,6 @@ class EditorPage extends React.Component<Props, State> {
);
};

updateRenderer() {
// Some widgets (namely the image widget) like to call onChange before
// anything has actually been mounted, which causes problems here. We
// just ensure don't update until we've mounted
const hasEditor = !this.props.developerMode || !this.props.jsonMode;
if (!this._isMounted || !hasEditor) {
return;
}

const touch =
this.props.previewDevice === "phone" ||
this.props.previewDevice === "tablet";
const deviceBasedApiOptions: APIOptionsWithDefaults = {
...this.getApiOptions(),
customKeypad: touch,
isMobile: touch,
};

this.itemEditor.current?.triggerPreviewUpdate({
type: "question",
data: _({
item: this.serialize(),
apiOptions: deviceBasedApiOptions,
initialHintsVisible: 0,
device: this.props.previewDevice,
linterContext: {
contentType: "exercise",
highlightLint: this.state.highlightLint,
// TODO(CP-4838): is it okay to use [] as a default?
paths: this.props.contentPaths || [],
},
reviewMode: true,
legacyPerseusLint: this.itemEditor.current?.getSaveWarnings(),
}).extend(
_(this.props).pick(
"workAreaSelector",
"solutionAreaSelector",
"hintsAreaSelector",
"problemNum",
),
),
});
}

getApiOptions(): APIOptionsWithDefaults {
return {
...ApiOptions.defaults,
Expand Down
50 changes: 5 additions & 45 deletions packages/perseus-editor/src/hint-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import * as React from "react";
import _ from "underscore";

import DeviceFramer from "./components/device-framer";
import ContentRenderer from "./content-renderer";
import Editor from "./editor";
import IframeContentRenderer from "./iframe-content-renderer";

import type {
APIOptions,
Expand Down Expand Up @@ -188,35 +188,6 @@ class CombinedHintEditor extends React.Component<CombinedHintEditorProps> {
};

editor = React.createRef<HintEditor>();
frame = React.createRef<IframeContentRenderer>();

componentDidMount() {
this.updatePreview();
}

componentDidUpdate() {
this.updatePreview();
}

updatePreview = () => {
const shouldBold =
this.props.isLast && !/\*\*/.test(this.props.hint.content);

this.frame.current?.sendNewData({
type: "hint",
data: {
hint: this.props.hint,
bold: shouldBold,
pos: this.props.pos,
apiOptions: this.props.apiOptions,
linterContext: {
contentType: "hint",
highlightLint: this.props.highlightLint,
paths: this.props.contentPaths,
},
},
});
};

getSaveWarnings = () => {
return this.editor.current?.getSaveWarnings();
Expand Down Expand Up @@ -262,12 +233,9 @@ class CombinedHintEditor extends React.Component<CombinedHintEditorProps> {
deviceType={this.props.deviceType}
nochrome={true}
>
<IframeContentRenderer
ref={this.frame}
datasetKey="mobile"
datasetValue={isMobile}
seamless={true}
url={this.props.previewURL}
<ContentRenderer
question={this.props.hint}
apiOptions={{isMobile}}
/>
</DeviceFramer>
</div>
Expand Down Expand Up @@ -409,7 +377,7 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
const {itemId, hints} = this.props;
const hintElems = _.map(
hints,
function (hint, i) {
(hint, i) => {
return (
<CombinedHintEditor
ref={"hintEditor" + i}
Expand All @@ -419,24 +387,16 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
itemId={itemId}
hint={hint}
pos={i}
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
imageUploader={this.props.imageUploader}
// eslint-disable-next-line react/jsx-no-bind
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation. | TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
onChange={this.handleHintChange.bind(this, i)}
// eslint-disable-next-line react/jsx-no-bind
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation. | TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
onRemove={this.handleHintRemove.bind(this, i)}
// eslint-disable-next-line react/jsx-no-bind
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation. | TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
onMove={this.handleHintMove.bind(this, i)}
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
deviceType={this.props.deviceType}
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
apiOptions={this.props.apiOptions}
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
highlightLint={this.props.highlightLint}
// @ts-expect-error - TS2683 - 'this' implicitly has type 'any' because it does not have a type annotation.
previewURL={this.props.previewURL}
// TODO(CP-4838): what should be passed here?
contentPaths={[]}
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export type PerseusItem = {
// A collection of hints to be offered to the user that support answering the question.
hints: ReadonlyArray<PerseusRenderer>;
// Details about the tools the user might need to answer the question
answerArea: PerseusAnswerArea | null | undefined;
answerArea: PerseusAnswerArea;
// Multi-item should only show up in Test Prep content and it is a variant of a PerseusItem
_multi: any;
// The version of the item. Not used by Perseus
Expand Down

0 comments on commit 39e40f1

Please sign in to comment.