Skip to content

Commit

Permalink
PerseusItem: split and clarify top-level types (#1525)
Browse files Browse the repository at this point in the history
## Summary:

In Perseus, the type `PerseusItem` represents the top-level data structure that the two main Perseus question renderers accept: `ServerItemRenderer` and `MultiRenderer`. 

This PR formalizes the fact that, although we had a single type for both, they are really two completely separate types. For now, I've retained the concept of the `PerseusItem` type, but in reality, most (all?) places should be using the `StandardItem` for `ServerItemRenderer` and `MultiItem` for `MultiRenderer`.

Issue: "none"

## Test plan:

`yarn tsc`

Author: jeremywiebe

Reviewers: nishasy, handeyeco, jeremywiebe

Required Reviewers:

Approved By: nishasy, handeyeco

Checks: ✅ codecov/project, ❌ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (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), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1525
  • Loading branch information
jeremywiebe authored Aug 19, 2024
1 parent 2509713 commit 426a3ae
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 24 deletions.
6 changes: 6 additions & 0 deletions .changeset/blue-apes-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": major
"@khanacademy/perseus-editor": patch
---

Change PerseusItem to no longer include multi items
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export const PerseusItemWithRadioWidget = generateTestPerseusItem({
{content: "Hint #3", images: {}, widgets: {}},
],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
});
Expand Down
1 change: 0 additions & 1 deletion packages/perseus/src/__testdata__/graphie.testdata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,5 @@ export const itemWithPieChart: PerseusItem = {
},
},
},
_multi: null,
answer: null,
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const itemWithInput: PerseusItem = {
{content: "Hint #3", images: {}, widgets: {}},
],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Expand Down Expand Up @@ -79,7 +78,6 @@ export const itemWithMultipleInputNumbers: PerseusItem = {
{content: "Hint #3", images: {}, widgets: {}},
],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Expand Down Expand Up @@ -132,7 +130,6 @@ export const itemWithNumericAndNumberInputs: PerseusItem = {
{content: "Hint #3", images: {}, widgets: {}},
],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Expand Down Expand Up @@ -224,7 +221,6 @@ export const itemWithRadioAndExpressionWidgets: PerseusItem = {
{content: "Hint #3", images: {}, widgets: {}},
],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Expand All @@ -233,7 +229,6 @@ export const labelImageItem: PerseusItem = {
answerArea: Object.fromEntries(
ItemExtras.map((extra) => [extra, false]),
) as PerseusAnswerArea,
_multi: null,
answer: null,
hints: [],
itemDataVersion: {major: 0, minor: 1},
Expand Down Expand Up @@ -342,7 +337,6 @@ export const mockedItem: PerseusItem = {
} as PerseusRenderer,
hints: [],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Expand All @@ -355,7 +349,6 @@ export const itemWithLintingError: PerseusItem = {
},
hints: [],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Expand Down Expand Up @@ -548,7 +541,6 @@ And what follows are _hints_...
},
],
answerArea: null,
_multi: null,
itemDataVersion: {major: 0, minor: 0},
answer: null,
};
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const mockedAssetItem: PerseusItem = {
) as PerseusAnswerArea,
itemDataVersion: {major: 0, minor: 1},
hints: [],
_multi: null,
answer: null,
} as const;

Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ export type {
PerseusRenderer,
PerseusWidget,
PerseusWidgetsMap,
MultiItem,
} from "./perseus-types";
export type {Coord} from "./interactive2/types";
export type {MarkerType} from "./widgets/label-image/types";
Expand Down
19 changes: 17 additions & 2 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,36 @@ export type PerseusWidgetsMap = {
[key in `video ${number}`]: VideoWidget;
};

/**
* A "PerseusItem" is a classic Perseus item. It is rendered by the
* `ServerItemRenderer` and the layout is pre-set.
*
* To render more complex Perseus items, see the `Item` type in the multi item
* area.
*/
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<Hint>;
// 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
_multi: any;
// The version of the item. Not used by Perseus
itemDataVersion: Version;
// Deprecated field
answer: any;
};

/**
* A "MultiItem" is an advanced Perseus item. It is rendered by the
* `MultiRenderer` and you can control the layout of individual parts of the
* item.
*/
export type MultiItem = {
// Multi-item should only show up in Test Prep content and it is a variant of a PerseusItem
_multi: any;
};

export type PerseusArticle = ReadonlyArray<PerseusRenderer>;

export type Version = {
Expand Down
8 changes: 5 additions & 3 deletions packages/perseus/src/util/fix-passage-refs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import _ from "underscore";

import {traverse} from "../traversal";

import type {MultiItem, PerseusItem} from "../perseus-types";

const findPassageRefR = new RegExp(
// [[ passage-ref 1]]
// capture 1: widget markdown
Expand Down Expand Up @@ -101,8 +103,8 @@ const fixRendererPassageRefs = (options: any) => {
return traverse(options, null, fixRadioWidget, fixWholeOptions);
};

const FixPassageRefs = (itemData: any): any => {
if (itemData._multi) {
const fixPassageRefs = (itemData: PerseusItem | MultiItem): any => {
if ("_multi" in itemData) {
// We're in a multi-item. Don't do anything, just return the original
// item data.
return itemData;
Expand All @@ -118,4 +120,4 @@ const FixPassageRefs = (itemData: any): any => {
});
};

export default FixPassageRefs;
export default fixPassageRefs;
4 changes: 0 additions & 4 deletions packages/perseus/src/util/test-utils.testdata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const basicObject: PerseusItem = {
minor: 1,
},
hints: [],
_multi: null,
answer: null,
};

Expand Down Expand Up @@ -82,7 +81,6 @@ export const expectedQuestionInfoAdded: PerseusItem = {
minor: 1,
},
hints: [],
_multi: null,
answer: null,
};

Expand Down Expand Up @@ -122,7 +120,6 @@ export const expectedAnswerAreaInfoAdded: PerseusItem = {
minor: 1,
},
hints: [],
_multi: null,
answer: null,
};

Expand Down Expand Up @@ -214,6 +211,5 @@ export const expectedHintsInfoAdded: PerseusItem = {
},
},
],
_multi: null,
answer: null,
};
1 change: 0 additions & 1 deletion packages/perseus/src/util/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const genericPerseusItemData: PerseusItem = {
minor: 1,
},
hints: [],
_multi: null,
answer: null,
} as const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,14 @@ export const ShowTickControllerMobile = (
item={
{
question: question2,
_multi: null,
answer: null,
answerArea: null,
itemDataVersion: {
major: 0,
minor: 1,
},
hints: [],
} as PerseusItem
} satisfies PerseusItem
}
apiOptions={{
isMobile: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const createItemJson = (
},
},
},
_multi: null,
answer: null,
answerArea: Object.fromEntries(
ItemExtras.map((extra) => [extra, false]),
Expand Down

0 comments on commit 426a3ae

Please sign in to comment.