Skip to content

Commit de2883b

Browse files
authored
Hints: fix hints type on PerseusItem to be correct (#1489)
## Summary: Our `hints` type on `ItemRenderer` used the `PerseusRenderer` type. That's mostly correct, except that a Hint can also have the `replace` key. Issue: "none" ## Test plan: `yarn typecheck` ✅ Author: jeremywiebe Reviewers: jeremywiebe, benchristel Required Reviewers: Approved By: benchristel Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (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: #1489
1 parent 395b44f commit de2883b

File tree

10 files changed

+34
-31
lines changed

10 files changed

+34
-31
lines changed

.changeset/gentle-planes-rest.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@khanacademy/perseus": patch
3+
"@khanacademy/perseus-editor": patch
4+
---
5+
6+
Correct `hints` type on ItemRenderer

packages/perseus-editor/src/hint-editor.tsx

+6-13
Original file line numberDiff line numberDiff line change
@@ -318,14 +318,13 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
318318
silent: boolean,
319319
) => {
320320
// TODO(joel) - lens
321-
const hints = _(this.props.hints).clone();
321+
const hints = [...this.props.hints];
322322
hints[i] = _.extend(
323323
{},
324324
this.serializeHint(i, {keepDeletedWidgets: true}),
325325
newProps,
326326
);
327327

328-
// @ts-expect-error - TS2740 - Type 'Hint' is missing the following properties from type 'readonly Hint[]': length, concat, join, slice, and 18 more.
329328
this.props.onChange({hints: hints}, cb, silent);
330329
};
331330

@@ -335,23 +334,18 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
335334
return;
336335
}
337336

338-
const hints = _(this.props.hints).clone();
339-
// @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'.
337+
const hints = [...this.props.hints];
340338
hints.splice(i, 1);
341-
// @ts-expect-error - TS2322 - Type 'Hint' is not assignable to type 'readonly Hint[]'.
342339
this.props.onChange({hints: hints});
343340
};
344341

345342
handleHintMove: (i: number, dir: number) => void = (
346343
i: number,
347344
dir: number,
348345
) => {
349-
const hints = _(this.props.hints).clone();
350-
// @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'.
346+
const hints = [...this.props.hints];
351347
const hint = hints.splice(i, 1)[0];
352-
// @ts-expect-error - TS2339 - Property 'splice' does not exist on type 'Hint'.
353348
hints.splice(i + dir, 0, hint);
354-
// @ts-expect-error - TS2322 - Type 'Hint' is not assignable to type 'readonly Hint[]'.
355349
this.props.onChange({hints: hints}, () => {
356350
// eslint-disable-next-line react/no-string-refs
357351
// @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'.
@@ -360,10 +354,9 @@ class CombinedHintsEditor extends React.Component<CombinedHintsEditorProps> {
360354
};
361355

362356
addHint: () => void = () => {
363-
const hints = _(this.props.hints)
364-
.clone()
365-
// @ts-expect-error - TS2339 - Property 'concat' does not exist on type 'Hint'.
366-
.concat([{content: ""}]);
357+
const hints = this.props.hints.concat([
358+
{content: "", images: {}, widgets: {}},
359+
]);
367360
this.props.onChange({hints: hints}, () => {
368361
const i = hints.length - 1;
369362
// eslint-disable-next-line react/no-string-refs

packages/perseus/src/hints-renderer.tsx

+2-3
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@ import mediaQueries from "./styles/media-queries";
2020
import sharedStyles from "./styles/shared";
2121
import Util from "./util";
2222

23+
import type {Hint} from "./perseus-types";
2324
import type Renderer from "./renderer";
2425
import type {APIOptionsWithDefaults} from "./types";
2526
import type {PropsFor} from "@khanacademy/wonder-blocks-core";
2627

2728
type Props = PropsFor<typeof Renderer> & {
2829
className?: string;
29-
// note (mcurtis): I think this should be $ReadOnlyArray<PerseusRenderer>,
30-
// but things spiraled out of control when I tried to change it
31-
hints: ReadonlyArray<any>;
30+
hints: ReadonlyArray<Hint>;
3231
hintsVisible?: number;
3332
};
3433

packages/perseus/src/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@ export type {
152152
DomInsertCheckFn,
153153
EditorMode,
154154
FocusPath,
155-
Hint,
156155
ImageDict,
157156
ImageUploader,
158157
JiptLabelStore,
@@ -167,6 +166,7 @@ export type {
167166
} from "./types";
168167
export type {ParsedValue} from "./util";
169168
export type {
169+
Hint,
170170
LockedFigure,
171171
LockedFigureColor,
172172
LockedFigureFillType,

packages/perseus/src/multi-items/multi-renderer.tsx

+8
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,14 @@ class MultiRenderer extends React.Component<Props, State> {
319319
<HintsRenderer
320320
{...this._getRendererProps()}
321321
findExternalWidgets={findExternalWidgets}
322+
// Note(Jeremy): The MultiRenderer codebase types are
323+
// slightly different from the rest of the codebase.
324+
// Ideally `HintNode` would spread the `Hint` type into it,
325+
// but there is a difference in optionality of `widgets`
326+
// and `images` and that causes a huge cascade of type
327+
// errors (they are truly optional, but most of our code
328+
// assumes they're provided/set). Ignoring this for now.
329+
// @ts-expect-error - TS2769 - Type 'HintNode' is not assignable to type 'Hint'.
322330
hints={[hint]}
323331
/>
324332
),

packages/perseus/src/perseus-types.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export type PerseusItem = {
8787
// The details of the question being asked to the user.
8888
question: PerseusRenderer;
8989
// A collection of hints to be offered to the user that support answering the question.
90-
hints: ReadonlyArray<PerseusRenderer>;
90+
hints: ReadonlyArray<Hint>;
9191
// Details about the tools the user might need to answer the question
9292
answerArea: PerseusAnswerArea | null | undefined;
9393
// Multi-item should only show up in Test Prep content and it is a variant of a PerseusItem
@@ -116,8 +116,6 @@ export type PerseusRenderer = {
116116
content: string;
117117
// A dictionary of {[widgetName]: Widget} to be referenced from the content field
118118
widgets: PerseusWidgetsMap;
119-
// 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.
120-
replace?: boolean;
121119
// Used in the PerseusGradedGroup widget. A list of "tags" that are keys that represent other content in the system. Not rendered to the user.
122120
// NOTE: perseus_data.go says this is required even though it isn't necessary.
123121
metadata?: ReadonlyArray<string>;
@@ -127,6 +125,15 @@ export type PerseusRenderer = {
127125
};
128126
};
129127

128+
export type Hint = PerseusRenderer & {
129+
/**
130+
* When `true`, causes the previous hint to be replaced with this hint when
131+
* displayed. When `false`, the previous hint remains visible when this one
132+
* is displayed. This allows for hints that build upon each other.
133+
*/
134+
replace?: boolean;
135+
};
136+
130137
export type PerseusImageDetail = {
131138
// The width of the image
132139
width: number;

packages/perseus/src/types.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import type {ILogger} from "./logging/log";
22
import type {Item} from "./multi-items/item-types";
33
import type {
4+
Hint,
45
PerseusAnswerArea,
5-
PerseusRenderer,
66
PerseusWidget,
77
PerseusWidgetsMap,
88
} from "./perseus-types";
@@ -47,10 +47,6 @@ export type PerseusScore =
4747
message?: string | null | undefined;
4848
};
4949

50-
export type Hint = PerseusRenderer & {
51-
replace?: boolean;
52-
};
53-
5450
export type Version = {
5551
major: number;
5652
minor: number;

packages/perseus/src/widgets/__testdata__/grapher.testdata.ts

-2
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ export const quadraticQuestion: PerseusRenderer = {
218218
content:
219219
"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]]",
220220
images: {},
221-
replace: false,
222221
widgets: {
223222
"grapher 1": {
224223
alignment: "default",
@@ -268,7 +267,6 @@ export const sinusoidQuestion: PerseusRenderer = {
268267
content:
269268
"###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 ",
270269
images: {},
271-
replace: false,
272270
widgets: {
273271
"grapher 3": {
274272
alignment: "default",

packages/perseus/src/widgets/__testdata__/interaction.testdata.ts

-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ export const question1: PerseusRenderer = {
44
content:
55
"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.* ",
66
images: {},
7-
replace: false,
87
widgets: {
98
"interaction 1": {
109
alignment: "default",

packages/perseus/src/widgets/__testdata__/video.testdata.ts

-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ export const question1: PerseusRenderer = {
1717
alignment: "block",
1818
},
1919
},
20-
replace: false,
2120
};
2221

2322
export const question2: PerseusRenderer = {
@@ -37,7 +36,6 @@ export const question2: PerseusRenderer = {
3736
alignment: "block",
3837
},
3938
},
40-
replace: false,
4139
};
4240

4341
export const question3: PerseusRenderer = {
@@ -57,5 +55,4 @@ export const question3: PerseusRenderer = {
5755
alignment: "block",
5856
},
5957
},
60-
replace: false,
6158
};

0 commit comments

Comments
 (0)