From ed428439c9fa5282e0f4d630887ff9fc401425a2 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 6 Aug 2024 10:45:39 -0700 Subject: [PATCH 1/6] Hints: fix hints type on PerseusItem to be correct --- packages/perseus-editor/src/hint-editor.tsx | 19 ++++++------------- packages/perseus/src/index.ts | 2 +- packages/perseus/src/perseus-types.ts | 6 +++++- packages/perseus/src/types.ts | 6 +----- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/packages/perseus-editor/src/hint-editor.tsx b/packages/perseus-editor/src/hint-editor.tsx index 37f32f86b4..05e3f21c6b 100644 --- a/packages/perseus-editor/src/hint-editor.tsx +++ b/packages/perseus-editor/src/hint-editor.tsx @@ -318,14 +318,13 @@ class CombinedHintsEditor extends React.Component { silent: boolean, ) => { // TODO(joel) - lens - const hints = _(this.props.hints).clone(); + const hints = [...this.props.hints]; hints[i] = _.extend( {}, this.serializeHint(i, {keepDeletedWidgets: true}), newProps, ); - // @ts-expect-error - TS2740 - Type 'Hint' is missing the following properties from type 'readonly Hint[]': length, concat, join, slice, and 18 more. this.props.onChange({hints: hints}, cb, silent); }; @@ -335,10 +334,8 @@ class CombinedHintsEditor extends React.Component { return; } - const hints = _(this.props.hints).clone(); - // @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'. + const hints = [...this.props.hints]; hints.splice(i, 1); - // @ts-expect-error - TS2322 - Type 'Hint' is not assignable to type 'readonly Hint[]'. this.props.onChange({hints: hints}); }; @@ -346,12 +343,9 @@ class CombinedHintsEditor extends React.Component { i: number, dir: number, ) => { - const hints = _(this.props.hints).clone(); - // @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'. + const hints = [...this.props.hints]; const hint = hints.splice(i, 1)[0]; - // @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'. hints.splice(i + dir, 0, hint); - // @ts-expect-error - TS2322 - Type 'Hint' is not assignable to type 'readonly Hint[]'. this.props.onChange({hints: hints}, () => { // eslint-disable-next-line react/no-string-refs // @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'. @@ -360,10 +354,9 @@ class CombinedHintsEditor extends React.Component { }; addHint: () => void = () => { - const hints = _(this.props.hints) - .clone() - // @ts-expect-error - TS2339 - Property 'concat' does not exist on type 'Hint'. - .concat([{content: ""}]); + const hints = [...this.props.hints].concat([ + {content: "", images: {}, widgets: {}}, + ]); this.props.onChange({hints: hints}, () => { const i = hints.length - 1; // eslint-disable-next-line react/no-string-refs diff --git a/packages/perseus/src/index.ts b/packages/perseus/src/index.ts index 9829ead8f5..8f2b540f61 100644 --- a/packages/perseus/src/index.ts +++ b/packages/perseus/src/index.ts @@ -152,7 +152,6 @@ export type { DomInsertCheckFn, EditorMode, FocusPath, - Hint, ImageDict, ImageUploader, JiptLabelStore, @@ -167,6 +166,7 @@ export type { } from "./types"; export type {ParsedValue} from "./util"; export type { + Hint, LockedFigure, LockedFigureColor, LockedFigureFillType, diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 85d2d94111..7ccc3bd723 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -87,7 +87,7 @@ export type PerseusItem = { // The details of the question being asked to the user. question: PerseusRenderer; // A collection of hints to be offered to the user that support answering the question. - hints: ReadonlyArray; + hints: ReadonlyArray; // Details about the tools the user might need to answer the question answerArea: PerseusAnswerArea | null | undefined; // Multi-item should only show up in Test Prep content and it is a variant of a PerseusItem @@ -127,6 +127,10 @@ export type PerseusRenderer = { }; }; +export type Hint = PerseusRenderer & { + replace?: boolean; +}; + export type PerseusImageDetail = { // The width of the image width: number; diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index 1145f4798a..f81efe70f4 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -1,8 +1,8 @@ import type {ILogger} from "./logging/log"; import type {Item} from "./multi-items/item-types"; import type { + Hint, PerseusAnswerArea, - PerseusRenderer, PerseusWidget, PerseusWidgetsMap, } from "./perseus-types"; @@ -47,10 +47,6 @@ export type PerseusScore = message?: string | null | undefined; }; -export type Hint = PerseusRenderer & { - replace?: boolean; -}; - export type Version = { major: number; minor: number; From c28bd409c8731d8ef5f321af5867255902fe6195 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 6 Aug 2024 10:47:41 -0700 Subject: [PATCH 2/6] Explain 'replace' key with comment --- packages/perseus/src/perseus-types.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 7ccc3bd723..854d4d3f88 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -128,6 +128,11 @@ export type PerseusRenderer = { }; export type Hint = PerseusRenderer & { + /** + * When `true`, causes the previous hint to be replaced with this hint when + * displayed. When `false`, the previous hint remains visible when this one + * is displayed. + */ replace?: boolean; }; From ce7be3747efe7ad9cee61438bd6098053295c1af Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 6 Aug 2024 10:48:09 -0700 Subject: [PATCH 3/6] Changeset --- .changeset/gentle-planes-rest.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/gentle-planes-rest.md diff --git a/.changeset/gentle-planes-rest.md b/.changeset/gentle-planes-rest.md new file mode 100644 index 0000000000..d12b4eeb20 --- /dev/null +++ b/.changeset/gentle-planes-rest.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": patch +"@khanacademy/perseus-editor": patch +--- + +Correct `hints` type on ItemRenderer From 03165cc93160b5762710435af1d80e699fd10d0b Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 6 Aug 2024 10:54:52 -0700 Subject: [PATCH 4/6] Fix hint type on HintsRenderer --- packages/perseus/src/hints-renderer.tsx | 5 ++--- packages/perseus/src/multi-items/multi-renderer.tsx | 8 ++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/perseus/src/hints-renderer.tsx b/packages/perseus/src/hints-renderer.tsx index 7e4248d537..8cf5145039 100644 --- a/packages/perseus/src/hints-renderer.tsx +++ b/packages/perseus/src/hints-renderer.tsx @@ -20,15 +20,14 @@ import mediaQueries from "./styles/media-queries"; import sharedStyles from "./styles/shared"; import Util from "./util"; +import type {Hint} from "./perseus-types"; import type Renderer from "./renderer"; import type {APIOptionsWithDefaults} from "./types"; import type {PropsFor} from "@khanacademy/wonder-blocks-core"; type Props = PropsFor & { className?: string; - // note (mcurtis): I think this should be $ReadOnlyArray, - // but things spiraled out of control when I tried to change it - hints: ReadonlyArray; + hints: ReadonlyArray; hintsVisible?: number; }; diff --git a/packages/perseus/src/multi-items/multi-renderer.tsx b/packages/perseus/src/multi-items/multi-renderer.tsx index f20f5f6615..f29e28d655 100644 --- a/packages/perseus/src/multi-items/multi-renderer.tsx +++ b/packages/perseus/src/multi-items/multi-renderer.tsx @@ -319,6 +319,14 @@ class MultiRenderer extends React.Component { ), From 3752db8279f5947e8136d0adc1df90293218273e Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 6 Aug 2024 14:20:04 -0700 Subject: [PATCH 5/6] Remove 'replace' key from PerseusRenderer (specific to hints, not all renderers) --- packages/perseus/src/perseus-types.ts | 4 +--- packages/perseus/src/widgets/__testdata__/grapher.testdata.ts | 2 -- .../perseus/src/widgets/__testdata__/interaction.testdata.ts | 1 - packages/perseus/src/widgets/__testdata__/video.testdata.ts | 3 --- 4 files changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 854d4d3f88..4684fbf199 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -116,8 +116,6 @@ export type PerseusRenderer = { content: string; // A dictionary of {[widgetName]: Widget} to be referenced from the content field widgets: PerseusWidgetsMap; - // Used only for PerseusItem.hints. If true, it replaces the previous hint in the list with the current one. This allows for hints that build upon each other. - replace?: boolean; // Used in the PerseusGradedGroup widget. A list of "tags" that are keys that represent other content in the system. Not rendered to the user. // NOTE: perseus_data.go says this is required even though it isn't necessary. metadata?: ReadonlyArray; @@ -131,7 +129,7 @@ export type Hint = PerseusRenderer & { /** * When `true`, causes the previous hint to be replaced with this hint when * displayed. When `false`, the previous hint remains visible when this one - * is displayed. + * is displayed. This allows for hints that build upon each other. */ replace?: boolean; }; diff --git a/packages/perseus/src/widgets/__testdata__/grapher.testdata.ts b/packages/perseus/src/widgets/__testdata__/grapher.testdata.ts index ff9e0b8277..9a6ab0e705 100644 --- a/packages/perseus/src/widgets/__testdata__/grapher.testdata.ts +++ b/packages/perseus/src/widgets/__testdata__/grapher.testdata.ts @@ -218,7 +218,6 @@ export const quadraticQuestion: PerseusRenderer = { content: "In conclusion, the vertex of the parabola is at\n\n$(3,-8)$\n\nand the zeros are\n\n$(5,0)$ and $(1,0)$\n\nIn order to graph, we need the vertex and another point. That other point can be one of the zeros we found, like $(1,0)$:\n\n[[☃ grapher 1]]", images: {}, - replace: false, widgets: { "grapher 1": { alignment: "default", @@ -268,7 +267,6 @@ export const sinusoidQuestion: PerseusRenderer = { content: "###The answer\n\nWe found that the graph of $y=-4\\cos\\left(x\\right)+3$ has a minimum point at $(0,-1)$ and then intersects its midline at $\\left(\\dfrac{1}{2}\\pi,3\\right)$.\n\n[[☃ grapher 3]]\n ", images: {}, - replace: false, widgets: { "grapher 3": { alignment: "default", diff --git a/packages/perseus/src/widgets/__testdata__/interaction.testdata.ts b/packages/perseus/src/widgets/__testdata__/interaction.testdata.ts index 91c2e187c3..03e8edbacc 100644 --- a/packages/perseus/src/widgets/__testdata__/interaction.testdata.ts +++ b/packages/perseus/src/widgets/__testdata__/interaction.testdata.ts @@ -4,7 +4,6 @@ export const question1: PerseusRenderer = { content: "Drag the dot all the way to the right.\n\n[[☃ interaction 1]]\n\n\n*Notice that we add a zero to the empty place value.* ", images: {}, - replace: false, widgets: { "interaction 1": { alignment: "default", diff --git a/packages/perseus/src/widgets/__testdata__/video.testdata.ts b/packages/perseus/src/widgets/__testdata__/video.testdata.ts index 36b2578d42..6e8a33125e 100644 --- a/packages/perseus/src/widgets/__testdata__/video.testdata.ts +++ b/packages/perseus/src/widgets/__testdata__/video.testdata.ts @@ -17,7 +17,6 @@ export const question1: PerseusRenderer = { alignment: "block", }, }, - replace: false, }; export const question2: PerseusRenderer = { @@ -37,7 +36,6 @@ export const question2: PerseusRenderer = { alignment: "block", }, }, - replace: false, }; export const question3: PerseusRenderer = { @@ -57,5 +55,4 @@ export const question3: PerseusRenderer = { alignment: "block", }, }, - replace: false, }; From 0a0fa72d1ab00895aead438043c8035e481bdcb2 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 6 Aug 2024 14:21:36 -0700 Subject: [PATCH 6/6] Don't need to clone the array before using `concat()`. It creates a new array already. Co-authored-by: Ben Christel --- packages/perseus-editor/src/hint-editor.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/perseus-editor/src/hint-editor.tsx b/packages/perseus-editor/src/hint-editor.tsx index 05e3f21c6b..c9fa8fc345 100644 --- a/packages/perseus-editor/src/hint-editor.tsx +++ b/packages/perseus-editor/src/hint-editor.tsx @@ -354,7 +354,7 @@ class CombinedHintsEditor extends React.Component { }; addHint: () => void = () => { - const hints = [...this.props.hints].concat([ + const hints = this.props.hints.concat([ {content: "", images: {}, widgets: {}}, ]); this.props.onChange({hints: hints}, () => {