From 326e8f7cac65c22638afdc5ab4bcc2765d8ba17d Mon Sep 17 00:00:00 2001 From: Khan Actions Bot <56267880+khan-actions-bot@users.noreply.github.com> Date: Mon, 9 Dec 2024 18:11:33 -0500 Subject: [PATCH 1/7] Update browserslist (#1970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary Updates the `browserslist` and `caniuse-lite` npm packages ## Reviewing notes: There should only be changes to the `yarn.lock` file in this PR. Check that there is only 1 `caniuse-lite` package reference in the `yarn.lock` file (the intent of this update is to ensure that `caniuse-lite` is on the latest version and that there aren't multiple, conflicting versions that different tools might see). If everything looks fine, please approve this PR and then land it (either with the Big Green Merge Button ™️ or by using `git land ` in a terminal). Author: khan-actions-bot Reviewers: anakaren-rojas, #frontend-infra-web Required Reviewers: Approved By: anakaren-rojas Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/1970 --- package.json | 3 ++- yarn.lock | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 0b39930026..d873e8d270 100644 --- a/package.json +++ b/package.json @@ -151,5 +151,6 @@ }, "nyc": { "report-dir": "coverage/cypress/" - } + }, + "dependencies": {} } diff --git a/yarn.lock b/yarn.lock index 8e94626cfb..2ba3f5266c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5138,9 +5138,9 @@ caniuse-api@^3.0.0: lodash.uniq "^4.5.0" caniuse-lite@^1.0.0, caniuse-lite@^1.0.30001332, caniuse-lite@^1.0.30001541, caniuse-lite@^1.0.30001587, caniuse-lite@^1.0.30001669: - version "1.0.30001685" - resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001685.tgz" - integrity sha512-e/kJN1EMyHQzgcMEEgoo+YTCO1NGCmIYHk5Qk8jT6AazWemS5QFKJ5ShCJlH3GZrNIdZofcNCEwZqbMjjKzmnA== + version "1.0.30001687" + resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001687.tgz" + integrity sha512-0S/FDhf4ZiqrTUiQ39dKeUjYRjkv7lOZU1Dgif2rIqrTzX/1wV2hfKu9TOm1IHkdSijfLswxTFzl/cvir+SLSQ== caseless@~0.12.0: version "0.12.0" From 2ad163b5ea20d40fb9f0edf30e03cd54ecf9bf31 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 10 Dec 2024 12:02:19 -0800 Subject: [PATCH 2/7] [Locked Figures Aria] Update the auto-generated text to spell out commas (#1976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: All our interactive graph aria labels are spelling out commas in coordinate pairs for clarity. When using a coordinate pair as is, the screen reader may skip the comma and read ambiguously. Example: (1000, 975) ==> "one thousand nine hundred seventy-five" (sounds like 1975) To fix this issue and be consistent with all the other interactive graph aria labels, the locked labels' autogeneration should also spell out commas. - Spell out commas in locked figures' auto-generated aria label text Issue: none ## Test plan: `yarn jest --silent` Storybook - Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags - Go through all the different locked figures and click "auto-generate" - Note that their coordinates should all spell out the comma within the auto-generated text - Do this for point, line, vector, ellipse, and polygon (not function or label) | Before | After | | --- | --- | | ![image](https://github.com/user-attachments/assets/f010757e-a070-4b96-8b79-a8bb38d01d84) | ![image](https://github.com/user-attachments/assets/2cd27821-7f47-41da-a371-0208aaae208f) | Author: nishasy Reviewers: anakaren-rojas, catandthemachines Required Reviewers: Approved By: anakaren-rojas Checks: ✅ Publish npm snapshot (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), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/1976 --- .changeset/seven-planets-matter.md | 5 +++++ .../locked-figures/locked-ellipse-settings.test.tsx | 12 ++++++------ .../locked-figures/locked-ellipse-settings.tsx | 2 +- .../locked-figures/locked-line-settings.test.tsx | 12 ++++++------ .../locked-figures/locked-line-settings.tsx | 2 +- .../locked-figures/locked-point-settings.test.tsx | 6 +++--- .../locked-figures/locked-point-settings.tsx | 2 +- .../locked-figures/locked-polygon-settings.test.tsx | 6 +++--- .../locked-figures/locked-polygon-settings.tsx | 2 +- .../locked-figures/locked-vector-settings.test.tsx | 6 +++--- .../locked-figures/locked-vector-settings.tsx | 2 +- 11 files changed, 31 insertions(+), 26 deletions(-) create mode 100644 .changeset/seven-planets-matter.md diff --git a/.changeset/seven-planets-matter.md b/.changeset/seven-planets-matter.md new file mode 100644 index 0000000000..555866ebc5 --- /dev/null +++ b/.changeset/seven-planets-matter.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-editor": patch +--- + +[Locked Figures Aria] Update the auto-generated text to spell out commas diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx index 056ab99f53..5f20a34d33 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx @@ -393,7 +393,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Circle with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle with radius 2, centered at 0 comma 0. Appearance solid gray border, with no fill.", }); }); @@ -421,7 +421,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Circle with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle with radius 2, centered at 0 comma 0. Appearance solid gray border, with no fill.", }); }); @@ -448,7 +448,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Ellipse with x radius 2 and y radius 3, centered at (0, 0). Appearance solid gray border, with no fill.", + "Ellipse with x radius 2 and y radius 3, centered at 0 comma 0. Appearance solid gray border, with no fill.", }); }); @@ -476,7 +476,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Ellipse with x radius 2 and y radius 3, centered at (0, 0), rotated by 90 degrees. Appearance solid gray border, with no fill.", + "Ellipse with x radius 2 and y radius 3, centered at 0 comma 0, rotated by 90 degrees. Appearance solid gray border, with no fill.", }); }); @@ -508,7 +508,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Circle spoken A with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle spoken A with radius 2, centered at 0 comma 0. Appearance solid gray border, with no fill.", }); }); @@ -544,7 +544,7 @@ describe("LockedEllipseSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Circle spoken A, spoken B with radius 2, centered at (0, 0). Appearance solid gray border, with no fill.", + "Circle spoken A, spoken B with radius 2, centered at 0 comma 0. Appearance solid gray border, with no fill.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx index 48d7066586..91ec4936c4 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx @@ -79,7 +79,7 @@ const LockedEllipseSettings = (props: Props) => { str += `Ellipse${visiblelabel} with x radius ${radius[0]} and y radius ${radius[1]}`; } - str += `, centered at (${center[0]}, ${center[1]})`; + str += `, centered at ${center[0]} comma ${center[1]}`; if (!isCircle && angle !== 0) { str += `, rotated by ${radianToDegree(angle)} degrees`; diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx index 0d444b82e3..44f40c0183 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx @@ -625,7 +625,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Segment from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Segment from point at 0 comma 0 to point at 2 comma 2. Appearance solid gray.", }); }); @@ -651,7 +651,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Line from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Line from point at 0 comma 0 to point at 2 comma 2. Appearance solid gray.", }); }); @@ -682,7 +682,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Line spoken A from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Line spoken A from point at 0 comma 0 to point at 2 comma 2. Appearance solid gray.", }); }); @@ -717,7 +717,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Line spoken A, spoken B from point at (0, 0) to point at (2, 2). Appearance solid gray.", + "Line spoken A, spoken B from point at 0 comma 0 to point at 2 comma 2. Appearance solid gray.", }); }); @@ -758,7 +758,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Line spoken A from point spoken C at (0, 0) to point spoken D at (2, 2). Appearance solid gray.", + "Line spoken A from point spoken C at 0 comma 0 to point spoken D at 2 comma 2. Appearance solid gray.", }); }); @@ -809,7 +809,7 @@ describe("LockedLineSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Line spoken A, spoken B from point spoken C, spoken C2 at (0, 0) to point spoken D, spoken D2 at (2, 2). Appearance solid gray.", + "Line spoken A, spoken B from point spoken C, spoken C2 at 0 comma 0 to point spoken D, spoken D2 at 2 comma 2. Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx index 982375bd88..da368b5f42 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx @@ -83,7 +83,7 @@ const LockedLineSettings = (props: Props) => { const point1VisibleLabel = await joinLabelsAsSpokenMath(point1.labels); const point2VisibleLabel = await joinLabelsAsSpokenMath(point2.labels); - let str = `${capitalizeKind}${visiblelabel} from point${point1VisibleLabel} at (${point1.coord[0]}, ${point1.coord[1]}) to point${point2VisibleLabel} at (${point2.coord[0]}, ${point2.coord[1]})`; + let str = `${capitalizeKind}${visiblelabel} from point${point1VisibleLabel} at ${point1.coord[0]} comma ${point1.coord[1]} to point${point2VisibleLabel} at ${point2.coord[0]} comma ${point2.coord[1]}`; const lineAppearance = generateLockedFigureAppearanceDescription( lineColor, diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx index 8f772e4ed6..026d873042 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx @@ -422,7 +422,7 @@ describe("LockedPointSettings", () => { // generateSpokenMathDetails is mocked to return the input string // with "Spoken math details for " prepended. expect(onChangeProps).toHaveBeenCalledWith({ - ariaLabel: "Point at (0, 0). Appearance solid gray.", + ariaLabel: "Point at 0 comma 0. Appearance solid gray.", }); }); @@ -454,7 +454,7 @@ describe("LockedPointSettings", () => { // generateSpokenMathDetails is mocked to return the input string // with "Spoken math details for " prepended. expect(onChangeProps).toHaveBeenCalledWith({ - ariaLabel: "Point spoken A at (0, 0). Appearance solid gray.", + ariaLabel: "Point spoken A at 0 comma 0. Appearance solid gray.", }); }); @@ -491,7 +491,7 @@ describe("LockedPointSettings", () => { // with "Spoken math details for " prepended. expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Point spoken A, spoken B at (0, 0). Appearance solid gray.", + "Point spoken A, spoken B at 0 comma 0. Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx index bceb0c2ddf..757ad272f9 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx @@ -119,7 +119,7 @@ const LockedPointSettings = (props: Props) => { async function getPrepopulatedAriaLabel() { const visiblelabel = await joinLabelsAsSpokenMath(labels); - let str = `Point${visiblelabel} at (${coord[0]}, ${coord[1]})`; + let str = `Point${visiblelabel} at ${coord[0]} comma ${coord[1]}`; const pointAppearance = generateLockedFigureAppearanceDescription(pointColor); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx index 62c3dd0a0a..35d485736a 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx @@ -608,7 +608,7 @@ describe("LockedPolygonSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Polygon with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", + "Polygon with 3 sides, vertices at 0 comma 0, 0 comma 1, 1 comma 1. Appearance solid gray border, with no fill.", }); }); @@ -644,7 +644,7 @@ describe("LockedPolygonSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Polygon spoken A with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", + "Polygon spoken A with 3 sides, vertices at 0 comma 0, 0 comma 1, 1 comma 1. Appearance solid gray border, with no fill.", }); }); @@ -684,7 +684,7 @@ describe("LockedPolygonSettings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Polygon spoken A, spoken B with 3 sides, vertices at (0, 0), (0, 1), (1, 1). Appearance solid gray border, with no fill.", + "Polygon spoken A, spoken B with 3 sides, vertices at 0 comma 0, 0 comma 1, 1 comma 1. Appearance solid gray border, with no fill.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx index e5b82f114c..5619442737 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx @@ -75,7 +75,7 @@ const LockedPolygonSettings = (props: Props) => { let str = `Polygon${visiblelabel} with ${points.length} sides, vertices at `; // Add the coordinates of each point to the aria label - str += points.map(([x, y]) => `(${x}, ${y})`).join(", "); + str += points.map(([x, y]) => `${x} comma ${y}`).join(", "); const polygonAppearance = generateLockedFigureAppearanceDescription( color, diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx index 1c52f82937..fe208e4aa5 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx @@ -441,7 +441,7 @@ describe("Locked Vector Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Vector from (0, 0) to (2, 2). Appearance solid gray.", + "Vector from 0 comma 0 to 2 comma 2. Appearance solid gray.", }); }); @@ -472,7 +472,7 @@ describe("Locked Vector Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Vector spoken A from (0, 0) to (2, 2). Appearance solid gray.", + "Vector spoken A from 0 comma 0 to 2 comma 2. Appearance solid gray.", }); }); @@ -507,7 +507,7 @@ describe("Locked Vector Settings", () => { // Assert expect(onChangeProps).toHaveBeenCalledWith({ ariaLabel: - "Vector spoken A, spoken B from (0, 0) to (2, 2). Appearance solid gray.", + "Vector spoken A, spoken B from 0 comma 0 to 2 comma 2. Appearance solid gray.", }); }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx index 9fee83fc04..2ae3247bc1 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx @@ -72,7 +72,7 @@ const LockedVectorSettings = (props: Props) => { async function getPrepopulatedAriaLabel() { const visiblelabel = await joinLabelsAsSpokenMath(labels); - let str = `Vector${visiblelabel} from (${tail[0]}, ${tail[1]}) to (${tip[0]}, ${tip[1]})`; + let str = `Vector${visiblelabel} from ${tail[0]} comma ${tail[1]} to ${tip[0]} comma ${tip[1]}`; const vectorAppearance = generateLockedFigureAppearanceDescription(lineColor); From 06c08df2b42053d705fea52e9f439bcc1bea53fe Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 10 Dec 2024 20:05:33 +0000 Subject: [PATCH 3/7] Version Packages --- .changeset/poor-numbers-reflect.md | 5 ----- .changeset/seven-planets-matter.md | 5 ----- packages/perseus-editor/CHANGELOG.md | 9 +++++++++ packages/perseus-editor/package.json | 4 ++-- packages/perseus/CHANGELOG.md | 6 ++++++ packages/perseus/package.json | 2 +- 6 files changed, 18 insertions(+), 13 deletions(-) delete mode 100644 .changeset/poor-numbers-reflect.md delete mode 100644 .changeset/seven-planets-matter.md diff --git a/.changeset/poor-numbers-reflect.md b/.changeset/poor-numbers-reflect.md deleted file mode 100644 index 4830ff7a54..0000000000 --- a/.changeset/poor-numbers-reflect.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -[Numeric Input] - BUGFIX - Adjust color contrast of tooltip text diff --git a/.changeset/seven-planets-matter.md b/.changeset/seven-planets-matter.md deleted file mode 100644 index 555866ebc5..0000000000 --- a/.changeset/seven-planets-matter.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus-editor": patch ---- - -[Locked Figures Aria] Update the auto-generated text to spell out commas diff --git a/packages/perseus-editor/CHANGELOG.md b/packages/perseus-editor/CHANGELOG.md index 91941befa8..d87e89c266 100644 --- a/packages/perseus-editor/CHANGELOG.md +++ b/packages/perseus-editor/CHANGELOG.md @@ -1,5 +1,14 @@ # @khanacademy/perseus-editor +## 15.1.4 + +### Patch Changes + +- [#1976](https://github.com/Khan/perseus/pull/1976) [`2ad163b5e`](https://github.com/Khan/perseus/commit/2ad163b5ea20d40fb9f0edf30e03cd54ecf9bf31) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figures Aria] Update the auto-generated text to spell out commas + +- Updated dependencies [[`e22a931d9`](https://github.com/Khan/perseus/commit/e22a931d987291258b66f2c80db3536970a4555d)]: + - @khanacademy/perseus@46.0.1 + ## 15.1.3 ### Patch Changes diff --git a/packages/perseus-editor/package.json b/packages/perseus-editor/package.json index 70db62a16c..bd710e8794 100644 --- a/packages/perseus-editor/package.json +++ b/packages/perseus-editor/package.json @@ -3,7 +3,7 @@ "description": "Perseus editors", "author": "Khan Academy", "license": "MIT", - "version": "15.1.3", + "version": "15.1.4", "publishConfig": { "access": "public" }, @@ -38,7 +38,7 @@ "@khanacademy/keypad-context": "^1.0.4", "@khanacademy/kmath": "^0.1.16", "@khanacademy/math-input": "^21.1.6", - "@khanacademy/perseus": "^46.0.0", + "@khanacademy/perseus": "^46.0.1", "@khanacademy/perseus-core": "1.5.3", "@khanacademy/pure-markdown": "^0.3.13", "mafs": "^0.19.0" diff --git a/packages/perseus/CHANGELOG.md b/packages/perseus/CHANGELOG.md index 4b2c5b0579..00b7002dc4 100644 --- a/packages/perseus/CHANGELOG.md +++ b/packages/perseus/CHANGELOG.md @@ -1,5 +1,11 @@ # @khanacademy/perseus +## 46.0.1 + +### Patch Changes + +- [#1966](https://github.com/Khan/perseus/pull/1966) [`e22a931d9`](https://github.com/Khan/perseus/commit/e22a931d987291258b66f2c80db3536970a4555d) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - [Numeric Input] - BUGFIX - Adjust color contrast of tooltip text + ## 46.0.0 ### Major Changes diff --git a/packages/perseus/package.json b/packages/perseus/package.json index e07e2d5110..70a785efe4 100644 --- a/packages/perseus/package.json +++ b/packages/perseus/package.json @@ -3,7 +3,7 @@ "description": "Core Perseus API (includes renderers and widgets)", "author": "Khan Academy", "license": "MIT", - "version": "46.0.0", + "version": "46.0.1", "publishConfig": { "access": "public" }, From e7b4db0bf193241a36508804dd6e58c729f0a3db Mon Sep 17 00:00:00 2001 From: Matthew Date: Wed, 11 Dec 2024 06:50:00 -0600 Subject: [PATCH 4/7] Remove MultiRenderer (#1955) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: GTP is dead and MultiRenderer existed for GTP. Removing it simplifies things quite a bit. Issue: [LEMS-2675](https://khanacademy.atlassian.net/browse/LEMS-2675) ## Test plan: Shouldn't affect anything, no longer in use [LEMS-2675]: https://khanacademy.atlassian.net/browse/LEMS-2675?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Author: handeyeco Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ .github/dependabot.yml Pull Request URL: https://github.com/Khan/perseus/pull/1955 --- .changeset/cool-taxis-clap.md | 6 + docs/architecture.md | 11 - .../perseus-editor/src/__tests__/i18n.test.ts | 501 ------- .../src/components/graph-settings.tsx | 2 - .../structured-item-diff.stories.tsx | 855 ------------ .../__tests__/structured-item-diff.test.ts | 433 ------ .../src/diffs/structured-item-diff.tsx | 275 ---- packages/perseus-editor/src/i18n.ts | 140 -- packages/perseus-editor/src/index.ts | 3 - .../src/multirenderer-editor.tsx | 1060 --------------- .../components/interactive-graph-settings.tsx | 2 - packages/perseus/src/__tests__/a11y.test.ts | 23 - .../src/__tests__/renderability.test.ts | 60 - packages/perseus/src/a11y.ts | 12 +- packages/perseus/src/index.ts | 28 - packages/perseus/src/multi-items.ts | 75 -- .../__stories__/multi-renderer.stories.tsx | 80 -- .../__testdata__/multi-renderer.testdata.ts | 176 --- .../multi-renderer.test.tsx.snap | 1181 ----------------- .../src/multi-items/__tests__/items.test.ts | 240 ---- .../__tests__/multi-renderer.test.tsx | 906 ------------- .../__tests__/prop-type-builders.test.ts | 133 -- .../src/multi-items/__tests__/shapes.test.ts | 43 - .../src/multi-items/__tests__/trees.test.ts | 317 ----- .../perseus/src/multi-items/item-types.ts | 57 - packages/perseus/src/multi-items/items.ts | 175 --- .../src/multi-items/multi-renderer.tsx | 584 -------- .../src/multi-items/prop-type-builders.ts | 74 -- .../perseus/src/multi-items/shape-types.ts | 52 - packages/perseus/src/multi-items/shapes.ts | 52 - .../perseus/src/multi-items/tree-types.ts | 47 - packages/perseus/src/multi-items/trees.ts | 359 ----- packages/perseus/src/perseus-types.ts | 13 - packages/perseus/src/renderability.ts | 18 - packages/perseus/src/types.ts | 3 - .../parse-perseus-json-snapshot.test.ts.snap | 28 - .../data/item-missing-answerArea.json | 16 - packages/perseus/src/widget-type-utils.ts | 2 - testing/multi-item-renderer-with-debug-ui.tsx | 76 -- 39 files changed, 7 insertions(+), 8111 deletions(-) create mode 100644 .changeset/cool-taxis-clap.md delete mode 100644 packages/perseus-editor/src/__tests__/i18n.test.ts delete mode 100644 packages/perseus-editor/src/diffs/__stories__/structured-item-diff.stories.tsx delete mode 100644 packages/perseus-editor/src/diffs/__tests__/structured-item-diff.test.ts delete mode 100644 packages/perseus-editor/src/diffs/structured-item-diff.tsx delete mode 100644 packages/perseus-editor/src/i18n.ts delete mode 100644 packages/perseus-editor/src/multirenderer-editor.tsx delete mode 100644 packages/perseus/src/multi-items.ts delete mode 100644 packages/perseus/src/multi-items/__stories__/multi-renderer.stories.tsx delete mode 100644 packages/perseus/src/multi-items/__testdata__/multi-renderer.testdata.ts delete mode 100644 packages/perseus/src/multi-items/__tests__/__snapshots__/multi-renderer.test.tsx.snap delete mode 100644 packages/perseus/src/multi-items/__tests__/items.test.ts delete mode 100644 packages/perseus/src/multi-items/__tests__/multi-renderer.test.tsx delete mode 100644 packages/perseus/src/multi-items/__tests__/prop-type-builders.test.ts delete mode 100644 packages/perseus/src/multi-items/__tests__/shapes.test.ts delete mode 100644 packages/perseus/src/multi-items/__tests__/trees.test.ts delete mode 100644 packages/perseus/src/multi-items/item-types.ts delete mode 100644 packages/perseus/src/multi-items/items.ts delete mode 100644 packages/perseus/src/multi-items/multi-renderer.tsx delete mode 100644 packages/perseus/src/multi-items/prop-type-builders.ts delete mode 100644 packages/perseus/src/multi-items/shape-types.ts delete mode 100644 packages/perseus/src/multi-items/shapes.ts delete mode 100644 packages/perseus/src/multi-items/tree-types.ts delete mode 100644 packages/perseus/src/multi-items/trees.ts delete mode 100644 packages/perseus/src/util/parse-perseus-json/regression-tests/data/item-missing-answerArea.json delete mode 100644 testing/multi-item-renderer-with-debug-ui.tsx diff --git a/.changeset/cool-taxis-clap.md b/.changeset/cool-taxis-clap.md new file mode 100644 index 0000000000..5cf5a872e1 --- /dev/null +++ b/.changeset/cool-taxis-clap.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": major +"@khanacademy/perseus-editor": major +--- + +Remove support for MultiRenderer diff --git a/docs/architecture.md b/docs/architecture.md index 9224910412..b57ea3ab44 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -55,17 +55,6 @@ with a short description and the main Perseus data type it accepts. article, but the scoring is not available outside of the renderer. - - MultiItemRenderer - { _multi: any } - - The `MultiItemRenderer` is a more advanced renderer. It accepts an - object representing a tree of `PerseusRenderer` objects. A `Shape` - object defines the structure of the tree and you must provide a - "callback" object of the same structure which defines how and where - to render each `PerseusRenderer` that appears in the tree. - - Renderer PerseusRenderer diff --git a/packages/perseus-editor/src/__tests__/i18n.test.ts b/packages/perseus-editor/src/__tests__/i18n.test.ts deleted file mode 100644 index d909731be4..0000000000 --- a/packages/perseus-editor/src/__tests__/i18n.test.ts +++ /dev/null @@ -1,501 +0,0 @@ -import {Dependencies} from "@khanacademy/perseus"; -import _ from "underscore"; - -import {testDependencies} from "../../../../testing/test-dependencies"; -import i18n from "../i18n"; -import {registerAllWidgetsAndEditorsForTesting} from "../util/register-all-widgets-and-editors-for-testing"; - -const exerciseImagesEverywhere = { - question: { - content: - "![question-a](question-a)\n\n![question-b](question-b)\n\n[[☃ categorizer 1]]\n\n[[☃ group 1]]\n\n[[☃ image 1]]\n\n[[☃ matcher 1]]\n\n[[☃ matrix 1]]\n\n[[☃ orderer 1]]\n\n[[☃ passage 1]]\n\n[[☃ radio 1]]\n\n[[☃ sorter 1]]\n\n[[☃ table 1]]\n\n[[☃ grapher 1]]\n\n[[☃ interactive-graph 1]]\n\n[[☃ measurer 1]]\n\n[[☃ plotter 1]]", - images: {}, - widgets: { - "categorizer 1": { - type: "categorizer", - graded: true, - options: { - items: [ - "![category-item-a](category-item-a)", - "![category-item-b](category-item-b)", - ], - categories: [ - "![category-category-a](category-category-a)", - "![category-category-b](category-category-b)", - ], - values: [], - randomizeItems: false, - }, - version: {major: 0, minor: 0}, - }, - "group 1": { - type: "group", - graded: true, - options: { - content: - "![group-a](group-a)\n\n![group-b](group-b)\n\n[[☃ image 1]]", - images: {}, - widgets: { - "image 1": { - type: "image", - graded: true, - options: { - title: "![group-image-title-a](group-image-title-a) ![group-image-title-b](group-image-title-b)", - range: [ - [0, 10], - [0, 10], - ], - box: [0, 0], - backgroundImage: { - url: "group-image-a", - width: 0, - height: 0, - }, - labels: [], - alt: "", - caption: - "![group-image-caption-a](group-image-caption-a) ![group-image-caption-b](group-image-caption-b)", - }, - version: {major: 0, minor: 0}, - }, - }, - }, - version: {major: 0, minor: 0}, - }, - "image 1": { - type: "image", - graded: true, - options: { - title: "![image-title-a](image-title-a) ![image-title-b](image-title-b)", - range: [ - [0, 10], - [0, 10], - ], - box: [0, 0], - backgroundImage: {url: "image-a", width: 0, height: 0}, - labels: [], - alt: "", - caption: - "![image-caption-a](image-caption-a) ![image-caption-b](image-caption-b)", - }, - version: {major: 0, minor: 0}, - }, - "matcher 1": { - type: "matcher", - graded: true, - options: { - left: [ - "![matcher-left-a](matcher-left-a)", - "![matcher-left-b](matcher-left-b)", - ], - right: [ - "![matcher-right-a](matcher-right-a)", - "![matcher-right-b](matcher-right-b)", - ], - labels: [ - "![matcher-label-a](matcher-label-a)", - "![matcher-label-b](matcher-label-b)", - ], - orderMatters: false, - padding: true, - }, - version: {major: 0, minor: 0}, - }, - "matrix 1": { - type: "matrix", - graded: true, - options: { - matrixBoardSize: [3, 3], - answers: [[]], - prefix: "![matrix-prefix-a](matrix-prefix-a) ![matrix-prefix-b](matrix-prefix-b)", - suffix: "![matrix-suffix-a](matrix-suffix-a) ![matrix-suffix-b](matrix-suffix-b)", - cursorPosition: [0, 0], - }, - version: {major: 0, minor: 0}, - }, - "orderer 1": { - type: "orderer", - graded: true, - options: { - options: [ - {content: "![orderer-correct-a](orderer-correct-a)"}, - {content: "![orderer-correct-b](orderer-correct-b)"}, - {content: "![orderer-other-a](orderer-other-a)"}, - {content: "![orderer-other-b](orderer-other-b)"}, - ], - correctOptions: [ - {content: "![orderer-correct-a](orderer-correct-a)"}, - {content: "![orderer-correct-b](orderer-correct-b)"}, - ], - otherOptions: [ - {content: "![orderer-other-a](orderer-other-a)"}, - {content: "![orderer-other-b](orderer-other-b)"}, - ], - height: "normal", - layout: "horizontal", - }, - version: {major: 0, minor: 0}, - }, - "passage 1": { - type: "passage", - graded: true, - options: { - passageTitle: - "![passage-title-a](passage-title-a) ![passage-title-b](passage-title-b)", - passageText: "", - footnotes: "", - showLineNumbers: true, - }, - version: {major: 0, minor: 0}, - }, - "radio 1": { - type: "radio", - graded: true, - options: { - choices: [ - { - content: - "![radio-choice1-a](radio-choice1-a) ![radio-choice1-b](radio-choice1-b)", - }, - { - content: - "![radio-choice2-a](radio-choice2-a) ![radio-choice2-b](radio-choice2-b)", - }, - ], - randomize: false, - multipleSelect: false, - displayCount: null, - hasNoneOfTheAbove: false, - onePerLine: true, - deselectEnabled: false, - }, - version: {major: 1, minor: 0}, - }, - "sorter 1": { - type: "sorter", - graded: true, - options: { - correct: [ - "![sorter-correct-a](sorter-correct-a)", - "![sorter-correct-b](sorter-correct-b)", - "![sorter-correct-c](sorter-correct-c)", - ], - layout: "horizontal", - padding: true, - }, - version: {major: 0, minor: 0}, - }, - "table 1": { - type: "table", - graded: true, - options: { - headers: [ - "![table-header-a](table-header-a)", - "![table-header-b](table-header-b)", - "![table-header-c](table-header-c)", - ], - rows: 4, - columns: 3, - answers: [ - ["", "", ""], - ["", "", ""], - ["", "", ""], - ["", "", ""], - ], - }, - version: {major: 0, minor: 0}, - }, - "grapher 1": { - type: "grapher", - graded: true, - options: { - correct: {type: "linear", coords: null, asymptote: null}, - availableTypes: ["linear"], - graph: { - editableSettings: ["graph", "snap", "image"], - range: [ - [-10, 10], - [-10, 10], - ], - labels: ["x", "y"], - step: [1, 1], - gridStep: [1, 1], - snapStep: [1, 1], - valid: true, - backgroundImage: { - url: "grapher-a", - width: 32, - height: 32, - }, - markings: "graph", - rulerLabel: "", - rulerTicks: 10, - showProtractor: false, - showRuler: false, - }, - }, - version: {major: 0, minor: 0}, - }, - "interactive-graph 1": { - type: "interactive-graph", - graded: true, - options: { - step: [1, 1], - backgroundImage: { - url: "interactive-graph-a", - width: 32, - height: 32, - }, - markings: "graph", - labels: ["x", "y"], - showProtractor: false, - showRuler: false, - rulerLabel: "", - rulerTicks: 10, - range: [ - [-10, 10], - [-10, 10], - ], - gridStep: [1, 1], - snapStep: [0.5, 0.5], - graph: {type: "linear"}, - correct: {type: "linear", coords: null}, - }, - version: {major: 0, minor: 0}, - }, - "measurer 1": { - type: "measurer", - graded: true, - options: { - box: [480, 480], - image: {url: "measurer-a"}, - showProtractor: true, - showRuler: false, - rulerLabel: "", - rulerTicks: 10, - rulerPixels: 40, - rulerLength: 10, - }, - version: {major: 1, minor: 0}, - }, - "plotter 1": { - type: "plotter", - graded: true, - options: { - correct: [1, 1], - starting: [1, 1], - type: "pic", - labels: ["", ""], - categories: ["a", "a"], - scaleY: 1, - maxY: 10, - snapsPerLine: 2, - labelInterval: 1, - picUrl: "plotter-a", - }, - version: {major: 0, minor: 0}, - }, - }, - }, - answerArea: {calculator: false}, - itemDataVersion: {major: 0, minor: 1}, - hints: [ - { - content: - "![hint1-a](hint1-a)\n\n![hint1-b](hint1-b)\n\n[[☃ image 1]]", - images: {}, - widgets: { - "image 1": { - type: "image", - graded: true, - options: { - title: "![hint1-image-title-a](hint1-image-title-a) ![hint1-image-title-b](hint1-image-title-b)", - range: [ - [0, 10], - [0, 10], - ], - box: [0, 0], - backgroundImage: { - url: "hint1-image-a", - width: 0, - height: 0, - }, - labels: [], - alt: "", - caption: - "![hint1-image-caption-a](hint1-image-caption-a) ![hint1-image-caption-b](hint1-image-caption-b)", - }, - version: {major: 0, minor: 0}, - }, - }, - }, - { - content: - "![hint2-a](hint2-a)\n\n![hint2-b](hint2-b)\n\n[[☃ image 1]]", - images: {}, - widgets: { - "image 1": { - type: "image", - graded: true, - options: { - title: "![hint2-image-title-a](hint2-image-title-a) ![hint2-image-title-b](hint2-image-title-b)", - range: [ - [0, 10], - [0, 10], - ], - box: [0, 0], - backgroundImage: { - url: "hint2-image-a", - width: 0, - height: 0, - }, - labels: [], - alt: "", - caption: - "![hint2-image-caption-a](hint2-image-caption-a) ![hint2-image-caption-b](hint2-image-caption-b)", - }, - version: {major: 0, minor: 0}, - }, - }, - }, - ], -} as const; - -// Article perseus format is very similar to exercises and should parse the -// same way, but should just be a list of renders. So we reuse the test data -// from exerciseImagesEverywhere, but include the question and hints as if they -// were paragraphs in the article. -const articleImagesEverywhere = [exerciseImagesEverywhere.question].concat( - // @ts-expect-error - TS2769 - No overload matches this call. - exerciseImagesEverywhere.hints, -); - -// All of the images that are in the `exerciseImagesEverywhere` item. Also the -// same list of images that are in the `articleImagesEverywhere` item. -const allImages = [ - "question-a", - "question-b", - "category-category-a", - "category-category-b", - "category-item-a", - "category-item-b", - "group-a", - "group-b", - "group-image-title-a", - "group-image-title-b", - "group-image-a", - "group-image-caption-a", - "group-image-caption-b", - "image-title-a", - "image-title-b", - "image-a", - "image-caption-a", - "image-caption-b", - "matcher-label-a", - "matcher-label-b", - "matcher-left-a", - "matcher-left-b", - "matcher-right-a", - "matcher-right-b", - "matrix-prefix-a", - "matrix-prefix-b", - "matrix-suffix-a", - "matrix-suffix-b", - "orderer-correct-a", - "orderer-correct-b", - "orderer-other-a", - "orderer-other-b", - "passage-title-a", - "passage-title-b", - "radio-choice1-a", - "radio-choice1-b", - "radio-choice2-a", - "radio-choice2-b", - "sorter-correct-a", - "sorter-correct-b", - "sorter-correct-c", - "table-header-a", - "table-header-b", - "table-header-c", - "grapher-a", - "interactive-graph-a", - "measurer-a", - "plotter-a", - "hint1-a", - "hint1-a", - "hint1-image-title-a", - "hint1-image-title-b", - "hint1-image-a", - "hint1-image-caption-a", - "hint1-image-caption-b", - "hint2-a", - "hint2-a", - "hint2-image-title-a", - "hint2-image-title-b", - "hint2-image-a", - "hint2-image-caption-a", - "hint2-image-caption-b", -]; - -describe("i18n", () => { - beforeAll(() => { - registerAllWidgetsAndEditorsForTesting(); - }); - - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); - }); - - describe("Exercise image finding", () => { - it("should find all of the images in items", () => { - const foundImages = i18n.findImagesInItemData( - exerciseImagesEverywhere, - ); - - expect(foundImages.length <= allImages.length).toBeTruthy(); - - _.each(allImages, (image) => { - expect(foundImages.indexOf(image) !== -1).toBeTruthy(); - }); - }); - }); - - describe("Article image finding", () => { - it("should find all of the images in items", () => { - const foundImages = i18n.findImagesInArticles( - articleImagesEverywhere, - ); - - expect(foundImages.length <= allImages.length).toBeTruthy(); - - _.each(allImages, (image) => { - expect(foundImages.indexOf(image) !== -1).toBeTruthy(); - }); - }); - }); - - describe("Multi-item image finding", () => { - it("should find all of the images in each leaf node", () => { - const foundImages = i18n.findImagesInItemData({ - _multi: { - question: { - __type: "content", - ...exerciseImagesEverywhere.question, - }, - hints: exerciseImagesEverywhere.hints.map((hint) => ({ - __type: "hint", - ...hint, - })), - }, - }); - - expect(foundImages.length <= allImages.length).toBeTruthy(); - - _.each(allImages, (image) => { - expect(foundImages.indexOf(image) !== -1).toBeTruthy(); - }); - }); - }); -}); diff --git a/packages/perseus-editor/src/components/graph-settings.tsx b/packages/perseus-editor/src/components/graph-settings.tsx index 483033dba5..27bc39778b 100644 --- a/packages/perseus-editor/src/components/graph-settings.tsx +++ b/packages/perseus-editor/src/components/graph-settings.tsx @@ -92,8 +92,6 @@ const GraphSettings = createReactClass({ }, UNSAFE_componentWillReceiveProps: function (nextProps) { - // Make sure that state updates when switching - // between different items in a multi-item editor. if ( !_.isEqual(this.props.labels, nextProps.labels) || !_.isEqual(this.props.gridStep, nextProps.gridStep) || diff --git a/packages/perseus-editor/src/diffs/__stories__/structured-item-diff.stories.tsx b/packages/perseus-editor/src/diffs/__stories__/structured-item-diff.stories.tsx deleted file mode 100644 index eaa4d79b78..0000000000 --- a/packages/perseus-editor/src/diffs/__stories__/structured-item-diff.stories.tsx +++ /dev/null @@ -1,855 +0,0 @@ -import * as React from "react"; - -import StructuredItemDiff from "../structured-item-diff"; - -import Wrapper from "./perseus-diff-wrapper"; - -import("../../styles/perseus-editor.less"); - -type StoryArgs = Record; - -type Story = { - title: string; - decorators: ReadonlyArray< - (StoryComponent: typeof React.Component) => React.ReactElement - >; -}; - -export default { - title: "PerseusEditor/Diffs/Structured Item Diff", - decorators: [ - (StoryComponent) => ( - - - - ), - ], -} as Story; - -const tags = { - a: "a tag", - b: "b tag", - c: "c tag", -} as const; - -export const ContentAdded = (args: StoryArgs): React.ReactElement => { - const props = { - before: { - _multi: { - directions: { - __type: "content", - content: "", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "", - images: {}, - widgets: {}, - }, - hints: [], - questions: [], - }, - }, - after: { - _multi: { - directions: { - type: "content", - content: "directions", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "passage", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - hints: [ - { - type: "hint", - content: "hint 1", - images: {}, - widgets: {}, - }, - ], - questions: [ - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - ], - }, - }, - shape: { - type: "object", - shape: { - directions: { - type: "content", - }, - overview: { - type: "content", - }, - passage: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - questions: { - type: "array", - elementShape: { - type: "object", - shape: { - tags: {type: "tags"}, - question: { - type: "content", - }, - overview: { - type: "content", - }, - keepInMind: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - }, - }, - }, - }, - }, - tags: { - idToName: (id) => tags[id], - nameToId: (name) => name[0], - names: ["a tag", "b tag", "c tag"], - }, - } as const; - - return ; -}; - -// second instance -export const ContentAddedRemovedAndChanged = ( - args: StoryArgs, -): React.ReactElement => { - const props = { - before: { - _multi: { - directions: { - __type: "content", - content: "directions", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "passage", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - hints: [ - { - type: "hint", - content: "hint 1", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "hint 2", - images: {}, - widgets: {}, - }, - ], - questions: [ - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - ], - }, - }, - after: { - _multi: { - directions: { - type: "content", - content: "directions", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "passage", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - hints: [ - { - type: "hint", - content: "hint 1", - images: {}, - widgets: {}, - }, - ], - questions: [ - { - hints: [], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question edited", - images: {}, - widgets: {}, - }, - tags: ["a", "c"], - }, - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - ], - }, - }, - shape: { - type: "object", - shape: { - directions: { - type: "content", - }, - overview: { - type: "content", - }, - passage: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - questions: { - type: "array", - elementShape: { - type: "object", - shape: { - tags: {type: "tags"}, - question: { - type: "content", - }, - overview: { - type: "content", - }, - keepInMind: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - }, - }, - }, - }, - }, - tags: { - idToName: (id) => tags[id], - nameToId: (name) => name[0], - names: ["a tag", "b tag", "c tag"], - }, - } as const; - return ; -}; - -// third instance -export const MiscContentChanges = (args: StoryArgs): React.ReactElement => { - const props = { - before: { - _multi: { - directions: { - __type: "content", - content: "directions", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "passage", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - hints: [ - { - type: "hint", - content: "hint 1", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "hint 2", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "hint 3", - images: {}, - widgets: {}, - }, - ], - questions: [ - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "question hint 2", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "question hint 3", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question edited", - images: {}, - widgets: {}, - }, - tags: ["a", "c"], - }, - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - ], - }, - }, - after: { - _multi: { - directions: { - __type: "content", - content: "directions", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "passage", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - hints: [ - { - type: "hint", - content: "hint 1", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "hint 2", - images: {}, - widgets: {}, - }, - ], - questions: [ - { - hints: [ - { - type: "hint", - content: "question hint 1 edited", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question edited", - images: {}, - widgets: {}, - }, - tags: ["a", "c"], - }, - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - ], - }, - }, - shape: { - type: "object", - shape: { - directions: { - type: "content", - }, - overview: { - type: "content", - }, - passage: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - questions: { - type: "array", - elementShape: { - type: "object", - shape: { - tags: {type: "tags"}, - question: { - type: "content", - }, - overview: { - type: "content", - }, - keepInMind: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - }, - }, - }, - }, - }, - tags: { - idToName: (id) => tags[id], - nameToId: (name) => name[0], - names: ["a tag", "b tag", "c tag"], - }, - } as const; - - return ; -}; - -// fourth -export const ContentRemoved = (args: StoryArgs): React.ReactElement => { - const props = { - before: { - _multi: { - directions: { - __type: "content", - content: "directions", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "passage", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - hints: [ - { - type: "hint", - content: "hint 1", - images: {}, - widgets: {}, - }, - { - type: "hint", - content: "hint 2", - images: {}, - widgets: {}, - }, - ], - questions: [ - { - hints: [ - { - type: "hint", - content: "question hint 1 edited", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question edited", - images: {}, - widgets: {}, - }, - tags: ["a", "c"], - }, - { - hints: [ - { - type: "hint", - content: "question hint 1", - images: {}, - widgets: {}, - }, - ], - keepInMind: { - type: "content", - content: "keep in mind", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "overview", - images: {}, - widgets: {}, - }, - question: { - type: "content", - content: "question", - images: {}, - widgets: {}, - }, - tags: ["a", "b"], - }, - ], - }, - }, - after: { - _multi: { - directions: { - __type: "content", - content: "", - images: {}, - widgets: {}, - }, - passage: { - type: "content", - content: "", - images: {}, - widgets: {}, - }, - overview: { - type: "content", - content: "", - images: {}, - widgets: {}, - }, - hints: [], - questions: [], - }, - }, - shape: { - type: "object", - shape: { - directions: { - type: "content", - }, - overview: { - type: "content", - }, - passage: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - questions: { - type: "array", - elementShape: { - type: "object", - shape: { - tags: {type: "tags"}, - question: { - type: "content", - }, - overview: { - type: "content", - }, - keepInMind: { - type: "content", - }, - hints: { - type: "array", - elementShape: { - type: "hint", - }, - }, - }, - }, - }, - }, - }, - tags: { - idToName: (id) => tags[id], - nameToId: (name) => name[0], - names: ["a tag", "b tag", "c tag"], - }, - } as const; - - return ; -}; diff --git a/packages/perseus-editor/src/diffs/__tests__/structured-item-diff.test.ts b/packages/perseus-editor/src/diffs/__tests__/structured-item-diff.test.ts deleted file mode 100644 index 37d29ecac0..0000000000 --- a/packages/perseus-editor/src/diffs/__tests__/structured-item-diff.test.ts +++ /dev/null @@ -1,433 +0,0 @@ -import {buildEmptyItemTreeForShape, shapes} from "@khanacademy/perseus"; - -import StructuredItemDiff from "../structured-item-diff"; - -import type {Path} from "@khanacademy/perseus"; - -const gtpPassageShape = shapes.shape({ - directions: shapes.content, - passage: shapes.content, - overview: shapes.content, - hints: shapes.hints, - - questions: shapes.arrayOf( - shapes.shape({ - tags: shapes.tags, - question: shapes.content, - keepInMind: shapes.content, - overview: shapes.content, - hints: shapes.hints, - }), - ), -}); - -const gtpSingleQuestionShape = shapes.shape({ - blurb: shapes.content, - question: shapes.content, - hints: shapes.hints, -}); - -const emptyContent = buildEmptyItemTreeForShape(shapes.content); -const emptyHint = buildEmptyItemTreeForShape(shapes.hint); - -describe("StructuredItemDiff", function () { - /** - * Testing adding all empty content items, - * and then testing removing by reversing the order. - */ - it("test adding empty items, removing empty items", function () { - const beforeList = [ - [emptyContent, ["directions"]], - [emptyContent, ["passage"]], - [emptyContent, ["overview"]], - ]; - - const afterList = [ - [emptyContent, ["directions"]], - [emptyContent, ["passage"]], - [emptyContent, ["overview"]], - [emptyHint, ["hints", 0]], - [emptyHint, ["hints", 1]], - [emptyContent, ["questions", 0, "tags"]], - [emptyContent, ["questions", 0, "question"]], - [emptyContent, ["questions", 0, "keepInMind"]], - [emptyContent, ["questions", 0, "overview"]], - [emptyContent, ["questions", 1, "tags"]], - [emptyContent, ["questions", 1, "question"]], - [emptyContent, ["questions", 1, "keepInMind"]], - [emptyContent, ["questions", 1, "overview"]], - [emptyHint, ["questions", 1, "hints", 0]], - [emptyHint, ["questions", 1, "hints", 1]], - ]; - - const addingResult: Array = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - beforeList.slice(), - afterList.slice(), - addingResult, - gtpPassageShape, - [], - ); - - const removingResult = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - afterList, - beforeList, - removingResult, - gtpPassageShape, - [], - ); - - expect(addingResult).toEqual([ - ["directions"], - ["passage"], - ["overview"], - ["hints", 0], - ["hints", 1], - ["questions", 0, "tags"], - ["questions", 0, "question"], - ["questions", 0, "keepInMind"], - ["questions", 0, "overview"], - ["questions", 1, "tags"], - ["questions", 1, "question"], - ["questions", 1, "keepInMind"], - ["questions", 1, "overview"], - ["questions", 1, "hints", 0], - ["questions", 1, "hints", 1], - ]); - - expect(removingResult).toEqual([ - ["directions"], - ["passage"], - ["overview"], - ["hints", 0], - ["hints", 1], - ["questions", 0, "tags"], - ["questions", 0, "question"], - ["questions", 0, "keepInMind"], - ["questions", 0, "overview"], - ["questions", 1, "tags"], - ["questions", 1, "question"], - ["questions", 1, "keepInMind"], - ["questions", 1, "overview"], - ["questions", 1, "hints", 0], - ["questions", 1, "hints", 1], - ]); - }); - - /** - * Testing editing content of items, - * including top level items, as well as hints, - * and items within questions. - */ - it("test editing items", function () { - const passage = buildEmptyItemTreeForShape(shapes.content); - passage.content = "passage"; - const firstQuestion = buildEmptyItemTreeForShape(shapes.content); - firstQuestion.content = "question 1"; - const firstHint = buildEmptyItemTreeForShape(shapes.hint); - firstHint.content = "hint 1"; - const secondQuestionFirstHint = buildEmptyItemTreeForShape(shapes.hint); - secondQuestionFirstHint.content = "question 1 hint 1"; - const beforeList = [ - [emptyContent, ["directions"]], - [passage, ["passage"]], - [emptyContent, ["overview"]], - [firstHint, ["hints", 0]], - [emptyHint, ["hints", 1]], - [emptyContent, ["questions", 0, "tags"]], - [firstQuestion, ["questions", 0, "question"]], - [emptyContent, ["questions", 0, "keepInMind"]], - [emptyContent, ["questions", 0, "overview"]], - [emptyContent, ["questions", 1, "tags"]], - [emptyContent, ["questions", 1, "question"]], - [emptyContent, ["questions", 1, "keepInMind"]], - [emptyContent, ["questions", 1, "overview"]], - [secondQuestionFirstHint, ["questions", 1, "hints", 0]], - [emptyHint, ["questions", 1, "hints", 1]], - ]; - - const passageEdited = buildEmptyItemTreeForShape(shapes.content); - passageEdited.content = "passage edited"; - const firstQuestionEdited = buildEmptyItemTreeForShape(shapes.content); - firstQuestionEdited.content = "question edited 1"; - const firstHintEdited = buildEmptyItemTreeForShape(shapes.hint); - firstHintEdited.content = "edited hint 1"; - const secondQuestionFirstHintEdited = buildEmptyItemTreeForShape( - shapes.hint, - ); - secondQuestionFirstHintEdited.content = "question 1 hint 1 edited"; - const afterList = [ - [emptyContent, ["directions"]], - [passageEdited, ["passage"]], - [emptyContent, ["overview"]], - [firstHintEdited, ["hints", 0]], - [emptyHint, ["hints", 1]], - [emptyContent, ["questions", 0, "tags"]], - [firstQuestionEdited, ["questions", 0, "question"]], - [emptyContent, ["questions", 0, "keepInMind"]], - [emptyContent, ["questions", 0, "overview"]], - [emptyContent, ["questions", 1, "tags"]], - [emptyContent, ["questions", 1, "question"]], - [emptyContent, ["questions", 1, "keepInMind"]], - [emptyContent, ["questions", 1, "overview"]], - [secondQuestionFirstHintEdited, ["questions", 1, "hints", 0]], - [emptyHint, ["questions", 1, "hints", 1]], - ]; - - const firstResult: Array = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - beforeList.slice(), - afterList.slice(), - firstResult, - gtpPassageShape, - [], - ); - - const secondResult = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - afterList, - beforeList, - secondResult, - gtpPassageShape, - [], - ); - - expect(firstResult).toEqual([ - ["directions"], - ["passage"], - ["overview"], - ["hints", 0], - ["hints", 1], - ["questions", 0, "tags"], - ["questions", 0, "question"], - ["questions", 0, "keepInMind"], - ["questions", 0, "overview"], - ["questions", 1, "tags"], - ["questions", 1, "question"], - ["questions", 1, "keepInMind"], - ["questions", 1, "overview"], - ["questions", 1, "hints", 0], - ["questions", 1, "hints", 1], - ]); - - expect(secondResult).toEqual([ - ["directions"], - ["passage"], - ["overview"], - ["hints", 0], - ["hints", 1], - ["questions", 0, "tags"], - ["questions", 0, "question"], - ["questions", 0, "keepInMind"], - ["questions", 0, "overview"], - ["questions", 1, "tags"], - ["questions", 1, "question"], - ["questions", 1, "keepInMind"], - ["questions", 1, "overview"], - ["questions", 1, "hints", 0], - ["questions", 1, "hints", 1], - ]); - }); - - /** - * Testing these cases: - * (1) adding/removing overall hints - * (2) adding/removing question hints of a middle question - * (question is surrounded by other questions) - * (3) adding/removing question hints of the last question - * (4) adding/removing questions entirely - * - * This test is done with empty items, since we've tested for edits, - * and generateCompletePathLists() doesn't look at the contents. - */ - it("testing adding and removing in special cases", function () { - const beforeList = [ - [emptyContent, ["directions"]], - [emptyContent, ["passage"]], - [emptyContent, ["overview"]], - [emptyContent, ["questions", 0, "tags"]], - [emptyContent, ["questions", 0, "question"]], - [emptyContent, ["questions", 0, "keepInMind"]], - [emptyContent, ["questions", 0, "overview"]], - [emptyContent, ["questions", 1, "tags"]], - [emptyContent, ["questions", 1, "question"]], - [emptyContent, ["questions", 1, "keepInMind"]], - [emptyContent, ["questions", 1, "overview"]], - [emptyHint, ["questions", 1, "hints", 0]], - [emptyHint, ["questions", 1, "hints", 1]], - [emptyContent, ["questions", 2, "tags"]], - [emptyContent, ["questions", 2, "question"]], - [emptyContent, ["questions", 2, "keepInMind"]], - [emptyContent, ["questions", 2, "overview"]], - [emptyHint, ["questions", 2, "hints", 0]], - [emptyHint, ["questions", 2, "hints", 1]], - [emptyContent, ["questions", 3, "tags"]], - [emptyContent, ["questions", 3, "question"]], - [emptyContent, ["questions", 3, "keepInMind"]], - [emptyContent, ["questions", 3, "overview"]], - ]; - - const afterList = [ - [emptyContent, ["directions"]], - [emptyContent, ["passage"]], - [emptyContent, ["overview"]], - [emptyHint, ["hints", 0]], // add - [emptyHint, ["hints", 1]], // add - [emptyContent, ["questions", 0, "tags"]], - [emptyContent, ["questions", 0, "question"]], - [emptyContent, ["questions", 0, "keepInMind"]], - [emptyContent, ["questions", 0, "overview"]], - [emptyHint, ["questions", 0, "hints", 0]], // add - [emptyHint, ["questions", 0, "hints", 1]], // add - [emptyContent, ["questions", 1, "tags"]], - [emptyContent, ["questions", 1, "question"]], - [emptyContent, ["questions", 1, "keepInMind"]], - [emptyContent, ["questions", 1, "overview"]], - [emptyHint, ["questions", 1, "hints", 0]], - // remove second hint of question 1 - [emptyContent, ["questions", 2, "tags"]], - [emptyContent, ["questions", 2, "question"]], - [emptyContent, ["questions", 2, "keepInMind"]], - [emptyContent, ["questions", 2, "overview"]], - [emptyHint, ["questions", 2, "hints", 0]], - [emptyHint, ["questions", 2, "hints", 1]], - // removed question 2, added two hints to question 3 - ]; - - const firstResult: Array = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - beforeList.slice(), - afterList.slice(), - firstResult, - gtpPassageShape, - [], - ); - - const secondResult: Array = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - afterList, - beforeList, - secondResult, - gtpPassageShape, - [], - ); - - expect(firstResult).toEqual([ - ["directions"], - ["passage"], - ["overview"], - ["hints", 0], - ["hints", 1], - ["questions", 0, "tags"], - ["questions", 0, "question"], - ["questions", 0, "keepInMind"], - ["questions", 0, "overview"], - ["questions", 0, "hints", 0], - ["questions", 0, "hints", 1], - ["questions", 1, "tags"], - ["questions", 1, "question"], - ["questions", 1, "keepInMind"], - ["questions", 1, "overview"], - ["questions", 1, "hints", 0], - ["questions", 1, "hints", 1], - ["questions", 2, "tags"], - ["questions", 2, "question"], - ["questions", 2, "keepInMind"], - ["questions", 2, "overview"], - ["questions", 2, "hints", 0], - ["questions", 2, "hints", 1], - ["questions", 3, "tags"], - ["questions", 3, "question"], - ["questions", 3, "keepInMind"], - ["questions", 3, "overview"], - ]); - - expect(secondResult).toEqual([ - ["directions"], - ["passage"], - ["overview"], - ["hints", 0], - ["hints", 1], - ["questions", 0, "tags"], - ["questions", 0, "question"], - ["questions", 0, "keepInMind"], - ["questions", 0, "overview"], - ["questions", 0, "hints", 0], - ["questions", 0, "hints", 1], - ["questions", 1, "tags"], - ["questions", 1, "question"], - ["questions", 1, "keepInMind"], - ["questions", 1, "overview"], - ["questions", 1, "hints", 0], - ["questions", 1, "hints", 1], - ["questions", 2, "tags"], - ["questions", 2, "question"], - ["questions", 2, "keepInMind"], - ["questions", 2, "overview"], - ["questions", 2, "hints", 0], - ["questions", 2, "hints", 1], - ["questions", 3, "tags"], - ["questions", 3, "question"], - ["questions", 3, "keepInMind"], - ["questions", 3, "overview"], - ]); - }); - - /** - * Testing single question layout shape. - */ - it("testing single question layout", function () { - const beforeList = [ - [emptyContent, ["blurb"]], - [emptyContent, ["question"]], - ]; - - const afterList = [ - [emptyContent, ["blurb"]], - [emptyContent, ["question"]], - [emptyHint, ["hints", 0]], - [emptyHint, ["hints", 1]], - ]; - - const firstResult = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - beforeList.slice(), - afterList.slice(), - firstResult, - gtpSingleQuestionShape, - [], - ); - - const secondResult = []; - StructuredItemDiff.generateCompletePathsList( - // @ts-expect-error - TS2345 - Argument of type 'any[][]' is not assignable to parameter of type 'ItemList[]'. - afterList, - beforeList, - secondResult, - gtpSingleQuestionShape, - [], - ); - - expect(firstResult).toEqual([ - ["blurb"], - ["question"], - ["hints", 0], - ["hints", 1], - ]); - - expect(secondResult).toEqual([ - ["blurb"], - ["question"], - ["hints", 0], - ["hints", 1], - ]); - }); -}); diff --git a/packages/perseus-editor/src/diffs/structured-item-diff.tsx b/packages/perseus-editor/src/diffs/structured-item-diff.tsx deleted file mode 100644 index 43cccc1298..0000000000 --- a/packages/perseus-editor/src/diffs/structured-item-diff.tsx +++ /dev/null @@ -1,275 +0,0 @@ -/* eslint-disable @khanacademy/ts-no-error-suppressions */ -/** - * A side by side diff view for Perseus exercise items - * that do not have the standard question layout. - */ -import { - buildEmptyItemTreeForShape, - buildMapper, - itemToTree, - shapes, -} from "@khanacademy/perseus"; -import * as React from "react"; - -import RendererDiff from "./renderer-diff"; -import TagsDiff from "./tags-diff"; - -import type {Item, Path, Shape} from "@khanacademy/perseus"; - -type ItemList = [unknown, Path]; - -/** - * Outputs true if path begins with beginPath, false otherwise. - */ -function beginsWith(path: Path, beginPath: Path): boolean { - let matches = true; - for (let i = 0; i < beginPath.length; i++) { - if (i >= path.length) { - return false; - } - if (beginPath[i] !== path[i]) { - matches = false; - } - } - return matches; -} - -/** - * Outputs true if beforePath and afterPath are the same. - */ -function checkPath(beforePath: Path, afterPath: Path): boolean { - if (beforePath.length !== afterPath.length) { - return false; - } - for (let i = 0, l = beforePath.length; i < l; i++) { - if (beforePath[i] !== afterPath[i]) { - return false; - } - } - return true; -} - -/** - * Given a path, returns a title. Puts colons after numbers in the path. - */ -function getTitle(path: Path): string { - const title: Array = []; - for (let i = 0; i < path.length; i++) { - if (typeof path[i] === "number") { - // @ts-expect-error - TS2365 - Operator '+' cannot be applied to types 'string | number' and 'number'. - title.push((path[i] + 1).toString() + ":"); - } else { - // @ts-expect-error - TS2345 - Argument of type 'string | number' is not assignable to parameter of type 'string'. - title.push(path[i]); - } - } - return title.join(" "); -} - -type Tag = { - idToName: (arg1: string) => string; - nameToId: (arg1: string) => string; - names: ReadonlyArray; -}; - -type Props = { - after: Item; - before: Item; - shape: Shape; - tags: Tag; -}; - -class StructuredItemDiff extends React.Component { - /** - * Traverses the given shape and adds paths that are present in - * beforeList and afterList to result. Note that this method assumes - * the order of elements in beforeList and afterList, which are - * from buildMapper(), is the same order they appear in in the shape. - */ - static generateCompletePathsList( - beforeList: Array, - afterList: Array, - result: Array, - shape: Shape, - path: ReadonlyArray, - ): void { - if ( - shape.type === "content" || - shape.type === "hint" || - shape.type === "tags" - ) { - const beforePath = - beforeList.length > 0 && checkPath(path, beforeList[0][1]); - const afterPath = - afterList.length > 0 && checkPath(path, afterList[0][1]); - if (beforePath && afterPath) { - result.push(path); - beforeList.splice(0, 1); - afterList.splice(0, 1); - } else if (beforePath) { - result.push(path); - beforeList.splice(0, 1); - } else if (afterPath) { - result.push(path); - afterList.splice(0, 1); - } - } else if (shape.type === "array") { - let index = 0; - let newPath = path.concat(index); - - // For array types, the paths will be in the form [, n], - // where n is an integer > 0 and increments. - // As long as either beforeList or afterList has a next element that - // matches [, n], we recurse into that item with the new path. - while ( - (beforeList.length > 0 && - beginsWith(beforeList[0][1], newPath)) || - (afterList.length > 0 && beginsWith(afterList[0][1], newPath)) - ) { - StructuredItemDiff.generateCompletePathsList( - beforeList, - afterList, - result, - shape.elementShape, - newPath, - ); - index++; - newPath = path.concat(index); - } - } else if (shape.type === "object") { - const keys = Object.keys(shape.shape); - for (let i = 0; i < keys.length; i++) { - const newPath = path.concat([keys[i]]); - StructuredItemDiff.generateCompletePathsList( - beforeList, - afterList, - result, - shape.shape[keys[i]], - newPath, - ); - } - } - } - - render(): React.ReactNode { - const {before, after, shape, tags} = this.props; - - const beforeList: Array = []; - const afterList = []; - - buildMapper() - .setContentMapper((c, _, p) => beforeList.push([c, p])) - .setHintMapper((c, _, p) => beforeList.push([c, p])) - .setTagsMapper((c, _, p) => beforeList.push([c, p])) - .mapTree(itemToTree(before), shape); - - buildMapper() - // @ts-expect-error - TS2322 - Type 'unknown' is not assignable to type 'never'. | TS2322 - Type 'Path' is not assignable to type 'never'. - .setContentMapper((c, _, p) => afterList.push([c, p])) - // @ts-expect-error - TS2322 - Type 'unknown' is not assignable to type 'never'. | TS2322 - Type 'Path' is not assignable to type 'never'. - .setHintMapper((c, _, p) => afterList.push([c, p])) - // @ts-expect-error - TS2322 - Type 'unknown' is not assignable to type 'never'. | TS2322 - Type 'Path' is not assignable to type 'never'. - .setTagsMapper((c, _, p) => afterList.push([c, p])) - .mapTree(itemToTree(after), shape); - - // These are used in generateCompletePathsList() - // and are modified in that method. - const beforeListModified = beforeList.slice(); - const afterListModified = afterList.slice(); - - const allDiffPaths: Array = []; - StructuredItemDiff.generateCompletePathsList( - beforeListModified, - afterListModified, - allDiffPaths, - shape, - [], - ); - - const diffCount = allDiffPaths.length; - - const diffs: React.ReactNode = allDiffPaths.map((path, n) => { - const isTag = path[path.length - 1] === "tags"; - const currentTitle = getTitle(path); - - let before = beforeList.find((e) => { - return checkPath(e[1], path); - }); - let after = afterList.find((e) => { - return checkPath(e[1], path); - }); - - if (isTag) { - if (!before) { - before = [[], path]; - } - if (!after) { - // @ts-expect-error - TS2322 - Type 'Path[]' is not assignable to type 'undefined'. - after = [[], path]; - } - - const beforeTags: Array = []; - if (Array.isArray(before[0])) { - before[0].forEach((tagId) => { - if (typeof tagId === "string") { - beforeTags.push(tags.idToName(tagId)); - } - }); - } - const afterTags: Array = []; - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - if (Array.isArray(after[0])) { - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - after[0].forEach((tagId) => { - if (typeof tagId === "string") { - afterTags.push(tags.idToName(tagId)); - } - }); - } - - const intersection = beforeTags.filter((tag) => - afterTags.includes(tag), - ); - const beforeOnly = beforeTags.filter( - (tag) => !afterTags.includes(tag), - ); - const afterOnly = afterTags.filter( - (tag) => !beforeTags.includes(tag), - ); - - return ( - - ); - } - if (!before) { - before = [buildEmptyItemTreeForShape(shapes.content), path]; - } - if (!after) { - // @ts-expect-error - TS2322 - Type 'any[]' is not assignable to type 'undefined'. - after = [buildEmptyItemTreeForShape(shapes.content), path]; - } - return ( - - ); - }); - - return
{diffs}
; - } -} - -export default StructuredItemDiff; diff --git a/packages/perseus-editor/src/i18n.ts b/packages/perseus-editor/src/i18n.ts deleted file mode 100644 index 281b82f962..0000000000 --- a/packages/perseus-editor/src/i18n.ts +++ /dev/null @@ -1,140 +0,0 @@ -/** - * Functions for extracting data from items for use in i18n. - */ -import {traverse, MultiItems, PerseusMarkdown} from "@khanacademy/perseus"; -import _ from "underscore"; - -const {findContentNodesInItem, findHintNodesInItem, inferItemShape} = - MultiItems; - -// Takes a renderer content and parses the markdown for images -function findImagesInContent(content: any, images: Array) { - // @ts-expect-error - TS2554 - Expected 2 arguments, but got 1. - const parsed = PerseusMarkdown.parse(content); - - PerseusMarkdown.traverseContent(parsed, function (node) { - if (node.type === "image") { - images.push(node.target); - } - }); -} - -// Background images in some widgets are annoying to deal with because -// sometimes the objects aren't full when there isn't an image. So, we do some -// extra checking to make sure we don't cause an error or push an empty image. -function handleBackgroundImage(graph, images: Array) { - if (graph && graph.backgroundImage && graph.backgroundImage.url) { - images.push(graph.backgroundImage.url); - } -} - -// The callback called for each widget. We check each of the areas of each -// widget where they contain a renderer for images by calling -// findImagesInContent. We don't have to recurse through child widgets, because -// traverseRendererDeep does that for us. -function widgetCallback(widgetInfo: any, images: Array) { - if (!widgetInfo.options) { - return; - } - - // TODO(emily/aria): Move this into the widget files, so we don't have the - // logic out here. - if (widgetInfo.type === "categorizer") { - _.each(widgetInfo.options.items, function (item) { - findImagesInContent(item, images); - }); - _.each(widgetInfo.options.categories, function (category) { - findImagesInContent(category, images); - }); - } else if (widgetInfo.type === "image") { - findImagesInContent(widgetInfo.options.title, images); - findImagesInContent(widgetInfo.options.caption, images); - } else if (widgetInfo.type === "matcher") { - _.each(widgetInfo.options.left, function (option) { - findImagesInContent(option, images); - }); - _.each(widgetInfo.options.right, function (option) { - findImagesInContent(option, images); - }); - _.each(widgetInfo.options.labels, function (label) { - findImagesInContent(label, images); - }); - } else if (widgetInfo.type === "matrix") { - findImagesInContent(widgetInfo.options.prefix, images); - findImagesInContent(widgetInfo.options.suffix, images); - } else if (widgetInfo.type === "orderer") { - _.each(widgetInfo.options.options, function (option) { - findImagesInContent(option.content, images); - }); - } else if (widgetInfo.type === "passage") { - findImagesInContent(widgetInfo.options.passageTitle, images); - } else if (widgetInfo.type === "radio") { - _.each(widgetInfo.options.choices, function (choice) { - findImagesInContent(choice.content, images); - }); - } else if (widgetInfo.type === "sorter") { - _.each(widgetInfo.options.correct, function (option) { - findImagesInContent(option, images); - }); - } else if (widgetInfo.type === "table") { - _.each(widgetInfo.options.headers, function (header) { - findImagesInContent(header, images); - }); - } - - if (widgetInfo.type === "grapher") { - handleBackgroundImage(widgetInfo.options.graph, images); - } else if (widgetInfo.type === "image") { - handleBackgroundImage(widgetInfo.options, images); - } else if (widgetInfo.type === "interactive-graph") { - handleBackgroundImage(widgetInfo.options, images); - } else if (widgetInfo.type === "measurer" && widgetInfo.options.image) { - images.push(widgetInfo.options.image.url); - } else if (widgetInfo.type === "plotter") { - images.push(widgetInfo.options.picUrl); - } -} - -function findImagesInRenderers(renderers) { - const images = []; - - _.each(renderers, (renderer) => { - traverse( - renderer, - (content) => { - findImagesInContent(content, images); - }, - (widget) => widgetCallback(widget, images), - ); - }); - - return images; -} - -// Calls findImagesInContent on all of the different content areas for -// assessment items -function findImagesInItemData(itemData: any): any { - let renderers = []; - if (itemData._multi) { - const shape = inferItemShape(itemData); - // @ts-expect-error - TS2345 - Argument of type 'ContentNode' is not assignable to parameter of type 'never'. - findContentNodesInItem(itemData, shape, (node) => renderers.push(node)); - // @ts-expect-error - TS2345 - Argument of type 'HintNode' is not assignable to parameter of type 'never'. - findHintNodesInItem(itemData, shape, (node) => renderers.push(node)); - } else { - // @ts-expect-error - TS2322 - Type 'any' is not assignable to type 'never'. | TS2322 - Type 'any' is not assignable to type 'never'. - renderers = [itemData.question, ...itemData.hints]; - } - return findImagesInRenderers(renderers); -} - -// Calls findImagesInContent on all of the different content areas for -// articles -function findImagesInArticles(perseusContent: any): any { - return findImagesInRenderers(perseusContent); -} - -export default { - findImagesInArticles: findImagesInArticles, - findImagesInItemData: findImagesInItemData, -}; diff --git a/packages/perseus-editor/src/index.ts b/packages/perseus-editor/src/index.ts index 73e1b5921f..964f9b7140 100644 --- a/packages/perseus-editor/src/index.ts +++ b/packages/perseus-editor/src/index.ts @@ -5,12 +5,9 @@ export {default as DeviceFramer} from "./components/device-framer"; export {default as ViewportResizer} from "./components/viewport-resizer"; export {default as ArticleDiff} from "./diffs/article-diff"; export {default as ItemDiff} from "./diffs/item-diff"; -export {default as StructuredItemDiff} from "./diffs/structured-item-diff"; export {default as EditorPage} from "./editor-page"; export {default as Editor} from "./editor"; -export {default as i18n} from "./i18n"; export {default as IframeContentRenderer} from "./iframe-content-renderer"; -export {default as MultiRendererEditor} from "./multirenderer-editor"; import "./styles/perseus-editor.less"; diff --git a/packages/perseus-editor/src/multirenderer-editor.tsx b/packages/perseus-editor/src/multirenderer-editor.tsx deleted file mode 100644 index 2d0445c7fd..0000000000 --- a/packages/perseus-editor/src/multirenderer-editor.tsx +++ /dev/null @@ -1,1060 +0,0 @@ -/** - * Editor for a multi-item question. - * - * TODO(mdr): The UI for managing arrays isn't visually consistent with - * HintsEditor. Should we bring them in line with each other? - */ -import { - ApiOptions, - buildEmptyItemTreeForShape, - components, - iconChevronDown, - iconTrash, - itemToTree, - MultiItems, -} from "@khanacademy/perseus"; -import {StyleSheet, css} from "aphrodite"; -// eslint-disable-next-line import/no-extraneous-dependencies -import lens from "hubble"; -import * as React from "react"; -import ReactDOM from "react-dom"; - -import JsonEditor from "./components/json-editor"; -import SimpleButton from "./components/simple-button"; -import Editor from "./editor"; -import {HintEditor} from "./hint-editor"; - -import type { - APIOptions, - ChangeHandler, - EditorMode, - - // Multi-item item types - Item, - ItemTree, - ItemObjectNode, - ItemArrayNode, - ContentNode, - HintNode, - TagsNode, - - // Multi-item shape types - Shape, - ArrayShape, - ObjectShape, - ContentShape, - HintShape, - TagsShape, -} from "@khanacademy/perseus"; - -const {InlineIcon} = components; -const {MultiRenderer} = MultiItems; - -// TODO(CP-4849): figure out when $ReadOnlyArray vs $ReadOnlyArray should be used -type Path = ReadonlyArray; - -type ModeDropdownProps = { - currentMode: EditorMode; - // A function that takes in a string signifying the mode (ex: "edit") - onChange: (mode: EditorMode) => unknown; -}; - -/** - * Component that displays the mode dropdown. - * - * The mode dropdown is the selector at the top of the editor that lets you - * switch between edit, preview, and dev-only JSON mode. - */ -class ModeDropdown extends React.Component { - _handleSelectMode = (event: React.ChangeEvent) => { - if (this.props.onChange) { - // event.target.value corresponds to the options' values below which - // are limited to EditorMode, but TypeScript doesn't know that so we have - // to cast through any here. - const value = event.target.value as EditorMode; - this.props.onChange(value); - } - }; - - render(): React.ReactNode { - return ( - - ); - } -} - -/** - * Convert a camel-cased string to a human-formatted string. - * "superCoolThings" -> "super cool things" - */ -function camelCaseToHuman(str: string) { - // Decapitalize the capital letters, and add a space before each. - return str.replace(/[A-Z]/g, (s) => " " + s.toLowerCase()); -} - -/** - * Capitalize the first letter of the given string. - * "super cool things" -> "Super cool things" - */ -function capitalize(str: string) { - return str.charAt(0).toUpperCase() + str.slice(1).toLowerCase(); -} - -/** - * Convert the given pluralized word to a singularized word. - * "super cool things" -> "super cool thing" - */ -function pluralToSingular(str: string) { - if (str.charAt(str.length - 1) === "s") { - // Incredibly weak implementation :P - return str.slice(0, -1); - } - // Uh oh, dunno how to singularize anything but the simplest case! - // Let's just return the plural form, and hope the user forgives the - // grammatical inconsistency. - return str; -} - -/** - * When iterating through the editors, we don't keep track of the extra - * `_multi` part at the beginning. This is a helper function which takes a path - * and prepends that key. - */ -function multiPath(path: Path | Array) { - return ["_multi", ...path]; -} - -// Return an h1 if depth=0, h2 if depth=1, etc. -// NOTE: This component accepts pass-through props. -type HeaderProps = { - depth: number; -}; - -const Header = ({depth, ...props}: HeaderProps): React.ReactElement => { - const headerLevel = Math.min(depth, 5) + 1; - const HeaderTag = `h${headerLevel}`; - return ; -}; - -// Actions is inexact so that we can pass an instance MultiRendererEditor. -interface Actions { - addArrayElement: (path: Path, shape: Shape) => void; - mergeValueAtPath: (path: Path, newValue?: any) => void; - setValueAtPath: (path: Path, newValue?: any) => void; - moveArrayElementDown: (path: Path) => void; - moveArrayElementUp: (path: Path) => void; - removeArrayElement: (path: Path) => void; -} - -// This type is used to define prop types for various nodes. The S and D type -// params should match when used, e.g. the prop types for `HintNodeContent` is -// `NodePropTypes`. -type NodePropTypes = { - shape: S; - data: D; - path: Path; - actions: Actions; - apiOptions: APIOptions; - // For the left-hand column, we use edit mode and leave renderers empty. - // For the right-hand column, we use preview mode and provide renderers - // via a MultiRenderer. - // TODO(CP-4850): figure out how to type this, it appears to be a tree where the - // leaf nodes could be typed using RendererInterface. - renderers?: any; -}; - -/** - * Render a node in the editor tree, given the shape of the target - * node, the data stored in the target node, the path to the target - * node, and any UI controls that affect how this node relates to its - * parent (e.g. remove from parent array). - * - * This returns a container element with a pretty title and additional - * UI controls for this node. Its contents are produced by - * `NodeContent`. The two functions are mutually recursive. - * - * Leaf nodes, like items and hints, render an editor pod around their - * content. Container nodes, like arrays and objects, render a header above - * their content. - */ -const NodeContainer = ( - props: NodePropTypes & { - controls?: ReadonlyArray; - name?: string; - }, -) => { - const { - shape, - data, - path, - actions, - name: givenName, - controls, - ...otherProps - } = props; - - const name = givenName || camelCaseToHuman(path[path.length - 1] || ""); - - const children = ( - - ); - - const key = path.join("."); - - if (shape.type === "array") { - return ( - - {children} - - ); - } - if (shape.type === "object") { - return ( - - {children} - - ); - } - return ( - - {children} - - ); -}; - -type LeafContainerProps = { - name: string; - controls?: React.ReactNode; - children?: React.ReactNode; - path: Path; - shape: Shape; -}; -const LeafContainer = ({ - name, - controls, - children, - path, - shape, -}: LeafContainerProps): React.ReactElement => { - const hasPreviewHeading = shape.type === "content" || shape.type === "hint"; - const previewHeading = hasPreviewHeading && ( -
- {/* @ts-expect-error - TS2322 - Type '{ children: string; depth: number; className: string; }' is not assignable to type 'IntrinsicAttributes & HeaderProps & { children?: ReactNode; }'. */} -
- {capitalize(name)} -
-
- ); - return ( -
- -
-
-
- {capitalize(name)} -
- {controls} -
-
-
{previewHeading}
- - {children} -
- ); -}; - -interface ArrayContainerActions { - addArrayElement: (path: Path, shape: Shape) => void; -} - -type ArrayContainerProps = { - name: string; - controls?: React.ReactNode; - children?: React.ReactNode; - path: Path; - shape: ArrayShape; - actions: ArrayContainerActions; -}; -const ArrayContainer = (props: ArrayContainerProps): React.ReactElement => { - const {name, controls, children, path, shape, actions} = props; - return ( -
- {controls && ( -
- {controls} -
- )} -
{children}
-
- {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */} - - actions.addArrayElement(path, shape.elementShape) - } - > - Add a {pluralToSingular(name)} - -
-
- ); -}; - -type ObjectContainerProps = { - name: string; - controls?: React.ReactNode; - children?: React.ReactNode; - path: Path; -}; -const ObjectContainer = ({ - name, - controls, - children, - path, -}: ObjectContainerProps): React.ReactElement => { - const headingEditor = ( -
- {/* @ts-expect-error - TS2322 - Type '{ children: string; depth: number; className: string; }' is not assignable to type 'IntrinsicAttributes & HeaderProps & { children?: ReactNode; }'. */} -
- {capitalize(name)} -
- {controls} -
- ); - const headingPreview = (name || controls) && ( -
- {/* @ts-expect-error - TS2322 - Type '{ children: string; depth: number; className: string; }' is not assignable to type 'IntrinsicAttributes & HeaderProps & { children?: ReactNode; }'. */} -
- {capitalize(name)} -
-
- ); - const hasBothHeadings = headingEditor && headingPreview; - return ( -
- {hasBothHeadings && ( - -
- {headingEditor} -
-
- {headingPreview} -
- - )} -
0 && styles.contentIndent)}> - {children} -
-
- ); -}; - -/** - * Render the content of node in the editor tree, given the shape of - * the target node, the data stored in the target node, and the path to - * the target node. - * - * If the target node is a leaf, this returns an editor. Otherwise, it - * iterates over the child nodes, and outputs `NodeContainer` for - * each of them. The two functions are mutually recursive. - */ -const NodeContent = (props: NodePropTypes) => { - const {shape, data, ...restProps} = props; - - // All uses of `data` have been cast through any which isn't safe. This was - // done to avoid introducing new logic which may have resulted in a change - // of behavior. Also, there doesn't appear to be a way to tell TypeScript the - // difference between and `TagsNode` and `ItemArrayNode`, see - // perseus-all-package/multi-items/item-types.js. - if (shape.type === "content") { - return ( - - ); - } - if (shape.type === "hint") { - return ( - - ); - } - if (shape.type === "tags") { - return ( - - ); - } - if (shape.type === "array") { - return ( - - ); - } - if (shape.type === "object") { - return ( - - ); - } - return null; -}; - -type WithStickinessProps = { - sticky: boolean; -}; - -type WithStickiness = T & WithStickinessProps; - -/** - * HOC that adds a "sticky" prop to the wrapped component that is true - * when the rendered component is taller than the window. Since sticky content - * can be somewhat distracting, we'd like to avoid it when not useful. This - * HOC is useful for only making content sticky when useful. - * - * It does so by polling the height and comparing it to the window height. - */ -const withStickiness = < - Config extends Record, - Component extends React.ComponentType>, ->( - WrappedComponent: Component, -): React.ComponentType => { - type State = { - sticky: boolean; - }; - return class StickyComponent extends React.Component { - // @ts-expect-error - TS2564 - Property 'stickynessTimer' has no initializer and is not definitely assigned in the constructor. - stickynessTimer: number; - - state = { - sticky: false, - }; - - componentDidMount() { - // TODO(jeff, CP-3128): Use Wonder Blocks Timing API. - // eslint-disable-next-line no-restricted-syntax - // @ts-expect-error - TS2322 - Type 'Timer' is not assignable to type 'number'. - this.stickynessTimer = setInterval(this.updateStickiness, 1000); - this.updateStickiness(); - } - - componentWillUnmount() { - // TODO(jeff, CP-3128): Use Wonder Blocks Timing API. - // eslint-disable-next-line no-restricted-syntax - clearInterval(this.stickynessTimer); - } - - updateStickiness = () => { - const domNode = ReactDOM.findDOMNode(this); - // @ts-expect-error - TS2531 - Object is possibly 'null'. | TS2339 - Property 'offsetHeight' does not exist on type 'Element | Text'. - const height = domNode.offsetHeight; - const windowHeight = window.innerHeight; - const sticky = height > windowHeight; - if (sticky !== this.state.sticky) { - this.setState({ - sticky, - }); - } - }; - - render(): React.ReactNode { - return ( - // @ts-expect-error - TS2322 - Type 'Readonly & { sticky: boolean; children?: ReactNode; }' is not assignable to type 'IntrinsicAttributes & LibraryManagedAttributes>>'. - - ); - } - }; -}; - -const ItemNodeContent = withStickiness( - (props: NodePropTypes & WithStickinessProps) => { - const {data, path, actions, apiOptions, renderers, sticky} = props; - - const preview = ( -
{lens(renderers).get(path)}
- ); - - return ( - -
-
-
- {/* TODO(CP-4852): only pass the props to Editor that it uses. */} - { - // @ts-expect-error - TS2769 - No overload matches this call. - - actions.mergeValueAtPath(path, newVal) - } - apiOptions={apiOptions} - /> - } -
-
-
-
- {preview} -
-
-
- - ); - }, -); - -const HintNodeContent = withStickiness( - (props: NodePropTypes & WithStickinessProps) => { - const {data, path, actions, apiOptions, renderers, sticky} = props; - - const preview = ( -
{lens(renderers).get(path)}
- ); - - return ( -
-
-
- - actions.mergeValueAtPath(path, newVal) - } - apiOptions={apiOptions} - showTitle={false} - showRemoveButton={false} - showMoveButtons={false} - // no-op handler since showRemoveButton={false} - onRemove={() => {}} - // no-op handler since showMoveButtons={false} - onMove={(direction: number) => {}} - isFirst={true} - isLast={true} - /> -
-
-
-
- {preview} -
-
-
- ); - }, -); - -const TagsNodeContent = (props: NodePropTypes) => { - const {data, path, actions, apiOptions} = props; - const {GroupMetadataEditor} = apiOptions; - - if (GroupMetadataEditor == null) { - return null; - } - return ( -
-
- actions.setValueAtPath(path, newVal)} - showTitle={false} - /> -
-
- ); -}; - -const ArrayNodeContent = (props: NodePropTypes) => { - const {shape, data, path, actions, ...otherProps} = props; - - const collectionName = camelCaseToHuman(path[path.length - 1]); - const elementName = pluralToSingular(collectionName); - - const elementType = shape.elementShape.type; - const elementIsLeaf = elementType === "content" || elementType === "hint"; - - /** - * TODO(somewhatabstract, JIRA-XXXX): - * The NodePropTypes generic and specifically the ItemArrayNode could - * contain a variety of types that are not arrays. Probably need to refine - * the type more before doing this work, or rework the `ArrayNodeContent` - * component to be less permissive about the props it accepts. - */ - const children = data.map((subdata, i) => { - const subpath = path.concat(i); - const controls = [ - i > 0 && ( -
- actions.moveArrayElementUp(subpath)} - > -
- -
- -
- ), - i < data.length - 1 && ( -
- actions.moveArrayElementDown(subpath)} - > - - -
- ), -
- actions.removeArrayElement(subpath)} - > - - -
, - ]; - - return ( -
- -
- ); - }); - - return
{children}
; -}; - -const ObjectNodeContent = ( - props: NodePropTypes, -) => { - const {shape, data, path, ...otherProps} = props; - - // Object iteration order should automatically match the order in which the - // keys were defined in the object literal. So, whatever order semantically - // made sense to the shape's author is the order in which we'll iterate :) - const children = Object.keys(shape.shape).map((subkey) => ( -
- -
- )); - - return
{children}
; -}; - -interface LayoutStatics { - shape: Shape; -} - -type MultiRendererEditorProps = { - // eslint-disable-next-line no-restricted-syntax - Layout: React.ComponentType & LayoutStatics; - apiOptions: APIOptions; - item: Item; - editorMode: EditorMode; - onChange: ChangeHandler; -}; - -class MultiRendererEditor extends React.Component { - layout: React.ElementRef | null | undefined; - - _renderLayout: () => React.ReactElement = () => { - const {Layout, apiOptions, item} = this.props; - - return ( - (this.layout = node)} - item={item} - apiOptions={apiOptions} - /> - ); - }; - - _renderJson: () => React.ReactElement> = () => { - return ( -
- this.props.onChange({editorMode})} - /> - this.props.onChange({item})} - /> -
- ); - }; - - _renderPreview: () => React.ReactElement> = - () => { - return ( -
- - this.props.onChange({editorMode}) - } - /> - {this._renderLayout()} -
- ); - }; - - mergeValueAtPath: (path: Path, newValue?: any) => void = ( - path: Path, - newValue: unknown, - ) => { - this.props.onChange({ - item: lens(this.props.item) - .merge(multiPath(path), newValue) - .freeze(), - }); - }; - - setValueAtPath: (path: Path, newValue?: any) => void = ( - path: Path, - newValue: unknown, - ) => { - this.props.onChange({ - item: lens(this.props.item).set(multiPath(path), newValue).freeze(), - }); - }; - - addArrayElement: (path: Path, shape: Shape) => void = ( - path: Path, - shape: Shape, - ) => { - const currentLength = lens(this.props.item).get(multiPath(path)).length; - const newElementPath = path.concat(currentLength); - const newValue = buildEmptyItemTreeForShape(shape); - this.props.onChange({ - item: lens(this.props.item) - .set(multiPath(newElementPath), newValue) - .freeze(), - }); - }; - - removeArrayElement: (path: Path) => void = (path: Path) => { - this.props.onChange({ - item: lens(this.props.item).del(multiPath(path)).freeze(), - }); - }; - - moveArrayElementDown: (path: Path) => void = (path: Path) => { - // Moving an element down can also be expressed as swapping it with the - // following element. - const index = path[path.length - 1]; - const nextElementIndex = index + 1; - const nextElementPath = path.slice(0, -1).concat(nextElementIndex); - - const element = lens(this.props.item).get(multiPath(path)); - const nextElement = lens(this.props.item).get( - multiPath(nextElementPath), - ); - - this.props.onChange({ - item: lens(this.props.item) - .set(multiPath(path), nextElement) - .set(multiPath(nextElementPath), element) - .freeze(), - }); - }; - - moveArrayElementUp: (path: Path) => void = (path: Path) => { - // Moving an element up can also be expressed as moving the previous - // element down. - const index = path[path.length - 1]; - const previousElementPath = path.slice(0, -1).concat(index - 1); - this.moveArrayElementDown(previousElementPath); - }; - - _renderEdit: () => React.ReactElement> = () => { - const apiOptions = { - ...ApiOptions.defaults, - ...this.props.apiOptions, - } as const; - - const {item} = this.props; - const itemShape: Shape = this.props.Layout.shape; - - return ( -
- this.props.onChange({editorMode})} - /> - - {}}, - useVideo: (() => {}) as any, - }} - > - {({renderers}) => ( - - )} - -
- ); - }; - - score: () => any | undefined = () => { - if (this.layout) { - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - return this.layout.score(); - } - }; - - getSerializedState: () => any | undefined = () => { - if (this.layout) { - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - return this.layout.getSerializedState(); - } - }; - - restoreSerializedState: (state?: any) => void = (state: any) => { - if (this.layout) { - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - this.layout.restoreSerializedState(state); - } - }; - - _renderContent: () => - | React.ReactElement> - | React.ReactNode = () => { - switch (this.props.editorMode) { - case "json": - return this._renderJson(); - case "preview": - return this._renderPreview(); - case "edit": - return this._renderEdit(); - default: - return ( - - this.props.onChange({ - editorMode, - }) - } - /> - ); - } - }; - - render(): React.ReactNode { - return
{this._renderContent()}
; - } -} - -const styles = StyleSheet.create({ - // This is used in a number of places throughout this file. - container: {}, - - // eslint-disable-next-line react-native/no-unused-styles - editor: { - width: "100%", - }, - - // eslint-disable-next-line react-native/no-unused-styles - treePreview: { - position: "relative", - }, - - verticalFlip: { - transform: "scaleY(-1)", - }, - - control: { - marginLeft: 12, - }, - - containerHeader: { - alignItems: "flex-end", - display: "flex", - flexDirection: "row", - }, - - previewCollectionHeader: { - marginBottom: 16, - }, - - containerTitle: { - flexGrow: 1, - margin: 0, - }, - - contentIndent: { - marginLeft: 8, - }, - - hintEditor: { - paddingBottom: 0, - }, - - arrayElement: { - marginBottom: 16, - }, - - // Leaf nodes are already wrapped in cute little pods, so they don't need - // this extra border between array elements. - arrayElementAndNotLeaf: { - borderBottom: "1px solid #ccc", - ":first-child": { - borderTop: "1px solid #ccc", - paddingTop: 16, - }, - }, - - objectElement: { - marginBottom: 16, - }, - - tagsEditor: { - border: "1px solid #ddd", - padding: "5px 10px", - }, - - /** - * A row contains a fixed width editor and a preview that expands as - * needed. - */ - row: { - display: "flex", - position: "relative", - }, - - /** - * The editor. - */ - columnLeft: { - width: 360, - marginRight: 30, - // so that the `position: absolute` of line markers are positioned - // relative to this. - position: "relative", - }, - - /** - * The preview. - */ - columnRight: { - flex: 1, - marginLeft: 30, - position: "relative", - }, - - /** - * Sticks to just under the heading. - */ - sticky: { - position: "sticky", - top: 33, // height of the cute pod for the editor - }, - - /** - * Used for sticky headings. - */ - rowHeading: { - position: "sticky", - backgroundColor: "white", - width: "100%", - // TODO(joshuan): Make this less arbitrary. It should be higher than - // perseus content. - zIndex: 101, - top: -1, - }, -}); - -export default MultiRendererEditor; diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx index 2ae43e2008..73a1ef623e 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx @@ -159,8 +159,6 @@ class InteractiveGraphSettings extends React.Component { } UNSAFE_componentWillReceiveProps(nextProps) { - // Make sure that state updates when switching - // between different items in a multi-item editor. if ( !_.isEqual(this.props.labels, nextProps.labels) || !_.isEqual(this.props.gridStep, nextProps.gridStep) || diff --git a/packages/perseus/src/__tests__/a11y.test.ts b/packages/perseus/src/__tests__/a11y.test.ts index 5a83c58828..c1c101b96d 100644 --- a/packages/perseus/src/__tests__/a11y.test.ts +++ b/packages/perseus/src/__tests__/a11y.test.ts @@ -275,29 +275,6 @@ describe("a11y", () => { const result = violatingWidgets(emptyImageWithoutAltText); expect(result).toHaveLength(0); }); - - it("should handle all these same cases in multi-items", () => { - const decorateItem = (item) => ({ - __type: "content", - ...item.question, - }); - const result = violatingWidgets({ - _multi: { - sharedContext: decorateItem(oneInaccessibleWidget), - questions: [ - decorateItem(noWidgets), - decorateItem(oneAccessibleWidget), - decorateItem(imageWithAltText), - decorateItem(imageWithoutAltText), - decorateItem(emptyImageWithoutAltText), - ], - }, - }); - result.sort(); // don't depend on iteration order - expect(result).toHaveLength(2); - expect(result[0]).toBe("image"); - expect(result[1]).toBe("matrix"); - }); }); }); }); diff --git a/packages/perseus/src/__tests__/renderability.test.ts b/packages/perseus/src/__tests__/renderability.test.ts index 7226d73d96..d1076f8a5c 100644 --- a/packages/perseus/src/__tests__/renderability.test.ts +++ b/packages/perseus/src/__tests__/renderability.test.ts @@ -416,65 +416,5 @@ describe("Renderability", () => { expect(result).toBe(false); }); }); - - describe("Multi-items", () => { - it("should be renderable with no items", () => { - const result = isItemRenderableByVersion( - { - _multi: { - questions: [], - }, - }, - PerseusItemVersion, - ); - expect(result).toBe(true); - }); - - it("should be renderable when all items are", () => { - const result = isItemRenderableByVersion( - { - _multi: { - sharedContext: { - __type: "content", - ...sampleV0InputNumberItem.question, - }, - questions: [ - { - __type: "content", - ...sampleV1MeasurerItem.question, - }, - ], - }, - }, - PerseusItemVersion, - ); - expect(result).toBe(true); - }); - - it("should not be renderable when one item is not", () => { - const result = isItemRenderableByVersion( - { - _multi: { - sharedContext: { - __type: "content", - ...sampleV0InputNumberItem.question, - }, - questions: [ - { - __type: "content", - ...sampleImpossibleWidgetsItem.question, - }, - { - __type: "content", - ...sampleV1MeasurerItem.question, - }, - ], - }, - }, - PerseusItemVersion, - ); - expect(result).toBe(false); - }); - }); }); }); diff --git a/packages/perseus/src/a11y.ts b/packages/perseus/src/a11y.ts index 2803f6db7f..778d79689b 100644 --- a/packages/perseus/src/a11y.ts +++ b/packages/perseus/src/a11y.ts @@ -5,12 +5,9 @@ import _ from "underscore"; -import MultiItems from "./multi-items"; import {traverse} from "./traversal"; import * as Widgets from "./widgets"; -const {findContentNodesInItem, inferItemShape} = MultiItems; - // Iterate over a single Perseus renderer, mutating `widgets` by appending // violating widget types discovered in this item. function traverseRenderer(itemData, widgets: Array) { @@ -31,14 +28,7 @@ export function violatingWidgets(itemData: any): any { // TODO(jordan): Hints as well const widgets = []; - if (itemData._multi) { - const shape = inferItemShape(itemData); - findContentNodesInItem(itemData, shape, (content) => - traverseRenderer(content, widgets), - ); - } else { - traverseRenderer(itemData.question, widgets); - } + traverseRenderer(itemData.question, widgets); // Uniquify the list of widgets (by type) return _.uniq(widgets); diff --git a/packages/perseus/src/index.ts b/packages/perseus/src/index.ts index 3c43155d69..9c071022e0 100644 --- a/packages/perseus/src/index.ts +++ b/packages/perseus/src/index.ts @@ -229,39 +229,11 @@ export type { PerseusWidget, PerseusWidgetsMap, PerseusWidgetTypes, - MultiItem, WidgetOptions, } from "./perseus-types"; export type {UserInputMap} from "./validation.types"; export type {Coord} from "./interactive2/types"; export type {MarkerType} from "./widgets/label-image/types"; - -/** - * Multi-items - */ -export {default as MultiItems} from "./multi-items"; -export {buildEmptyItemTreeForShape, itemToTree} from "./multi-items/items"; -export {default as shapes} from "./multi-items/shapes"; -export {buildMapper} from "./multi-items/trees"; - -export type { - ContentNode, - HintNode, - Item, - ItemTree, - ItemObjectNode, - ItemArrayNode, - TagsNode, -} from "./multi-items/item-types"; -export type { - Shape, - ArrayShape, - ObjectShape, - ContentShape, - HintShape, - TagsShape, -} from "./multi-items/shape-types"; -export type {Path} from "./multi-items/trees"; export type { RendererPromptJSON, WidgetPromptJSON, diff --git a/packages/perseus/src/multi-items.ts b/packages/perseus/src/multi-items.ts deleted file mode 100644 index d347764828..0000000000 --- a/packages/perseus/src/multi-items.ts +++ /dev/null @@ -1,75 +0,0 @@ -/** - * This library provides support for Perseus multi-items: structured Perseus - * content that content creators can easily create, and that applications can - * easily render into different parts of the layout. - * - * For more details about application and motivation, see: - * https://sites.google.com/a/khanacademy.org/forge/for-developers/perseus-items-and-multi-items - * - * This file primarily exposes the `MultiRenderer` component, which performs - * multi-rendering. To multi-render a question, pass in the content of the item - * to the `MultiRenderer` component as a props. Then, pass in a function which - * takes an object of renderers (in the same structure as the content), and - * return a render tree. The `MultiRenderer` component will allow you to - * combine scores, serialized state, etc. without having to manually call on - * each of the functions. It also handles inter-widgets requests between the - * different renderers. - * For more details, see `multi-items/multi-renderer.jsx`. - * - * Example: - * - * item = {_multi: { - * left: , - * right: [, ], - * }} - * shape = shapes.shape({ - * left: shapes.content, - * right: shapes.arrayOf(shapes.content), - * }) - * - * - * {({renderers}) => - *
- *
{renderers.left}
- * - *
- * } - * - * - * This file also exposes `shapes`, which helps you construct a runtime type - * declaration for your particular class of multi-item. This can then be used - * to create a MultirendererEditor for your multi-item shape, and to validate - * that a multi-item conforms to the shape via `buildPropTypeForShape`. - * For more details, see `multi-items/shapes.js`. - * - * This file also exposes some utility functions for working with generic - * multi-items, like `findContentNodesInItem`, `findHintNodesInItem`, - * `inferItemShape`, and `buildEmptyItemForShape`. - * For more details, see `multi-items/items.js`. - */ -import { - buildEmptyItemForShape, - findContentNodesInItem, - findHintNodesInItem, - inferItemShape, -} from "./multi-items/items"; -import MultiRenderer from "./multi-items/multi-renderer"; -import {buildPropTypeForShape} from "./multi-items/prop-type-builders"; -import shapes from "./multi-items/shapes"; - -export default { - // Tools for rendering your multi-items - MultiRenderer, - - // Tools for declaring your multi-item shapes - shapes, - buildPropTypeForShape, - - // Tools for generically manipulating multi-items - buildEmptyItemForShape, - findContentNodesInItem, - findHintNodesInItem, - inferItemShape, -}; diff --git a/packages/perseus/src/multi-items/__stories__/multi-renderer.stories.tsx b/packages/perseus/src/multi-items/__stories__/multi-renderer.stories.tsx deleted file mode 100644 index 04921926dd..0000000000 --- a/packages/perseus/src/multi-items/__stories__/multi-renderer.stories.tsx +++ /dev/null @@ -1,80 +0,0 @@ -import {View} from "@khanacademy/wonder-blocks-core"; -import {HeadingSmall} from "@khanacademy/wonder-blocks-typography"; -import {StyleSheet} from "aphrodite"; -import * as React from "react"; - -import {MultiItemRendererWithDebugUI} from "../../../../../testing/multi-item-renderer-with-debug-ui"; -import {question1} from "../__testdata__/multi-renderer.testdata"; - -type StoryArgs = Record; - -type Story = { - title: string; -}; - -export const SingleItem = (args: StoryArgs): React.ReactElement => { - const item = { - _multi: { - ...question1._multi, - blurb: { - ...question1._multi.blurb, - content: - "This is a short snippet to help you understand the context of the question. We call it the 'blurb'.", - }, - }, - } as const; - return ( - - {({renderers}) => { - const {blurb, question, hints} = renderers; - return ( - - - - Blurb - - {blurb} - - - - Question - - {question} - - - - Hints - - - {// @ts-expect-error [FEI-5003] - TS2339 - Property 'firstN' does not exist on type 'readonly ReactNode[]'. - hints?.firstN(2)} - - - - ); - }} - - ); -}; - -const styles = StyleSheet.create({ - section: { - backgroundColor: "#F5F5F5", - padding: "5px", - borderWidth: "1px", - marginTop: "5px", - marginBottom: "5px", - }, - heading: { - backgroundColor: "#A9A9A9", - margin: "-5px", - padding: "5px", - }, - hints: { - marginLeft: "50px", - }, -}); - -export default { - title: "Perseus/Renderers/Multi Renderer", -} as Story; diff --git a/packages/perseus/src/multi-items/__testdata__/multi-renderer.testdata.ts b/packages/perseus/src/multi-items/__testdata__/multi-renderer.testdata.ts deleted file mode 100644 index 0bb6ab4e6b..0000000000 --- a/packages/perseus/src/multi-items/__testdata__/multi-renderer.testdata.ts +++ /dev/null @@ -1,176 +0,0 @@ -import shapes from "../shapes"; - -import type {Item} from "../item-types"; -import type {Shape} from "../shape-types"; - -export const simpleQuestionShape: Shape = shapes.shape({ - blurb: shapes.content, - question: shapes.content, - hints: shapes.hints, -}); - -// Shape: simpleQuestionShape -export const question1: Item = { - _multi: { - blurb: { - __type: "content", - content: "", - images: {}, - widgets: {}, - }, - hints: [ - { - __type: "hint", - content: - "If two triangles are congruent, then they have the same side lengths and angle measures.", - images: {}, - replace: false, - widgets: {}, - }, - { - __type: "hint", - content: - "A triangle congruent to triangle $ABC$ must also have side lengths of $12$, $14$ and $20$.", - images: {}, - replace: false, - widgets: {}, - }, - { - __type: "hint", - content: - "The following triangle is congruent to triangle $ABC$:\n\n* A triangle with side lengths of $12$, $14$, and $20$", - images: {}, - replace: false, - widgets: {}, - }, - ], - question: { - __type: "content", - content: - "Triangle $ABC$ has side lengths of $12$, $14$, and $20$. Which of the following triangles is congruent to triangle $ABC$ ?\n\n[[☃ radio 1]]\n\nEnter the number 3 into this field: [[☃ input-number 1]]", - widgets: { - "radio 1": { - alignment: "default", - graded: true, - options: { - choices: [ - { - clue: "Congruent triangles have the same side lengths.", - content: - "A triangle with side lengths of $3$, $4$, and $5$", - correct: false, - }, - { - clue: "Congruent triangles have the same side lengths.\n\nThis choice is similar to triangle $ABC$.", - content: - "A triangle with side lengths of $6$, $7$, and $10$", - correct: false, - }, - { - clue: "Congruent triangles have the same side lengths.", - content: - "A triangle with side lengths of $10$, $12$, and $18$", - correct: false, - isNoneOfTheAbove: false, - }, - { - clue: "Congruent triangles have the same side lengths.", - content: - "A triangle with side lengths of $12$, $14$, and $20$", - correct: true, - isNoneOfTheAbove: false, - }, - { - clue: "Congruent triangles have the same side lengths.\n\nThis choice is similar to triangle $ABC$.", - content: - "A triangle with side lengths of $24$, $28$, and $40$", - correct: false, - isNoneOfTheAbove: false, - }, - ], - countChoices: false, - deselectEnabled: false, - displayCount: null, - hasNoneOfTheAbove: false, - multipleSelect: false, - randomize: false, - }, - static: false, - type: "radio", - version: { - major: 1, - minor: 0, - }, - }, - "input-number 1": { - type: "input-number", - graded: true, - options: { - answerType: "number", - value: "-42", - simplify: "required", - size: "normal", - inexact: false, - maxError: 0.1, - }, - }, - }, - }, - }, -}; - -const definitionWidgetConfig = { - graded: true, - version: {major: 0, minor: 0}, - static: false, - type: "definition", - options: { - togglePrompt: "word", - definition: "", - static: false, - }, - alignment: "default", -} as const; - -// Shape: simpleQuestionShape -export const definitionQuestion1: Item = { - _multi: { - blurb: { - __type: "content", - content: "Here's a blurb [[☃ definition 1]]", - images: {}, - widgets: { - "definition 1": definitionWidgetConfig, - }, - }, - hints: [ - { - __type: "hint", - content: "Hint #1 [[☃ definition 2]]", - images: {}, - replace: false, - widgets: { - "definition 2": definitionWidgetConfig, - }, - }, - { - __type: "hint", - content: "Hint #1 [[☃ definition 3]]", - images: {}, - replace: false, - widgets: { - "definition 3": definitionWidgetConfig, - }, - }, - ], - question: { - __type: "content", - content: - "What is the answer to this?\n\n[[☃ definition 4]]\n\n[[☃ definition 5]]", - widgets: { - "definition 4": definitionWidgetConfig, - "definition 5": definitionWidgetConfig, - }, - }, - }, -}; diff --git a/packages/perseus/src/multi-items/__tests__/__snapshots__/multi-renderer.test.tsx.snap b/packages/perseus/src/multi-items/__tests__/__snapshots__/multi-renderer.test.tsx.snap deleted file mode 100644 index 286f99fa5e..0000000000 --- a/packages/perseus/src/multi-items/__tests__/__snapshots__/multi-renderer.test.tsx.snap +++ /dev/null @@ -1,1181 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`multi-item renderer should snapshot: initial render 1`] = ` -
-
-
-
-
- - -
-
-
-
-
-
-
- Triangle - - - - ABC - - - - has side lengths of - - - - 12 - - - - , - - - - 14 - - - - , and - - - - 20 - - - - . Which of the following triangles is congruent to triangle - - - - ABC - - - - ? -
-
-
-
-
-
-
- - Choose 1 answer: - - -
    -
  • -
    -
    -
    - -
    - -
    -
    -
    • -
  • -
    -
    -
    - -
    - -
    -
    -
    • -
  • -
    -
    -
    - -
    - -
    -
    -
    • -
  • -
    -
    -
    - -
    - -
    -
    -
    • -
  • -
    -
    -
    - -
    - -
    -
    -
    • -
-
-
-
-
-
-
-
- Enter the number 3 into this field: -
- -
- - -
-
- -
-
-
-
-
-
-
- - Hint #1 - - - 1 / 3 - -
-
-
- If two triangles are congruent, then they have the same side lengths and angle measures. -
-
-
-
-
-
-
-
-`; diff --git a/packages/perseus/src/multi-items/__tests__/items.test.ts b/packages/perseus/src/multi-items/__tests__/items.test.ts deleted file mode 100644 index 0206670540..0000000000 --- a/packages/perseus/src/multi-items/__tests__/items.test.ts +++ /dev/null @@ -1,240 +0,0 @@ -import { - buildEmptyItemTreeForShape, - buildEmptyItemForShape, - findContentNodesInItem, - findHintNodesInItem, - inferItemShape, - itemToTree, - treeToItem, -} from "../items"; -import shapes from "../shapes"; - -import type { - Item, - ItemTree, - ContentNode, - HintNode, - TagsNode, - ItemArrayNode, -} from "../item-types"; -import type { - ContentShape, - HintShape, - TagsShape, - ArrayShape, - ObjectShape, -} from "../shape-types"; - -describe("treeToItem", () => { - it("wraps an item tree in the `_multi` key", () => { - const tree = {__type: "hint"} as const; - expect({_multi: tree}).toEqual(treeToItem(tree)); - }); -}); - -describe("itemToTree", () => { - it("unwraps an item tree from the `_multi` key", () => { - const tree = {__type: "hint"} as const; - expect(tree).toEqual(itemToTree({_multi: tree})); - }); -}); - -describe("buildEmptyItemTreeForShape and buildEmptyItemForShape", () => { - const expectedEmptyContentNode = { - content: "", - images: {}, - widgets: {}, - __type: "content", - } as const; - - const expectedEmptyHintNode = { - replace: false, - content: "", - images: {}, - widgets: {}, - __type: "hint", - } as const; - - function assertEmptyItemTreeForShape( - expectedEmptyTree: ItemTree, - shape: ContentShape | HintShape | TagsShape | ArrayShape | ObjectShape, - ) { - const emptyTree = buildEmptyItemTreeForShape(shape); - expect(emptyTree).toEqual(expectedEmptyTree); - - const expectedEmptyItem = treeToItem(expectedEmptyTree); - const emptyItem = buildEmptyItemForShape(shape); - expect(emptyItem).toEqual(expectedEmptyItem); - } - - it("creates an empty item", () => { - assertEmptyItemTreeForShape(expectedEmptyContentNode, shapes.content); - }); - - it("creates an empty hint", () => { - assertEmptyItemTreeForShape(expectedEmptyHintNode, shapes.hint); - }); - - it("creates empty tags", () => { - assertEmptyItemTreeForShape([] as TagsNode, shapes.tags); - }); - - it("creates an empty array", () => { - assertEmptyItemTreeForShape( - [] as ItemArrayNode, - shapes.arrayOf(shapes.content), - ); - }); - - it("creates an empty object containing all node types", () => { - const shape = shapes.shape({ - instructions: shapes.content, - hint: shapes.hint, - questions: shapes.arrayOf(shapes.content), - context: shapes.shape({ - prompt: shapes.content, - footnotes: shapes.content, - }), - }); - const expectedEmptyTree = { - instructions: expectedEmptyContentNode, - hint: expectedEmptyHintNode, - questions: [] as ItemArrayNode, - context: { - prompt: expectedEmptyContentNode, - footnotes: expectedEmptyContentNode, - }, - } as const; - assertEmptyItemTreeForShape(expectedEmptyTree, shape); - }); -}); - -describe("inferItemShape", () => { - it("infers a content node's shape", () => { - const item = buildEmptyItemForShape(shapes.content); - expect(shapes.content).toEqual(inferItemShape(item)); - }); - - // TODO(mdr): Remove #LegacyContentNode support. - it("infers a legacy content node's shape", () => { - const item = { - _multi: { - __type: "item", - content: "", - images: {}, - widgets: {}, - }, - } as const; - expect(shapes.content).toEqual(inferItemShape(item)); - }); - - it("infers a hint node's shape", () => { - const item = buildEmptyItemForShape(shapes.hint); - expect(shapes.hint).toEqual(inferItemShape(item)); - }); - - it("infers a tags node's shape", () => { - const item = treeToItem(["foo", "bar"] as TagsNode); - expect(shapes.tags).toEqual(inferItemShape(item)); - }); - - it("infers an object node's shape", () => { - const shape = shapes.shape({ - content: shapes.content, - hint: shapes.hint, - }); - const item = buildEmptyItemForShape(shape); - expect(shape).toEqual(inferItemShape(item)); - }); - - it("poorly infers an empty array node's shape", () => { - const item = treeToItem([] as ItemArrayNode); - expect(shapes.arrayOf(shapes.content)).toEqual(inferItemShape(item)); - }); - - it("correctly infers an nonempty single-typed array node's shape", () => { - const item = treeToItem([ - /** - * TODO(somewhatabstract, JIRA-XXXX): - * The Tree types are really hard to work with properly. - */ buildEmptyItemTreeForShape(shapes.hint), - buildEmptyItemTreeForShape(shapes.hint), - buildEmptyItemTreeForShape(shapes.hint), - ] as ItemArrayNode); - expect(shapes.arrayOf(shapes.hint)).toEqual(inferItemShape(item)); - }); - - it("poorly infers an invalid multi-type array node's shape", () => { - const item = treeToItem([ - /** - * TODO(somewhatabstract, JIRA-XXXX): - * The Tree types are really hard to work with properly. - */ buildEmptyItemTreeForShape(shapes.hint), - buildEmptyItemTreeForShape(shapes.content), - buildEmptyItemTreeForShape(shapes.hint), - ] as ItemArrayNode); - expect(shapes.arrayOf(shapes.hint)).toEqual(inferItemShape(item)); - }); -}); - -function content(n: number): ContentNode { - return { - __type: "content", - content: `content ${n}`, - }; -} - -function hint(n: number): HintNode { - return { - __type: "hint", - content: `hint ${n}`, - }; -} - -const shape = shapes.shape({ - a: shapes.content, - b: shapes.arrayOf(shapes.content), - c: shapes.shape({ - d: shapes.content, - e: shapes.hint, - }), - f: shapes.hint, -}); - -const item: Item = treeToItem({ - a: content(1), - /** - * TODO(somewhatabstract, JIRA-XXXX): - * The Tree types are really hard to work with properly. - */ b: [content(2), content(3), content(4)] as ItemArrayNode, - c: { - d: content(5), - e: hint(6), - }, - f: hint(7), -}); - -describe("findContentNodesInItem", () => { - it("calls the callback for each content node in the item", () => { - const contents = []; - // @ts-expect-error - TS2345 - Argument of type 'ContentNode' is not assignable to parameter of type 'never'. - findContentNodesInItem(item, shape, (c) => contents.push(c)); - contents.sort(); - expect([ - content(1), - content(2), - content(3), - content(4), - content(5), - ]).toEqual(contents); - }); -}); - -describe("findHintNodesInItem", () => { - it("calls the callback for each hint node in the item", () => { - const hints: Array = []; - findHintNodesInItem(item, shape, (c) => hints.push(c)); - hints.sort(); - expect([hint(6), hint(7)]).toEqual(hints); - }); -}); diff --git a/packages/perseus/src/multi-items/__tests__/multi-renderer.test.tsx b/packages/perseus/src/multi-items/__tests__/multi-renderer.test.tsx deleted file mode 100644 index 11bd727b6c..0000000000 --- a/packages/perseus/src/multi-items/__tests__/multi-renderer.test.tsx +++ /dev/null @@ -1,906 +0,0 @@ -import {RenderStateRoot} from "@khanacademy/wonder-blocks-core"; -import {act, render, screen} from "@testing-library/react"; -import {userEvent as userEventLib} from "@testing-library/user-event"; -import * as React from "react"; - -import { - testDependencies, - testDependenciesV2, -} from "../../../../../testing/test-dependencies"; -import * as Dependencies from "../../dependencies"; -import {registerAllWidgetsForTesting} from "../../util/register-all-widgets-for-testing"; -import { - definitionQuestion1, - question1, - simpleQuestionShape, -} from "../__testdata__/multi-renderer.testdata"; -import MultiRenderer from "../multi-renderer"; -import shapes from "../shapes"; - -import type {FilterCriterion, Widget} from "../../types"; -import type {Item} from "../item-types"; -import type {Tree} from "../tree-types"; -import type {UserEvent} from "@testing-library/user-event"; - -// A little helper used in the render callback of a MultiRenderer. -type Props = {renderers: any}; -const SimpleLayout = ({renderers}: Props): React.ReactElement => { - if (renderers == null) { - throw new Error("renderers was null"); - } - - const {blurb, question, hints} = renderers; - return ( -
-
{blurb}
-
{question}
-
{hints?.firstN(1)}
-
- ); -}; - -// Note that the `question` passed in _must_ be of the simpleQuestionShape -// shape. -const renderSimpleQuestion = (question: Item) => { - // Arrange - let renderer: MultiRenderer | null | undefined = null; - const result = render( - - (renderer = r)} - dependencies={testDependenciesV2} - > - {({renderers}) => } - - , - ); - if (renderer == null) { - throw new Error("Rendering failed."); - } - - return { - ...result, - renderer, - }; -}; - -// A test helper to find widgets in a MultiRenderer. -const _findWidgets = ( - renderer: MultiRenderer, - filterCriterion: FilterCriterion, - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. -): Tree< - ReadonlyArray, - ReadonlyArray, - null -> => { - return renderer._mapRenderers((data) => { - if (data.ref == null) { - return []; - } - - // Note we use findInternalWidgets() here instead of - // _findWidgets(). Otherwise we get the same widget appearing in - // multiple places in the result tree. - return data.ref.findInternalWidgets(filterCriterion); - }); -}; - -describe("multi-item renderer", () => { - let userEvent: UserEvent; - beforeEach(() => { - userEvent = userEventLib.setup({ - advanceTimers: jest.advanceTimersByTime, - }); - - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); - registerAllWidgetsForTesting(); - }); - - it("should snapshot", () => { - // Arrange and Act - const {container} = renderSimpleQuestion(question1); - - // Assert - expect(container).toMatchSnapshot("initial render"); - }); - - it("should render the error if one occurs (item doesn't match shape)", () => { - // Arrange - // We mock out `console.error` because the we're explicitly - // triggering and error, but we don't want that to fail the test. - jest.spyOn(console, "error").mockImplementation(() => {}); - - // Act - render( - - - {({renderers}) => { - return
; - }} - - , - ); - - // Assert - expect(screen.getByText(/Error rendering:/)).toBeInTheDocument(); - }); - - describe("state serialization", () => { - it("should return serialized state from all items", async () => { - // Arrange - const {renderer} = renderSimpleQuestion(question1); - - // Nudge the widget to a non-default state (ie. an item is - // selected, a value is entered). You can see the result of this in the `choiceStates` - // array in the captured state below where the choice at index 2 has - // `"selected": true` (instead of false) and the input-number has a `currentValue`. - await userEvent.click(screen.getAllByRole("radio")[2]); // Correct - await userEvent.type(screen.getByRole("textbox"), "+42"); // Correct - - // Act - // @ts-expect-error - TS2339 - Property '_getSerializedState' does not exist on type 'never'. - const state = renderer._getSerializedState(null); - - // Assert - expect(state).toMatchInlineSnapshot(` - { - "blurb": {}, - "hints": [ - null, - null, - null, - ], - "question": { - "input-number 1": { - "answerType": "number", - "currentValue": "+42", - "rightAlign": undefined, - "simplify": "required", - "size": "normal", - }, - "radio 1": { - "choiceStates": [ - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": true, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - ], - "choices": [ - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $3$, $4$, and $5$", - "correct": false, - "originalIndex": 0, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $6$, $7$, and $10$", - "correct": false, - "originalIndex": 1, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $10$, $12$, and $18$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 2, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $12$, $14$, and $20$", - "correct": true, - "isNoneOfTheAbove": false, - "originalIndex": 3, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $24$, $28$, and $40$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 4, - }, - ], - "countChoices": false, - "deselectEnabled": false, - "hasNoneOfTheAbove": false, - "multipleSelect": false, - "numCorrect": 1, - "selectedChoices": [ - false, - false, - false, - true, - false, - ], - }, - }, - } - `); - }); - - it("should return values from lastSerializedState if ref's getSerializedState returns null", async () => { - // Arrange - const {renderer} = renderSimpleQuestion(question1); - - await userEvent.click(screen.getAllByRole("radio")[2]); - await userEvent.type(screen.getByRole("textbox"), "99"); - - // Act - // @ts-expect-error - TS2339 - Property '_getSerializedState' does not exist on type 'never'. - const state = renderer._getSerializedState({ - blurb: "last blurb", - hints: ["uno", "dos" /* intentionally not passing a third */], - }); - - // Assert - expect(state).toMatchInlineSnapshot(` - { - "blurb": {}, - "hints": [ - "uno", - "dos", - undefined, - ], - "question": { - "input-number 1": { - "answerType": "number", - "currentValue": "99", - "rightAlign": undefined, - "simplify": "required", - "size": "normal", - }, - "radio 1": { - "choiceStates": [ - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": true, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - ], - "choices": [ - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $3$, $4$, and $5$", - "correct": false, - "originalIndex": 0, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $6$, $7$, and $10$", - "correct": false, - "originalIndex": 1, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $10$, $12$, and $18$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 2, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $12$, $14$, and $20$", - "correct": true, - "isNoneOfTheAbove": false, - "originalIndex": 3, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $24$, $28$, and $40$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 4, - }, - ], - "countChoices": false, - "deselectEnabled": false, - "hasNoneOfTheAbove": false, - "multipleSelect": false, - "numCorrect": 1, - "selectedChoices": [ - false, - false, - false, - true, - false, - ], - }, - }, - } - `); - }); - - it("should restore serialized state to each widget", () => { - // Arrange - const {renderer} = renderSimpleQuestion(question1); - - // This state was built by getting the renderer into this state - // using `userEvent` calls and then calling `getSerializedState()` - // on the renderer. - const state = { - blurb: {}, - hints: [null, null, null], - question: { - "input-number 1": { - answerType: "number", - currentValue: "+42", - rightAlign: false, - simplify: "required", - size: "normal", - }, - "radio 1": { - choiceStates: [ - { - correctnessShown: false, - crossedOut: false, - highlighted: false, - previouslyAnswered: false, - rationaleShown: false, - readOnly: false, - selected: false, - }, - { - correctnessShown: false, - crossedOut: false, - highlighted: false, - previouslyAnswered: false, - rationaleShown: false, - readOnly: false, - selected: false, - }, - { - correctnessShown: false, - crossedOut: false, - highlighted: false, - previouslyAnswered: false, - rationaleShown: false, - readOnly: false, - selected: true, - }, - { - correctnessShown: false, - crossedOut: false, - highlighted: false, - previouslyAnswered: false, - rationaleShown: false, - readOnly: false, - selected: false, - }, - { - correctnessShown: false, - crossedOut: false, - highlighted: false, - previouslyAnswered: false, - rationaleShown: false, - readOnly: false, - selected: false, - }, - ], - choices: [ - { - clue: "Congruent triangles have the same side lengths.", - content: - "A triangle with side lengths of $3$, $4$, and $5$", - correct: false, - originalIndex: 0, - }, - { - clue: - "Congruent triangles have the same side lengths.\n" + - "\n" + - "This choice is similar to triangle $ABC$.", - content: - "A triangle with side lengths of $6$, $7$, and $10$", - correct: false, - originalIndex: 1, - }, - { - clue: "Congruent triangles have the same side lengths.", - content: - "A triangle with side lengths of $10$, $12$, and $18$", - correct: false, - isNoneOfTheAbove: false, - originalIndex: 2, - }, - { - clue: "Congruent triangles have the same side lengths.", - content: - "A triangle with side lengths of $12$, $14$, and $20$", - correct: true, - isNoneOfTheAbove: false, - originalIndex: 3, - }, - { - clue: - "Congruent triangles have the same side lengths.\n" + - "\n" + - "This choice is similar to triangle $ABC$.", - content: - "A triangle with side lengths of $24$, $28$, and $40$", - correct: false, - isNoneOfTheAbove: false, - originalIndex: 4, - }, - ], - countChoices: false, - deselectEnabled: false, - hasNoneOfTheAbove: false, - multipleSelect: false, - numCorrect: 1, - selectedChoices: [false, false, false, true, false], - }, - }, - } as const; - - // Act - // @ts-expect-error - TS2339 - Property 'restoreSerializedState' does not exist on type 'never'. - act(() => renderer.restoreSerializedState(state)); - - // Assert - expect(screen.getAllByRole("radio")[0]).not.toBeChecked(); - expect(screen.getAllByRole("radio")[1]).not.toBeChecked(); - expect(screen.getAllByRole("radio")[2]).toBeChecked(); - expect(screen.getAllByRole("radio")[3]).not.toBeChecked(); - expect(screen.getAllByRole("radio")[4]).not.toBeChecked(); - expect(screen.getByRole("textbox")).toHaveValue("+42"); - }); - - it("should call callback when restore serialized state is complete", () => { - // Arrange - const {renderer} = renderSimpleQuestion(definitionQuestion1); - const callback = jest.fn(); - - // Act - act(() => - // @ts-expect-error - TS2339 - Property 'restoreSerializedState' does not exist on type 'never'. - renderer.restoreSerializedState( - { - blurb: {"definition 1": 1}, - hints: [2, 3], - question: { - "definition 4": 4, - "definition 5": 5, - }, - }, - callback, - ), - ); - act(() => jest.runOnlyPendingTimers()); - - // Assert - expect(callback).toHaveBeenCalled(); - }); - }); - - it("should find widgets in other parts of the tree", () => { - // Arrange - const {renderer} = renderSimpleQuestion(definitionQuestion1); - // Grab "definition 5" out of the render tree. Note that the result - // here is a tree of the same shape as the `mockedQuestion1`'s shape. - // Each node in the tree is a `$ReadOnlyArray`. - const result = _findWidgets(renderer, (id) => id === "definition 5"); - - // Act - // We're using the API provided to the widget we found ("definition - // 5") to look into other parts of the render tree for another widget - // ("definition 1"). This verifies that the MultiRenderer has - // correctly passed down a functioning `findWidgets` prop to the - // widgets. - // Oh the TypeScript warnings we'll suppress. Welcome to multi items. - const widget1 = result.question[0].props.findWidgets("definition 1"); - - // Assert - expect(widget1).not.toBeNull(); - }); - - it("should return the scores in the shape of the tree", async () => { - // Arrange - const {renderer} = renderSimpleQuestion(question1); - - await userEvent.click(screen.getAllByRole("radio")[3]); // Correct - await userEvent.type(screen.getByRole("textbox"), "-42"); // Correct - - // Act - // @ts-expect-error - TS2339 - Property 'getScores' does not exist on type 'never'. - const score = renderer.getScores(); - - // Assert - expect(score).toMatchInlineSnapshot(` - { - "blurb": { - "correct": true, - "empty": false, - "guess": [], - "message": null, - "state": {}, - }, - "hints": [ - null, - null, - null, - ], - "question": { - "correct": true, - "empty": false, - "guess": [ - { - "choicesSelected": [ - false, - false, - false, - true, - false, - ], - }, - { - "currentValue": "-42", - }, - ], - "message": null, - "state": { - "input-number 1": { - "answerType": "number", - "currentValue": "-42", - "rightAlign": undefined, - "simplify": "required", - "size": "normal", - }, - "radio 1": { - "choiceStates": [ - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": true, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - ], - "choices": [ - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $3$, $4$, and $5$", - "correct": false, - "originalIndex": 0, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $6$, $7$, and $10$", - "correct": false, - "originalIndex": 1, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $10$, $12$, and $18$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 2, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $12$, $14$, and $20$", - "correct": true, - "isNoneOfTheAbove": false, - "originalIndex": 3, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $24$, $28$, and $40$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 4, - }, - ], - "countChoices": false, - "deselectEnabled": false, - "hasNoneOfTheAbove": false, - "multipleSelect": false, - "numCorrect": 1, - "selectedChoices": [ - false, - false, - false, - true, - false, - ], - }, - }, - }, - } - `); - }); - - it("should return the composite score for the whole tree", async () => { - // Arrange - const {renderer} = renderSimpleQuestion(question1); - - await userEvent.click(screen.getAllByRole("radio")[3]); // Correct - await userEvent.type(screen.getByRole("textbox"), "-42"); // Correct - - // Act - // @ts-expect-error - TS2339 - Property 'score' does not exist on type 'never'. - const score = renderer.score(); - - // Assert - expect(score).toMatchInlineSnapshot(` - { - "correct": true, - "empty": false, - "guess": { - "blurb": [], - "hints": [ - null, - null, - null, - ], - "question": [ - { - "choicesSelected": [ - false, - false, - false, - true, - false, - ], - }, - { - "currentValue": "-42", - }, - ], - }, - "message": null, - "state": [ - {}, - { - "input-number 1": { - "answerType": "number", - "currentValue": "-42", - "rightAlign": undefined, - "simplify": "required", - "size": "normal", - }, - "radio 1": { - "choiceStates": [ - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": true, - }, - { - "correctnessShown": false, - "crossedOut": false, - "highlighted": false, - "previouslyAnswered": false, - "rationaleShown": false, - "readOnly": false, - "selected": false, - }, - ], - "choices": [ - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $3$, $4$, and $5$", - "correct": false, - "originalIndex": 0, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $6$, $7$, and $10$", - "correct": false, - "originalIndex": 1, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $10$, $12$, and $18$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 2, - }, - { - "clue": "Congruent triangles have the same side lengths.", - "content": "A triangle with side lengths of $12$, $14$, and $20$", - "correct": true, - "isNoneOfTheAbove": false, - "originalIndex": 3, - }, - { - "clue": "Congruent triangles have the same side lengths. - - This choice is similar to triangle $ABC$.", - "content": "A triangle with side lengths of $24$, $28$, and $40$", - "correct": false, - "isNoneOfTheAbove": false, - "originalIndex": 4, - }, - ], - "countChoices": false, - "deselectEnabled": false, - "hasNoneOfTheAbove": false, - "multipleSelect": false, - "numCorrect": 1, - "selectedChoices": [ - false, - false, - false, - true, - false, - ], - }, - }, - ], - } - `); - }); -}); diff --git a/packages/perseus/src/multi-items/__tests__/prop-type-builders.test.ts b/packages/perseus/src/multi-items/__tests__/prop-type-builders.test.ts deleted file mode 100644 index 921d0efa54..0000000000 --- a/packages/perseus/src/multi-items/__tests__/prop-type-builders.test.ts +++ /dev/null @@ -1,133 +0,0 @@ -import {treeToItem} from "../items"; -import {buildPropTypeForShape} from "../prop-type-builders"; -import shapes from "../shapes"; - -// TODO(emily): [PERSEUS_MERGE] Calling prop types as a function fails these -// tests. Rewrite them once we're using the prop-types library. -describe.skip("buildPropTypeForShape", () => { - // The value we're wrapping might not be a valid ItemTree - that's the - // point of testing this PropType - so we disable type checking by casting - // `value` to type `any`. The current implementation of `treeToItem` can - // handle that possibility. - const tryPropType = (propType: any, value: any) => - propType({value: treeToItem(value)}, "value", ""); - - const assertPropTypePasses = (propType, value) => - expect(null).toEqual(tryPropType(propType, value)); - - const assertPropTypeFails = (propType, value) => - expect(tryPropType(propType, value) instanceof Error).toBeTruthy(); - - it("validates a content node", () => { - const propType = buildPropTypeForShape(shapes.content); - - // Perseus has default values for all item fields, so all except the - // type are optional. - assertPropTypePasses(propType, {__type: "content"}); - assertPropTypePasses(propType, { - content: "", - widgets: {}, - images: {}, - __type: "content", - }); - - // We also leave the full object optional by default, like the propType - // primitives. - assertPropTypePasses(propType, null); - - // But specifying a bad type for any field will fail the propType. - assertPropTypeFails(propType, {content: 1, __type: "content"}); - assertPropTypeFails(propType, {widgets: 1, __type: "content"}); - assertPropTypeFails(propType, {images: 1, __type: "content"}); - }); - - // TODO(mdr): Remove #LegacyContentNode support. - it("validates a legacy content node", () => { - const propType = buildPropTypeForShape(shapes.content); - - // Perseus has default values for all item fields, so all except the - // type are optional. - assertPropTypePasses(propType, {__type: "item"}); - assertPropTypePasses(propType, { - content: "", - widgets: {}, - images: {}, - __type: "item", - }); - - // We also leave the full object optional by default, like the propType - // primitives. - assertPropTypePasses(propType, null); - - // But specifying a bad type for any field will fail the propType. - assertPropTypeFails(propType, {content: 1, __type: "item"}); - assertPropTypeFails(propType, {widgets: 1, __type: "item"}); - assertPropTypeFails(propType, {images: 1, __type: "item"}); - }); - - it("validates a hint node", () => { - const propType = buildPropTypeForShape(shapes.hint); - - // Perseus has default values for all hint fields, so all except the - // type are optional. - assertPropTypePasses(propType, {__type: "hint"}); - assertPropTypePasses(propType, { - content: "", - widgets: {}, - images: {}, - replace: false, - __type: "hint", - }); - - // We also leave the full object optional by default, like the propType - // primitives. - assertPropTypePasses(propType, null); - - // But specifying a bad type for any field will fail the propType. - assertPropTypeFails(propType, {content: 1, __type: "hint"}); - assertPropTypeFails(propType, {widgets: 1, __type: "hint"}); - assertPropTypeFails(propType, {images: 1, __type: "hint"}); - assertPropTypeFails(propType, {replace: 1, __type: "hint"}); - }); - - it("validates an array", () => { - const propType = buildPropTypeForShape(shapes.arrayOf(shapes.content)); - const emptyItem = {__type: "content"} as const; - - assertPropTypePasses(propType, []); - assertPropTypePasses(propType, [emptyItem, emptyItem, emptyItem]); - - // While the array itself is optional, its elements are required. - assertPropTypePasses(propType, null); - assertPropTypeFails(propType, [emptyItem, null, emptyItem]); - }); - - it("validates tags", () => { - const propType = buildPropTypeForShape(shapes.tags); - - assertPropTypePasses(propType, []); - assertPropTypePasses(propType, ["a", "b", "c"]); - - assertPropTypePasses(propType, null); - assertPropTypeFails(propType, ["a", null, "b"]); - assertPropTypeFails(propType, [1, 2, 3]); - }); - - it("validates an object", () => { - const propType = buildPropTypeForShape( - shapes.shape({ - a: shapes.content, - b: shapes.content, - }), - ); - const emptyItem = {__type: "content"} as const; - - assertPropTypePasses(propType, {a: emptyItem, b: emptyItem}); - - // While the object itself is optional, its fields are required. - assertPropTypePasses(propType, null); - assertPropTypeFails(propType, {}); - assertPropTypeFails(propType, {a: emptyItem}); - assertPropTypeFails(propType, {b: emptyItem}); - }); -}); diff --git a/packages/perseus/src/multi-items/__tests__/shapes.test.ts b/packages/perseus/src/multi-items/__tests__/shapes.test.ts deleted file mode 100644 index f359f45cdf..0000000000 --- a/packages/perseus/src/multi-items/__tests__/shapes.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import shapes from "../shapes"; - -describe("shapes.content", () => { - it('has type "content"', () => { - const shape = shapes.content; - expect("content").toEqual(shape.type); - }); -}); - -describe("shapes.hint", () => { - it('has type "hint"', () => { - const shape = shapes.hint; - expect("hint").toEqual(shape.type); - }); -}); - -describe("shapes.arrayOf", () => { - it('has type "array"', () => { - const shape = shapes.arrayOf(shapes.content); - expect("array").toEqual(shape.type); - }); - - it("has the given shape as its `elementShape` property", () => { - const shape = shapes.arrayOf(shapes.content); - expect(shapes.content).toEqual(shape.elementShape); - }); -}); - -describe("shapes.shape", () => { - it('has type "object"', () => { - const shape = shapes.shape({}); - expect("object").toEqual(shape.type); - }); - - it("has the given shapes as its `shape` property", () => { - const shape = shapes.shape({ - foo: shapes.content, - bar: shapes.hint, - }); - expect(shapes.content).toEqual(shape.shape.foo); - expect(shapes.hint).toEqual(shape.shape.bar); - }); -}); diff --git a/packages/perseus/src/multi-items/__tests__/trees.test.ts b/packages/perseus/src/multi-items/__tests__/trees.test.ts deleted file mode 100644 index 96b2b6dfaa..0000000000 --- a/packages/perseus/src/multi-items/__tests__/trees.test.ts +++ /dev/null @@ -1,317 +0,0 @@ -import shapes from "../shapes"; -import {buildMapper} from "../trees"; - -import type { - ItemTree, - ContentNode, - HintNode, - TagsNode, - ItemArrayNode, -} from "../item-types"; -import type {TreeMapper} from "../trees"; - -describe("buildMapper", () => { - function content(n): ContentNode { - return { - __type: "content", - content: `content ${n}`, - }; - } - - function hint(n: number): HintNode { - return { - __type: "hint", - content: `hint ${n}`, - }; - } - - function tags(...elements): TagsNode { - // This function mostly exists as a shorthand way to clarify to TypeScript - // that this is a TagsNode, not a confused ItemArrayNode. - return elements; - } - - function array(...elements): ItemArrayNode { - // This function mostly exists as a shorthand way to clarify to TypeScript - // that this is an ItemArrayNode, not a confused TagsNode. - /** - * TODO(somewhatabstract, JIRA-XXXX): - * The Tree types are really hard to work with properly. - */ - return elements; - } - - const shape = shapes.shape({ - a: shapes.content, - b: shapes.arrayOf(shapes.content), - c: shapes.shape({ - d: shapes.content, - e: shapes.hint, - }), - f: shapes.hint, - g: shapes.tags, - }); - - const tree: ItemTree = { - a: content(1), - b: array(content(2), content(3), content(4)), - c: { - d: content(5), - e: hint(6), - }, - f: hint(7), - g: tags("foo", "bar"), - }; - - it("calls the content mapper for each of the content nodes", () => { - const calledWith = []; - buildMapper() - // @ts-expect-error - TS2345 - Argument of type 'unknown' is not assignable to parameter of type 'never'. - .setContentMapper((c) => calledWith.push(c)) - .mapTree(tree, shape); - calledWith.sort(); - - expect([ - content(1), - content(2), - content(3), - content(4), - content(5), - ]).toEqual(calledWith); - }); - - it("calls the hint mapper for each of the hint nodes", () => { - const calledWith = []; - buildMapper() - // @ts-expect-error - TS2345 - Argument of type 'unknown' is not assignable to parameter of type 'never'. - .setHintMapper((h) => calledWith.push(h)) - .mapTree(tree, shape); - calledWith.sort(); - - expect([hint(6), hint(7)]).toEqual(calledWith); - }); - - it("calls the tags mapper for each of the tags nodes", () => { - const calledWith: Array = []; - buildMapper() - .setTagsMapper((t) => calledWith.push(t)) - .mapTree(tree, shape); - calledWith.sort(); - - expect([["foo", "bar"]]).toEqual(calledWith); - }); - - it("returns a mapped tree with the correct shape", () => { - const mapper: TreeMapper< - ContentNode, - string, - HintNode, - string, - TagsNode, - string - > = buildMapper() - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setContentMapper((c) => `mapped content: ${c.content || ""}`) - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setHintMapper((h) => `mapped hint: ${h.content || ""}`) - .setTagsMapper((t) => `mapped tags: ${t.join(", ")}`); - const result = mapper.mapTree(tree, shape); - - expect({ - a: "mapped content: content 1", - b: [ - "mapped content: content 2", - "mapped content: content 3", - "mapped content: content 4", - ], - c: { - d: "mapped content: content 5", - e: "mapped hint: hint 6", - }, - f: "mapped hint: hint 7", - g: "mapped tags: foo, bar", - }).toEqual(result); - }); - - it("provides each node shape to the leaf mappers", () => { - // Return the type of the shape of the node in place of each node. - const result = buildMapper() - .setContentMapper((_, s) => s.type) - .setHintMapper((_, s) => s.type) - .setTagsMapper((_, s) => s.type) - .mapTree(tree, shape); - - expect({ - a: "content", - b: ["content", "content", "content"], - c: { - d: "content", - e: "hint", - }, - f: "hint", - g: "tags", - }).toEqual(result); - }); - - it("provides each node path to the leaf mappers", () => { - // Return the path to the node in place of each node. - const result = buildMapper() - .setContentMapper((_, __, p) => p) - .setHintMapper((_, __, p) => p) - .setTagsMapper((_, __, p) => p) - .mapTree(tree, shape); - - expect({ - a: ["a"], - b: [ - ["b", 0], - ["b", 1], - ["b", 2], - ], - c: { - d: ["c", "d"], - e: ["c", "e"], - }, - f: ["f"], - g: ["g"], - }).toEqual(result); - }); - - it("handles recursive objects", () => { - const shape = shapes.shape({ - a: shapes.shape({ - b: shapes.shape({ - c: shapes.content, - }), - }), - }); - - const tree = {a: {b: {c: content(1)}}} as const; - - // @ts-expect-error - TS2322 - Type 'TreeMapperJustForLeaves' is not assignable to type 'TreeMapper'. - const mapper: TreeMapper< - ContentNode, - string | null | undefined, - HintNode, - HintNode, - TagsNode, - TagsNode - > = buildMapper().setContentMapper((c) => c.content); - const result = mapper.mapTree(tree, shape); - - expect({a: {b: {c: "content 1"}}}).toEqual(result); - }); - - it("handles recursive arrays", () => { - const shape = shapes.arrayOf( - shapes.arrayOf(shapes.arrayOf(shapes.content)), - ); - - const tree = array( - array(array(content(0)), array(content(1))), - array(array(content(2)), array(content(3), content(4))), - ); - - // @ts-expect-error - TS2322 - Type 'TreeMapperJustForLeaves' is not assignable to type 'TreeMapper'. - const mapper: TreeMapper< - ContentNode, - string | null | undefined, - HintNode, - HintNode, - TagsNode, - TagsNode - > = buildMapper().setContentMapper((c) => c.content); - const result = mapper.mapTree(tree, shape); - - expect([ - [["content 0"], ["content 1"]], - [["content 2"], ["content 3", "content 4"]], - ]).toEqual(result); - }); - - it("handles empty arrays", () => { - const shape = shapes.arrayOf(shapes.arrayOf(shapes.content)); - - const tree = array(array(), array(content(1)), array()); - - // @ts-expect-error - TS2322 - Type 'TreeMapperJustForLeaves' is not assignable to type 'TreeMapper'. - const mapper: TreeMapper< - ContentNode, - string | null | undefined, - HintNode, - HintNode, - TagsNode, - TagsNode - > = buildMapper().setContentMapper((c) => c.content); - - // Test the outer array being empty. - expect([]).toEqual(mapper.mapTree(array(), shape)); - - // Test inner arrays being empty. - const result = mapper.mapTree(tree, shape); - expect([[], ["content 1"], []]).toEqual(result); - }); - - it("calls the array mapper for arrays", () => { - const shape = shapes.arrayOf(shapes.content); - const tree = array(content(1), content(2), content(3)); - - let wasCalled = false; - let callArgs: Record = {}; - // @ts-expect-error - TS2322 - Type 'TreeMapperForLeavesAndCollections' is not assignable to type 'TreeMapper'. - const mapper: TreeMapper< - ContentNode, - string | null | undefined, - HintNode, - HintNode, - TagsNode, - TagsNode - > = buildMapper() - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setContentMapper((c) => c.content) - .setArrayMapper((mappedArray, originalArray, shape, path) => { - wasCalled = true; - callArgs = {mappedArray, originalArray, shape, path}; - return mappedArray; - }); - mapper.mapTree(tree, shape); - - expect(wasCalled).toBeTruthy(); - - expect(["content 1", "content 2", "content 3"]).toEqual( - callArgs.mappedArray, - ); - expect(tree).toEqual(callArgs.originalArray); - expect(shape).toEqual(callArgs.shape); - expect([]).toEqual(callArgs.path); - }); - - it("uses the array mapper return value to construct the new tree", () => { - const mapper = buildMapper() - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setContentMapper((c) => c.content) - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setHintMapper((h) => h.content) - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setTagsMapper((t) => t.join(", ")) - .setArrayMapper((mappedArray, originalArray, shape, path) => { - return mappedArray.map((child) => `${String(child)} in array`); - }); - const result = mapper.mapTree(tree, shape); - - expect({ - a: "content 1", - b: [ - "content 2 in array", - "content 3 in array", - "content 4 in array", - ], - c: { - d: "content 5", - e: "hint 6", - }, - f: "hint 7", - g: "foo, bar", - }).toEqual(result); - }); -}); diff --git a/packages/perseus/src/multi-items/item-types.ts b/packages/perseus/src/multi-items/item-types.ts deleted file mode 100644 index 3e1ba29066..0000000000 --- a/packages/perseus/src/multi-items/item-types.ts +++ /dev/null @@ -1,57 +0,0 @@ -/** - * Type definitions for multi-item types, including: - * - * - Item: A multi-item tree wrapped in a `_multi` key, to help us recognize it - * as a multi-item in other contexts and avoid misinterpreting its - * other properties. - * - ItemTree: A multi-item without the `_multi` key. Conforms to the Tree - * interface, so it's compatible with our tree traversal functions. - * - And the various types of nodes that compose a tree. - */ -import type {Tree, ArrayNode, ObjectNode} from "./tree-types"; -import type {PerseusWidgetsMap} from "../perseus-types"; -import type {ImageDict} from "../types"; - -export type ContentNode = { - // TODO(mdr): When we first drafted the multi-item feature, we named - // content nodes "item" nodes, and later decided the term was - // ambiguous and switched to "content". But we're temporarily keeping - // support for the "item" string when inferring item shape, so that we - // don't crash on multi-items we've already created - but all new - // content nodes will be generated with the "content" string. - // - // Code blocks that enable this legacy support are greppable with the - // keyword #LegacyContentNode. - __type: "content" | "item"; - // Perseus has default values for these fields, so they're all optional. - content?: string | null | undefined; - images?: ImageDict | null | undefined; - widgets?: PerseusWidgetsMap | null | undefined; -}; -export type HintNode = { - __type: "hint"; - // Perseus has default values for these fields, so they're all optional. - content?: string | null | undefined; - images?: ImageDict | null | undefined; - widgets?: PerseusWidgetsMap | null | undefined; - replace?: boolean | null | undefined; -}; -export type TagsNode = ReadonlyArray; - -// @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. -export type ItemArrayNode = ArrayNode; -export type ItemObjectNode = ObjectNode; -// @ts-expect-error - TS2315 - Type 'Tree' is not generic. -export type ItemTree = Tree; - -// TODO(jeremy): I think we could refine this root type for multi items. Right -// now a _multi's value can be _anything_ including "primitive" node types such -// as "type: content" or "type: hint". That doesn't seem to match the shapes -// that are in use to date. We could also move to a strict type at that point -// because it appears that we _never_ add other root keys to an item that -// specifies the `_multi` key. I think we'd be safe to restrict this root -// object type to something like the following: -// export type Item = {|_multi: ItemArrayNode | ItemObjectNode|}; -export type Item = { - _multi: ItemTree; -}; diff --git a/packages/perseus/src/multi-items/items.ts b/packages/perseus/src/multi-items/items.ts deleted file mode 100644 index 5666101a73..0000000000 --- a/packages/perseus/src/multi-items/items.ts +++ /dev/null @@ -1,175 +0,0 @@ -/** - * Utility functions for constructing and manipulating multi-items. - * - * These functions apply *specifically* to Items and ItemTrees - things that - * actually semantically *are* multi-items. For more general functions for - * traversing and manipulating *anything* shaped like a multi-item (like a - * renderer tree or a score tree or, well, a multi-item), see trees.js. - */ - -import {Errors, PerseusError} from "@khanacademy/perseus-core"; - -import shapes from "./shapes"; -import {buildMapper} from "./trees"; - -import type { - Item, - ItemTree, - ContentNode, - HintNode, - TagsNode, - ItemArrayNode, - ItemObjectNode, -} from "./item-types"; -import type {Shape} from "./shape-types"; - -/** - * Return a semantically empty ItemTree that conforms to the given shape. - * - * - An empty content node has an empty content string and no widgets/images. - * - An empty hint node has an empty content string and no widgets/images. - * - An empty array node has no elements. - * - An empty object node has a semantically empty node for each of its keys. - * (That is, we recursively call buildEmptyItemTreeForShape for each key.) - */ -export function buildEmptyItemTreeForShape(shape: Shape): ItemTree { - if (shape.type === "content") { - return { - __type: "content", - content: "", - images: {}, - widgets: {}, - }; - } - if (shape.type === "hint") { - return { - __type: "hint", - replace: false, - content: "", - images: {}, - widgets: {}, - }; - } - if (shape.type === "tags") { - return [] as TagsNode; - } - if (shape.type === "array") { - return [] as ItemArrayNode; - } - if (shape.type === "object") { - const valueShapes = shape.shape; - const object: ItemObjectNode = {}; - Object.keys(valueShapes).forEach((key) => { - object[key] = buildEmptyItemTreeForShape(valueShapes[key]); - }); - return object; - } - throw new PerseusError( - // @ts-expect-error - TS2339 - Property 'type' does not exist on type 'never'. - `unexpected shape type ${shape.type}`, - Errors.InvalidInput, - ); -} - -/** - * Return a semantically empty Item that conforms to the given shape. - * - * - An empty content node has an empty content string and no widgets/images. - * - An empty hint node has an empty content string and no widgets/images. - * - An empty array node has no elements. - * - An empty object node has a semantically empty node for each of its keys. - * (That is, we recursively call buildEmptyItemTreeForShape for each key.) - */ -export function buildEmptyItemForShape(shape: Shape): Item { - return treeToItem(buildEmptyItemTreeForShape(shape)); -} - -/** - * Given an Item and its Shape, yield all of its content nodes to the callback. - */ -export function findContentNodesInItem( - item: Item, - shape: Shape, - callback: (c: ContentNode) => any, -) { - const itemTree = itemToTree(item); - buildMapper().setContentMapper(callback).mapTree(itemTree, shape); -} - -/** - * Given an Item and its Shape, yield all of its hint nodes to the callback. - */ -export function findHintNodesInItem( - item: Item, - shape: Shape, - callback: (h: HintNode) => any, -) { - const itemTree = itemToTree(item); - buildMapper().setHintMapper(callback).mapTree(itemTree, shape); -} - -/** - * Given an ItemTree, return a Shape that it conforms to. - * - * The Shape might not be complete or correct Shape that this Item was designed - * for. If you have access to the intended Shape, use that instead. - */ -export function inferItemShape(item: Item): Shape { - const itemTree = itemToTree(item); - return inferItemTreeShape(itemTree); -} - -function inferItemTreeShape(node: ItemTree): Shape { - if (Array.isArray(node)) { - if (node.length) { - if (typeof node[0] === "string") { - // There's no ItemTree that can manifest as a string. - // So, an array of strings must be a TagsNode, not ArrayNode. - return shapes.tags; - } - // Otherwise, assume that this is a valid ArrayNode, and - // therefore the shape of the first element applies to all - // elements in the array. - return shapes.arrayOf(inferItemTreeShape(node[0])); - } - // The array is empty, so we arbitrarily guess that it's a content - // array. As discussed in the docstring, this might be incorrect, - // and you shouldn't depend on it. - return shapes.arrayOf(shapes.content); - } - if ( - // TODO(mdr): Remove #LegacyContentNode support. - typeof node === "object" && - (node.__type === "content" || node.__type === "item") - ) { - return shapes.content; - } - if (typeof node === "object" && node.__type === "hint") { - return shapes.hint; - } - if (typeof node === "object") { - const valueShapes: Record = {}; - Object.keys(node).forEach((key) => { - valueShapes[key] = inferItemTreeShape(node[key]); - }); - return shapes.shape(valueShapes); - } - throw new PerseusError( - `unexpected multi-item node ${JSON.stringify(node)}`, - Errors.InvalidInput, - ); -} - -/** - * Convert the given Item to an ItemTree, by unwrapping the `_multi` key. - */ -export function itemToTree(item: Item): ItemTree { - return item._multi; -} - -/** - * Convert the given ItemTree to an Item, by wrapping it in the `_multi` key. - */ -export function treeToItem(node: ItemTree): Item { - return {_multi: node}; -} diff --git a/packages/perseus/src/multi-items/multi-renderer.tsx b/packages/perseus/src/multi-items/multi-renderer.tsx deleted file mode 100644 index 9b01836fb0..0000000000 --- a/packages/perseus/src/multi-items/multi-renderer.tsx +++ /dev/null @@ -1,584 +0,0 @@ -/* eslint-disable @khanacademy/ts-no-error-suppressions */ -/* eslint-disable react/no-unsafe */ -// TODO(mdr): There are some TypeScript errors in this file, and they're tricky, so -// I'm skipping TypeScript errors for now. -/** - * Main entry point to the MultiRenderer render portion. - * - * This file exposes the `MultiRenderer` component which performs - * multi-rendering. To multi-render a question, pass in the content of the item - * to the `MultiRenderer` component as a props. Then, pass in a function which - * takes an object of renderers (in the same structure as the content), and - * return a render tree. The `MultiRenderer` component will allow you to - * combine scores, serialized state, etc. without having to manually call on - * each of the functions. It also handles inter-widgets requests between the - * different renderers. - * - * Example: - * - * item = {_multi: { - * left: , - * right: [, ], - * }} - * shape = shapes.shape({ - * left: shapes.content, - * right: shapes.arrayOf(shapes.content), - * }) - * - * - * {({renderers}) => - *
- *
{renderers.left}
- * - *
- * } - * - */ -import {Errors} from "@khanacademy/perseus-core"; -import {StyleSheet, css} from "aphrodite"; -// eslint-disable-next-line import/no-extraneous-dependencies -import lens from "hubble"; -import * as React from "react"; - -import {PerseusI18nContext} from "../components/i18n-context"; -import {DependenciesContext} from "../dependencies"; -import HintsRenderer from "../hints-renderer"; -import {Log} from "../logging/log"; -import Renderer from "../renderer"; -import {combineScores, keScoreFromPerseusScore} from "../util/scoring"; - -import {itemToTree} from "./items"; -import {buildMapper} from "./trees"; - -import type {Item, ContentNode, HintNode, TagsNode} from "./item-types"; -import type {Shape, ArrayShape} from "./shape-types"; -import type {Tree} from "./tree-types"; -import type {TreeMapper, ContentMapper, HintMapper, Path} from "./trees"; -import type { - APIOptions, - FilterCriterion, - PerseusDependenciesV2, - PerseusScore, - Widget, -} from "../types"; -import type {PropsFor} from "@khanacademy/wonder-blocks-core"; - -type Hint = any; // TODO(mdr) -type Score = any; // TODO(mdr) -type SerializedState = any; // TODO(mdr) - -type RendererProps = PropsFor; - -type ContentRendererElement = React.ReactElement; -type HintRendererElement = React.ReactElement; -type ContentRendererData = { - makeRenderer: () => ContentRendererElement; - ref: Renderer | null | undefined; -}; -type FindWidgetsFunc = ( - criterion: FilterCriterion, -) => ReadonlyArray; -type HintRendererData = { - makeRenderer: () => HintRendererElement; - findExternalWidgets: FindWidgetsFunc | null | undefined; - ref: null; - hint: Hint; -}; -type RendererData = ContentRendererData | HintRendererData; -// @ts-expect-error - TS2315 - Type 'Tree' is not generic. -type RendererDataTree = Tree; - -/** - * TODO(somewhatabstract, JIRA-XXXX): - * Some usage of this type somewhere is causing TypeScript to believe that these - * elements should be components. Don't know where that is, to fix it up - * properly. In reality, these types are just too hard to use and aren't - * really helping us out as I now have to suppress this rather than - * get value from the type. :( - */ -// @ts-expect-error - TS2315 - Type 'Tree' is not generic. -type RendererTree = Tree; -// @ts-expect-error - TS2315 - Type 'Tree' is not generic. -type ScoreTree = Tree; -// @ts-expect-error - TS2315 - Type 'Tree' is not generic. -type SerializedStateTree = Tree; - -type Props = { - item: Item; - shape: Shape; - children: (tree: {renderers: RendererTree}) => React.ReactElement; - serializedState?: SerializedStateTree | null | undefined; - onSerializedStateUpdated?: (state: SerializedStateTree) => void; - onInteractWithWidget?: (id: string) => void; - apiOptions?: APIOptions; - reviewMode?: boolean | null | undefined; - - dependencies: PerseusDependenciesV2; -}; -type State = { - // We cache functions to generate renderers and refs in `rendererDataTree`, - // and change them every time content changes. This isn't just a performance - // optimization; see `_makeContentRendererData` for more discussion. - rendererDataTree: RendererDataTree | null | undefined; - // But, if traversing the tree fails, we store the Error in `renderError`. - renderError: Error | null | undefined; -}; - -class MultiRenderer extends React.Component { - static contextType = PerseusI18nContext; - declare context: React.ContextType; - - rendererDataTreeMapper: TreeMapper< - ContentNode, - ContentRendererData, - HintNode, - HintRendererData, - TagsNode, - null - >; - getRenderersMapper: TreeMapper< - ContentRendererData, - ContentRendererElement, - HintRendererData, - HintRendererElement, - null, - null - >; - - constructor(props: Props) { - super(props); - - this.rendererDataTreeMapper = buildMapper() - // @ts-expect-error - TS2345 - Argument of type 'unknown' is not assignable to parameter of type 'ContentNode'. - .setContentMapper((c, _, p) => this._makeContentRendererData(c, p)) - // @ts-expect-error - TS2345 - Argument of type 'unknown' is not assignable to parameter of type 'HintNode'. - .setHintMapper((h) => this._makeHintRendererData(h)) - .setTagsMapper((t) => null); - - // @ts-expect-error - TS2322 - Type 'TreeMapperForLeavesAndCollections' is not assignable to type 'TreeMapper'. - this.getRenderersMapper = buildMapper() - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setContentMapper((c) => c.makeRenderer()) - // @ts-expect-error - TS2571 - Object is of type 'unknown'. - .setHintMapper((h) => h.makeRenderer()) - .setArrayMapper((renderers, data, shape) => - this._annotateRendererArray( - renderers as any, - data as any, - shape, - ), - ); - - // Keep state in sync with props. - this.state = this._tryMakeRendererState(this.props); - } - - UNSAFE_componentWillReceiveProps(nextProps: Props) { - // Keep state in sync with props. - if (nextProps.item !== this.props.item) { - this.setState(this._tryMakeRendererState(nextProps)); - } - } - - /** - * Attempt to build a State that includes a renderer tree corresponding to - * the item provided in props. On error, return a state with `renderError` - * set instead. - */ - _tryMakeRendererState(props: Props): State { - try { - return { - rendererDataTree: this._makeRendererDataTree( - props.item, - props.shape, - ), - renderError: null, - }; - } catch (e: any) { - Log.error("Error building tree state", Errors.Internal, { - cause: e, - }); - return { - rendererDataTree: null, - renderError: e, - }; - } - } - - _handleSerializedStateUpdated: (path: Path, newState?: any) => void = ( - path, - newState, - ) => { - const {onSerializedStateUpdated} = this.props; - - if (onSerializedStateUpdated) { - const oldState = this._getSerializedState( - this.props.serializedState, - ); - onSerializedStateUpdated( - lens(oldState).set(path, newState).freeze(), - ); - } - }; - - /** - * Props that aren't directly used by the MultiRenderer are delegated to - * the underlying Renderers. - */ - _getRendererProps(): RendererProps { - // `item`, `children`, and others are unused. I'm - // explicitly pulling them out of `this.props` so I don't pass them to - // ``. I'm not sure how else to do this. - const { - item: _, - children: __, - shape: ___, - serializedState: ____, - onSerializedStateUpdated: _____, - ...otherProps - } = this.props; - return { - ...otherProps, - strings: this.context.strings, - }; - } - - /** - * Construct a Renderer and a ref placeholder for the given ContentNode. - */ - _makeContentRendererData( - content: ContentNode, - path: Path, - ): ContentRendererData { - // NOTE(emily): The `findExternalWidgets` function here is computed - // inline and thus changes each time we run this function. If it - // were to change every render, it would cause the Renderer to - // re-render a lot more than is necessary. Don't re-compute this - // element unless it is necessary! - // HACK(mdr): TypeScript can't prove that this is a ContentRendererData, - // because of how we awkwardly construct it in order to obtain a - // circular reference. But it is, I promise. - const data: any = {ref: null, makeRenderer: null}; - - const refFunc = (e: any) => (data.ref = e); - const findExternalWidgets = (criterion: any) => - this._findWidgets(data, criterion); - const handleSerializedState = (state: any) => - this._handleSerializedStateUpdated(path, state); - - data.makeRenderer = () => ( - /** - * TODO(somewhatabstract, JIRA-XXXX): - * `content` contains props that Renderer doesn't have. However, - * since the type for `content` is not exact, it's hard to know - * if this spread is including undocumented props so mapping - * one to one could introduce a bug. Need to work out the exact - * type for ContentNode and then fix this. - */ // @ts-expect-error - TS2322 - Type '{ ref: (e: any) => any; findExternalWidgets: (criterion: any) => readonly (Widget | null | undefined)[]; serializedState: any; onSerializedStateUpdated: (state: any) => void; __type: "content" | "item"; ... 9 more ...; reviewMode?: boolean | ... 1 more ... | undefined; }' is not assignable to type 'InexactPartial & Readonly<{ children?: ReactNode; }>, "content" | "images" | "onRender" | "linterContext" | "widgets" | "alwaysUpdate" | ... 6 more ... | "serializedState">>'. - - ); - return data; - } - - /** - * Construct a Renderer for the given HintNode, and keep track of the hint - * itself for future use, too. - */ - _makeHintRendererData(hint: HintNode): HintRendererData { - // TODO(mdr): Once HintsRenderer supports inter-widget communication, - // give it a ref. Until then, leave the ref null forever, to avoid - // confusing the findWidgets functions. - // - // NOTE(davidflanagan): As a partial step toward inter-widget - // communication we're going to pass a findExternalWidgets function - // (using a dummy data object). This allows passage-ref widgets in - // hints to use findWidget() to find the passage widgets they reference. - // Note that this is one-way only, however. It does not allow - // widgets in the question to find widgets in the hints, for example. - const findExternalWidgets = (criterion) => - this._findWidgets({} as any, criterion); - - return { - hint, - findExternalWidgets, // _annotateRendererArray() needs this - ref: null, - makeRenderer: () => ( - - ), - }; - } - - /** - * Construct a tree of interconnected RendererDatas, corresponding to the - * given item. Called in `_tryMakeRendererState`, in order to store this - * tree in the component state. - */ - _makeRendererDataTree(item: Item, shape: Shape): RendererDataTree { - const itemTree = itemToTree(item); - return this.rendererDataTreeMapper.mapTree(itemTree, shape); - } - - /** - * Return all widgets that meet the given criterion, from all Renderers - * except the Renderer that triggered this call. - * - * This function is provided to each Renderer's `findExternalWidgets` prop, - * which enables widgets in different Renderers to discover each other and - * communicate. - */ - _findWidgets( - callingData: RendererData, - filterCriterion: FilterCriterion, - ): ReadonlyArray { - const results: Array = []; - - this._mapRenderers((data) => { - if (callingData !== data && data.ref) { - results.push(...data.ref.findInternalWidgets(filterCriterion)); - } - }); - - return results; - } - - /** - * Copy the renderer tree, apply the given transformation to the leaf nodes - * and the optional given transformation to the array nodes, and return the - * result. - * - * Used to provide structured data to the call site (the Renderer tree on - * `render`, the Score tree on `getScores`, etc.), and to traverse the - * renderer tree even when we disregard the output (like in - * `_findWidgets`). - */ - _mapRenderers( - // eslint-disable-next-line no-restricted-syntax - leafMapper: ContentMapper & - HintMapper, - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. - ): Tree | null | undefined { - const {rendererDataTree} = this.state; - - if (!rendererDataTree) { - return null; - } - - const mapper = buildMapper() - .setContentMapper(leafMapper) - .setHintMapper(leafMapper); - return mapper.mapTree(rendererDataTree, this.props.shape); - } - - _scoreFromRef(ref?: Renderer | null): Score { - if (!ref) { - return null; - } - - const [guess, score] = ref.guessAndScore(); - let state; - if (ref.getSerializedState) { - state = ref.getSerializedState(); - } - return keScoreFromPerseusScore(score, guess, state); - } - - /** - * Return a tree in the shape of the multi-item, with scores at each of - * the content nodes and `null` at the other leaf nodes. - */ - getScores(): ScoreTree { - return this._mapRenderers((data) => this._scoreFromRef(data.ref)); - } - - /** - * Return a single composite score for all rendered content nodes. - * The `guess` is a tree in the shape of the multi-item, with an individual - * guess at each content node and `null` at the other leaf nodes. - */ - score(): Score { - const scores: Array = []; - const state: Array< - | any - | { - [id: string]: any; - } - > = []; - const guess = this._mapRenderers((data) => { - if (!data.ref) { - return null; - } - - if (data.ref.getSerializedState) { - state.push(data.ref.getSerializedState()); - } - - scores.push(data.ref.score()); - return data.ref?.getUserInput(); - }); - - const combinedScore = scores.reduce(combineScores); - - return keScoreFromPerseusScore(combinedScore, guess, state); - } - - /** - * Return a tree in the shape of the multi-item, with serialized state at - * each of the content nodes and `null` at the other leaf nodes. - * - * If the lastSerializedState argument is supplied, this function will fill - * in the state of not-currently-rendered content and hint nodes with the - * values from the previous serialized state. If no lastSerializedState is - * supplied, `null` will be returned for not-currently-rendered content and - * hint nodes. - */ - _getSerializedState( - lastSerializedState?: SerializedStateTree, - ): SerializedStateTree { - return this._mapRenderers((data, _, path) => { - if (data.ref) { - return data.ref.getSerializedState(); - } - if (lastSerializedState) { - return lens(lastSerializedState).get(path); - } - return null; - }); - } - - /** - * Given a tree in the shape of the multi-item, with serialized state at - * each of the content nodes, restore each state to the corresponding - * renderer if currently mounted. - */ - restoreSerializedState( - serializedState: SerializedState, - callback?: () => any, - ) { - // We want to call our async callback only once all of the childrens' - // callbacks have run. We add one to this counter before we call out to - // each renderer and decrement it when it runs our callback. - let numCallbacks = 0; - const countCallback = () => { - numCallbacks--; - if (callback && numCallbacks === 0) { - callback(); - } - }; - - this._mapRenderers((data, _, path) => { - if (!data.ref) { - return; - } - - const state = lens(serializedState).get(path); - if (!state) { - return; - } - - numCallbacks++; - data.ref?.restoreSerializedState(state, countCallback); - }); - } - - /** - * Given an array of renderers, if it happens to be an array of *hint* - * renderers, then attach a `firstN` method to the array, which allows the - * layout to render the hints together in one HintsRenderer. - */ - _annotateRendererArray( - renderers: ReadonlyArray, - rendererDatas: ReadonlyArray, - shape: ArrayShape, - ): ReadonlyArray { - if (shape.elementShape.type === "hint") { - // The shape says that these are HintRendererDatas, even though - // it's not provable at compile time, so perform a cast. - const hintRendererDatas: ReadonlyArray = - rendererDatas as any; - - renderers = [...renderers]; - (renderers as any).firstN = (n: any) => ( - d.hint)} - hintsVisible={n} - /> - ); - } - return renderers; - } - - /** - * Return a tree in the shape of the multi-item, with a Renderer at each - * content node and a HintRenderer at each hint node. - * - * This is generated by running each of the `makeRenderer` functions at the - * leaf nodes. - */ - _getRenderers(): RendererTree { - return this.getRenderersMapper.mapTree( - this.state.rendererDataTree, - this.props.shape, - ); - } - - render(): React.ReactNode { - if (this.state.renderError) { - return ( -
- {this.context.strings.errorRendering({ - error: String(this.state.renderError), - })} -
- ); - } - - // Pass the renderer tree to the `children` function, which will - // determine the actual content of this component. - return ( - - {this.props.children({ - renderers: this._getRenderers(), - })} - - ); - } -} - -const styles = StyleSheet.create({ - error: { - color: "red", - }, -}); - -export default MultiRenderer; diff --git a/packages/perseus/src/multi-items/prop-type-builders.ts b/packages/perseus/src/multi-items/prop-type-builders.ts deleted file mode 100644 index 397eba865c..0000000000 --- a/packages/perseus/src/multi-items/prop-type-builders.ts +++ /dev/null @@ -1,74 +0,0 @@ -/** - * Utility functions to build React PropTypes for multi-items and shapes. - * - * If you're writing new components, though, consider using the Item and Shape - * types instead. - */ -/* instanbul ignore file */ - -import {Errors, PerseusError} from "@khanacademy/perseus-core"; -import PropTypes from "prop-types"; - -import type {Shape} from "./shape-types"; - -/** - * Return a PropType that accepts Items of the given shape, and rejects other - * objects. - * - * Usage: `propTypes: {item: buildPropTypeForShape(myShape)}` - */ -export function buildPropTypeForShape(shape: Shape): any { - return PropTypes.oneOfType([ - PropTypes.shape({ - _multi: buildTreePropTypeForShape(shape), - }), - PropTypes.oneOf([null, undefined]), - ]); -} - -/** - * Return a PropType that accepts ItemTrees of the given shape, and rejects - * other objects. - */ -function buildTreePropTypeForShape(shape: Shape): any { - if (shape.type === "content") { - return PropTypes.shape({ - // TODO(mdr): Remove #LegacyContentNode support. - __type: PropTypes.oneOf(["content", "item"]).isRequired, - content: PropTypes.string, - images: PropTypes.objectOf(PropTypes.any), - widgets: PropTypes.objectOf(PropTypes.any), - }); - } - if (shape.type === "hint") { - return PropTypes.shape({ - __type: PropTypes.oneOf(["hint"]).isRequired, - content: PropTypes.string, - images: PropTypes.objectOf(PropTypes.any), - widgets: PropTypes.objectOf(PropTypes.any), - replace: PropTypes.bool, - }); - } - if (shape.type === "tags") { - return PropTypes.arrayOf(PropTypes.string.isRequired); - } - if (shape.type === "array") { - const elementPropType = buildTreePropTypeForShape(shape.elementShape); - return PropTypes.arrayOf(elementPropType.isRequired); - } - if (shape.type === "object") { - const valueShapes = shape.shape; - const propTypeShape: Record = {}; - Object.keys(valueShapes).forEach((key) => { - propTypeShape[key] = buildTreePropTypeForShape( - valueShapes[key], - ).isRequired; - }); - return PropTypes.shape(propTypeShape); - } - throw new PerseusError( - // @ts-expect-error - TS2339 - Property 'type' does not exist on type 'never'. - `unexpected shape type ${shape.type}`, - Errors.InvalidInput, - ); -} diff --git a/packages/perseus/src/multi-items/shape-types.ts b/packages/perseus/src/multi-items/shape-types.ts deleted file mode 100644 index 4ffa67f5f3..0000000000 --- a/packages/perseus/src/multi-items/shape-types.ts +++ /dev/null @@ -1,52 +0,0 @@ -/** - * Type definitions for multi-item shapes. - * - * A shape is an object that serves as a runtime type declaration: it specifies - * a tree structure for a particular class of multi-item. - * - * We use shapes instead static compile-time typing because the CMS needs to - * understand the shape of our content library's multi-items at runtime, and - * it's not always possible to infer the full shape from an example multi-item. - * - * Shapes also enable us to traverse a multi-item-shaped tree with confidence, - * even when we can't infer the shape from the tree alone. - * - * We *could* go all-in on a more general library to make certain types - * runtime-inspectable, in order to DRY some things up, but that's probably a - * big ol' infrastructural magic mess, and the narrower scope of Shapes makes - * it easier to be confident that we've covered all cases rather than having to - * deal with all possible Javascript types. - */ -export type ContentShape = { - type: "content"; -}; -export type HintShape = { - type: "hint"; -}; -export type TagsShape = { - type: "tags"; -}; -export type ArrayShape = { - type: "array"; - /** - * Each element of an ArrayNode has the same shape, which is specified by - * the `elementShape` property. - */ - elementShape: Shape; -}; -export type ObjectShape = { - type: "object"; - /** - * Each property of an ObjectNode has its own shape, which is specified - * under the corresponding key in the `shape` property. - */ - shape: { - [k: string]: Shape; - }; -}; -export type Shape = - | ContentShape - | HintShape - | TagsShape - | ArrayShape - | ObjectShape; diff --git a/packages/perseus/src/multi-items/shapes.ts b/packages/perseus/src/multi-items/shapes.ts deleted file mode 100644 index 6f70a87b36..0000000000 --- a/packages/perseus/src/multi-items/shapes.ts +++ /dev/null @@ -1,52 +0,0 @@ -/** - * Utility functions for constructing and inferring multi-item shapes. - * - * A shape is an object that serves as a runtime type declaration: it specifies - * a tree structure for a particular class of multi-item. See shape-types.js - * for further discussion. - * - * This module allows you to construct arbitrary Shape trees, by combining - * leaf node shapes like `content` and `hint` into composite shapes like - * `arrayOf(shape({foo: content, bar: hint}))`. - */ -import type { - Shape, - ContentShape, - HintShape, - TagsShape, - ArrayShape, - ObjectShape, -} from "./shape-types"; - -/** - * These tools allow you to construct arbirtary shapes, by combining simple - * leaf shapes like `content` and `hint` into composite shapes like - * `arrayOf(shape({question: content, hints: arrayOf(hint)}))`. - */ -const contentShape: ContentShape = { - type: "content", -}; -const hintShape: HintShape = { - type: "hint", -}; -const tagsShape: TagsShape = { - type: "tags", -}; -const buildArrayShape = (elementShape: Shape): ArrayShape => ({ - type: "array", - elementShape, -}); -const buildObjectShape = (shape: {[k: string]: Shape}): ObjectShape => ({ - type: "object", - shape, -}); -const hintsShape: ArrayShape = buildArrayShape(hintShape); - -export default { - content: contentShape, - hint: hintShape, - hints: hintsShape, - tags: tagsShape, - arrayOf: buildArrayShape, - shape: buildObjectShape, -}; diff --git a/packages/perseus/src/multi-items/tree-types.ts b/packages/perseus/src/multi-items/tree-types.ts deleted file mode 100644 index f4df0c6fb9..0000000000 --- a/packages/perseus/src/multi-items/tree-types.ts +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Type definitions for generic trees that are shaped like multi-items. - * - * Multi-items are trees! But we also often have other trees that are shaped - * like multi-items - for example, if we map a multi-item tree into a tree of - * renderer info and state, and then map that again into a tree of just the - * renderer nodes, like we do in MultiRenderer. - * - * Therefore, we provide the type Tree, which represents a tree that's - * shaped like a multi-item, but the data that lives at each type of leaf could - * be anything. - * - * So, in a Tree, content leaf nodes have type C, and hint leaf nodes - * have type H. An ItemTree is a Tree. - * - * That is, we still preserve the distinction between node types, and thereby - * enforce the Tree's adherence to some multi-item Shape, but we're flexible - * about exactly what type of data the tree contains at its leaves. - * - * This enables us to write generic tree-traversal and tree-mapping functions, - * as you'll see in trees.js. - */ - -// TODO(jeremy): We lost "arrayness" in D69450. It'd be nice to restore that -// because right now ArrayNode doesn't add anything. -// @ts-expect-error - TS2456 - Type alias 'ArrayNode' circularly references itself. | TS2315 - Type 'Tree' is not generic. -export type ArrayNode = Tree; -export type ObjectNode = { - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. - [k: string]: Tree; -}; - -/** - * TODO(somewhatabstract, JIRA-XXXX): - * We should ditch this type or create some helpers for working with it. - * As it stands, consumers have to check each node's type before using it - * in order to ensure that TypeScript can track its refinement and appropriate use. - * It's messy. - */ -// @ts-expect-error - TS2456 - Type alias 'Tree' circularly references itself. | TS2315 - Type 'ArrayNode' is not generic. -export type Tree = - | C - | H - | T - // @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. - | ArrayNode - | ObjectNode; diff --git a/packages/perseus/src/multi-items/trees.ts b/packages/perseus/src/multi-items/trees.ts deleted file mode 100644 index 6ef124139c..0000000000 --- a/packages/perseus/src/multi-items/trees.ts +++ /dev/null @@ -1,359 +0,0 @@ -/** - * Utility functions for manipulating multi-item-shaped trees. - * - * Multi-items are trees! But we also often have other trees that are shaped - * like multi-items - for example, if we map a multi-item tree into a tree of - * renderer info and state, and then map that again into a tree of just the - * renderer nodes, like we do in MultiRenderer. See tree-types.js for further - * discussion. - * - * These functions enable us to manipulate generic multi-item-shaped trees, - * regardless of what type of data they contain at their leaves. You can use - * the mapper functions to transform a tree into another tree of the same - * shape, or to discover all the nodes of a particular type. - * - * We expose two simple mapper functions (mapContentNodes and mapHintNodes), - * and also a more complex interface for creating a mapping over all of a - * tree's node types simultaneously: - * - * `buildMapper()` returns a TreeMapper object that allows you to build your - * mapper object one node type at a time. Then, you can execute your mapping by - * calling the `mapTree` method. - * - * For example: - * const renderers = buildMapper() - * .setContentMapper(this.renderContentNode) - * .setHintMapper(this.renderHintNode) - * .setTagsMapper(this.renderTagsNode) - * .setArrayMapper(this.hideSkippedQuestions) - * .mapTree(tree, shape); - * - * This will copy the given tree, apply the given transformations to the - * content, hint, and array nodes respectively, and return the resulting tree. - * - * For node types whose mappers aren't specified, we default to the identity - * function. (This builder interface enables us to implement that default - * behavior in a provably type-safe way, while not requiring the call site to - * be aware of all the node types. Hooray!) - * - * The call to `setArrayMapper` must come last, because the array mapper's - * argument types depend on the other mappers' types. See ArrayMapper for more - * details. - * - * WARNING: These functions trust that the provided tree conforms to the - * provided shape. If not, behavior is undefined and may not respect the type - * signatures specified here. - */ - -import {Errors, PerseusError} from "@khanacademy/perseus-core"; - -import type { - Shape, - ContentShape, - HintShape, - TagsShape, - ArrayShape, -} from "./shape-types"; -import type {Tree, ArrayNode, ObjectNode} from "./tree-types"; - -/** - * The sequence of edges that lead to a particular node in a Tree. - * Elements can be `string` to correspond to an ObjectNode key, or `number` to - * correspond to an ArrayNode index. - */ -export type Path = ReadonlyArray; - -/** - * These are function interfaces for mapping over various types of tree nodes. - * - * ArrayMapper is a bit more complicated than the leaf node mappers. It's - * executed in the context of a `mapTree` call, after we've finished mapping - * its child nodes, so the function has access to both the resulting array - * (with mapped elements) and the original array (with the original untouched - * elements). - * - * The ArrayMapper then has the opportunity to apply a final transformation to - * the resulting array, like filtering certain elements or (in the hacky - * MultiRenderer case) attaching a `renderHints` method to arrays of hint - * renderers :) - * - * This is why `TreeMapper#setArrayMapper` must be called last: ArrayMapper's - * types depend on the ContentMapper and HintMapper's types. And, since you can - * only specify one mapper at a time in this builder interface (which is - * necessary to provide default mappers in a type-safe way), you need your - * dependencies to already be in place by the time you call `setArrayMapper`. - * Otherwise, we'd have to set the ArrayMapper and *hope* that you *eventually* - * provide a compatible ContentMapper and HintMapper, which is difficult to - * prove at compile time. - * - * There's no ObjectMapper here, but not for any particular reason. We just - * don't have a use case for it yet, so we haven't built it yet. - */ -export type ContentMapper = ( - content: CI, - shape: ContentShape, - path: Path, -) => CO; -export type HintMapper = (hint: HI, shape: HintShape, path: Path) => HO; -export type TagsMapper = (tag: TI, shape: TagsShape, path: Path) => TO; -export type ArrayMapper = ( - // @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. - mappedArray: ArrayNode, - // @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. - originalArray: ArrayNode, - shape: ArrayShape, - path: Path, - // @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. -) => ArrayNode; - -/** - * A TreeMapper is a collection of node mappers, which, together, compose the - * behavior for mapping over an entire tree. - * - * This serves as the interface for the two TreeMapper classes, including both - * the internal mapper properties that we care about, and the `mapTree` - * function that the call site will use. - */ -export interface TreeMapper { - content: ContentMapper; - hint: HintMapper; - tags: TagsMapper; - array: ArrayMapper; - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. | TS2315 - Type 'Tree' is not generic. - mapTree(tree: Tree, shape: Shape): Tree; -} - -/** - * This is a TreeMapper that only has mappers specified for its leaf nodes; its - * array mapper is the identity function. - * - * This is the TreeMapper initially returned by `buildMapper`. It allows you to - * change the types of your ContentMapper and HintMapper, which is safe because - * none of the other mappers that depend on those types (aka ArrayMapper) have - * been specified yet. (Or, more specifically, the ArrayMapper is currently - * `identity`, which can trivially vary with the ContentMapper and HintMapper's - * types.) - * - * Once you call `setArrayMapper`, however, we move to the other class: - * TreeMapperForLeavesAndCollections. - */ -class TreeMapperJustForLeaves { - content: ContentMapper; - hint: HintMapper; - tags: TagsMapper; - array: ArrayMapper; - - constructor( - content: ContentMapper, - hint: HintMapper, - tags: TagsMapper, - ) { - this.content = content; - this.hint = hint; - this.tags = tags; - this.array = identity; - } - - setContentMapper( - newContentMapper: ContentMapper, - ): TreeMapperJustForLeaves { - return new TreeMapperJustForLeaves( - newContentMapper, - this.hint, - this.tags, - ); - } - - setHintMapper( - newHintMapper: HintMapper, - ): TreeMapperJustForLeaves { - return new TreeMapperJustForLeaves( - this.content, - newHintMapper, - this.tags, - ); - } - - setTagsMapper( - newTagsMapper: TagsMapper, - ): TreeMapperJustForLeaves { - return new TreeMapperJustForLeaves( - this.content, - this.hint, - newTagsMapper, - ); - } - - setArrayMapper( - newArrayMapper: ArrayMapper, - ): TreeMapperForLeavesAndCollections { - return new TreeMapperForLeavesAndCollections( - this.content, - this.hint, - this.tags, - newArrayMapper, - ); - } - - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. | TS2315 - Type 'Tree' is not generic. - mapTree(tree: Tree, shape: Shape): Tree { - return mapTree(tree, shape, [], this); - } -} - -/** - * This is a TreeMapper that already has an ArrayMapper specified, so its - * ContentMapper and HintMapper are now locked in. - */ -class TreeMapperForLeavesAndCollections { - content: ContentMapper; - hint: HintMapper; - tags: TagsMapper; - array: ArrayMapper; - - constructor( - content: ContentMapper, - hint: HintMapper, - tags: TagsMapper, - array: ArrayMapper, - ) { - this.content = content; - this.hint = hint; - this.tags = tags; - this.array = array; - } - - setArrayMapper( - newArrayMapper: ArrayMapper, - ): TreeMapperForLeavesAndCollections { - return new TreeMapperForLeavesAndCollections( - this.content, - this.hint, - this.tags, - newArrayMapper, - ); - } - - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. | TS2315 - Type 'Tree' is not generic. - mapTree(tree: Tree, shape: Shape): Tree { - return mapTree(tree, shape, [], this); - } -} - -function identity(x: T): T { - return x; -} - -/** - * Return a new TreeMapper that will perform a no-op transformation on an input - * tree. To make it useful, chain any combination of `setContentMapper`, - * `setHintMapper`, `setTagMapper`, and `setArrayMapper` to specify - * transformations for the individual node types. - */ -// @ts-expect-error - TS2300 - Duplicate identifier 'C'. | TS2300 - Duplicate identifier 'H'. | TS2300 - Duplicate identifier 'T'. -export function buildMapper(): TreeMapperJustForLeaves< - C, - C, - H, - H, - T, - T -> { - return new TreeMapperJustForLeaves(identity, identity, identity); -} - -/** - * Copy the given tree, apply the corresponding transformation specified in the - * TreeMapper to each node, and return the resulting tree. - */ -function mapTree( - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. - tree: Tree, - shape: Shape, - path: Path, - mappers: TreeMapper, - // @ts-expect-error - TS2315 - Type 'Tree' is not generic. -): Tree { - // We trust the shape of the multi-item to match the shape provided at - // runtime. Therefore, in each shape branch, we cast the node to `any` and - // reinterpret it as the expected node type. - if (shape.type === "content") { - const content: CI = tree as any; - return mappers.content(content, shape, path); - } - if (shape.type === "hint") { - const hint: HI = tree as any; - return mappers.hint(hint, shape, path); - } - if (shape.type === "tags") { - const tags: TI = tree as any; - return mappers.tags(tags, shape, path); - } - if (shape.type === "array") { - // @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. - const array: ArrayNode = tree as any; - - if (!Array.isArray(array)) { - throw new PerseusError( - `Invalid object of type "${typeof array}" found at path ` + - // @ts-expect-error - TS2769 - No overload matches this call. - `${[""].concat(path).join(".")}. Expected array.`, - Errors.Internal, - ); - } - - const elementShape = shape.elementShape; - // @ts-expect-error - TS2315 - Type 'ArrayNode' is not generic. - const mappedElements: ArrayNode = array.map((inner, i) => - mapTree(inner, elementShape, path.concat(i), mappers), - ); - return mappers.array(mappedElements, array, shape, path); - } - if (shape.type === "object") { - const object: ObjectNode = tree as any; - - if (object && typeof object !== "object") { - throw new PerseusError( - `Invalid object of type "${typeof object}" found at ` + - // @ts-expect-error - TS2769 - No overload matches this call. - `path ${[""].concat(path).join(".")}. Expected ` + - `"object" type.`, - Errors.InvalidInput, - ); - } - - const valueShapes = shape.shape; - if (!valueShapes) { - throw new PerseusError( - `Unexpected shape ${JSON.stringify(shape)} at path ` + - // @ts-expect-error - TS2769 - No overload matches this call. - `${[""].concat(path).join(".")}.`, - Errors.InvalidInput, - ); - } - const newObject: Record = {}; - Object.keys(valueShapes).forEach((key) => { - if (!(key in object)) { - throw new PerseusError( - `Key "${key}" is missing from shape at path ` + - // @ts-expect-error - TS2769 - No overload matches this call. - `${[""].concat(path).join(".")}.`, - Errors.InvalidInput, - ); - } - - newObject[key] = mapTree( - object[key], - valueShapes[key], - path.concat(key), - mappers, - ); - }); - return newObject; - } - throw new PerseusError( - // @ts-expect-error - TS2339 - Property 'type' does not exist on type 'never'. - `unexpected shape type ${shape.type}`, - Errors.InvalidInput, - ); -} diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index d6b73bc3c3..8f922ea33f 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -140,19 +140,6 @@ export type PerseusItem = { */ export type PerseusArticle = PerseusRenderer | ReadonlyArray; -/** - * 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. - * - * @deprecated MultiItem support is slated for removal in a future Perseus - * release. - */ -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 Version = { // The major part of the version major: number; diff --git a/packages/perseus/src/renderability.ts b/packages/perseus/src/renderability.ts index 2cda9a0b22..910eaeba22 100644 --- a/packages/perseus/src/renderability.ts +++ b/packages/perseus/src/renderability.ts @@ -11,14 +11,11 @@ import {Errors, PerseusError} from "@khanacademy/perseus-core"; import _ from "underscore"; -import MultiItems from "./multi-items"; import {traverse} from "./traversal"; import * as Widgets from "./widgets"; import type {PerseusWidget} from "./perseus-types"; -const {findContentNodesInItem, inferItemShape} = MultiItems; - const isUpgradedWidgetInfoRenderableBy = function ( widgetInfo: PerseusWidget, widgetRendererVersion: any, @@ -86,21 +83,6 @@ export const isItemRenderableByVersion = function ( Errors.InvalidInput, ); } - if (itemData._multi) { - const shape = inferItemShape(itemData); - - let isRenderable = true; - findContentNodesInItem(itemData, shape, (node) => { - const nodeIsRenderable = isRendererContentRenderableBy( - node, - rendererContentVersion, - ); - if (!nodeIsRenderable) { - isRenderable = false; - } - }); - return isRenderable; - } return isRendererContentRenderableBy( itemData.question, rendererContentVersion, diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index 7ba9d2d385..e686ea0a00 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -1,5 +1,4 @@ import type {ILogger} from "./logging/log"; -import type {Item} from "./multi-items/item-types"; import type { Hint, PerseusAnswerArea, @@ -138,8 +137,6 @@ export type ChangeHandler = ( question?: any; answerArea?: PerseusAnswerArea | null; itemDataVersion?: Version; - // used in MutirenderEditor - item?: Item; editorMode?: EditorMode; jsonMode?: boolean; // perseus-all-package/widgets/unit.jsx diff --git a/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap b/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap index ec368c0e50..55689676e2 100644 --- a/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap +++ b/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap @@ -1608,34 +1608,6 @@ exports[`parseAndTypecheckPerseusItem correctly parses data/interactive-graph-wi } `; -exports[`parseAndTypecheckPerseusItem correctly parses data/item-missing-answerArea.json 1`] = ` -{ - "_multi": { - "blurb": { - "__type": "content", - "content": "", - "images": {}, - "widgets": {}, - }, - "hints": [], - "question": { - "__type": "content", - "content": "", - "widgets": {}, - }, - }, - "answer": undefined, - "answerArea": {}, - "hints": [], - "itemDataVersion": undefined, - "question": { - "content": "", - "images": {}, - "widgets": {}, - }, -} -`; - exports[`parseAndTypecheckPerseusItem correctly parses data/matrix-missing-version.json 1`] = ` { "answer": undefined, diff --git a/packages/perseus/src/util/parse-perseus-json/regression-tests/data/item-missing-answerArea.json b/packages/perseus/src/util/parse-perseus-json/regression-tests/data/item-missing-answerArea.json deleted file mode 100644 index 41e1dbc06a..0000000000 --- a/packages/perseus/src/util/parse-perseus-json/regression-tests/data/item-missing-answerArea.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "_multi": { - "blurb": { - "__type": "content", - "content": "", - "images": {}, - "widgets": {} - }, - "hints": [], - "question": { - "__type": "content", - "content": "", - "widgets": {} - } - } -} diff --git a/packages/perseus/src/widget-type-utils.ts b/packages/perseus/src/widget-type-utils.ts index 3cc315ab7f..6bac814b6d 100644 --- a/packages/perseus/src/widget-type-utils.ts +++ b/packages/perseus/src/widget-type-utils.ts @@ -116,8 +116,6 @@ export function getWidgetFromWidgetMap( /** * Get select widgets from a widget map. - * Useful for multi-items that needs to split - * one PerseusItem into several * * @param {ReadonlyArray} widgetIds to extract * @param {PerseusWidgetsMap} widgetMap to extract from diff --git a/testing/multi-item-renderer-with-debug-ui.tsx b/testing/multi-item-renderer-with-debug-ui.tsx deleted file mode 100644 index 08ec0e9e42..0000000000 --- a/testing/multi-item-renderer-with-debug-ui.tsx +++ /dev/null @@ -1,76 +0,0 @@ -import Button from "@khanacademy/wonder-blocks-button"; -import {View} from "@khanacademy/wonder-blocks-core"; -import * as React from "react"; - -import {MultiItems} from "../packages/perseus/src/index"; -import {simpleQuestionShape} from "../packages/perseus/src/multi-items/__testdata__/multi-renderer.testdata"; - -import KEScoreUI from "./ke-score-ui"; -import SideBySide from "./side-by-side"; -import {storybookDependenciesV2} from "./test-dependencies"; - -import type {Item as MultiItem} from "../packages/perseus/src/multi-items/item-types"; -import type {APIOptions} from "../packages/perseus/src/types"; -import type {KEScore} from "@khanacademy/perseus-core"; - -type SimpleItemRenderTree = { - blurb: React.ReactNode; - question: React.ReactNode; - hints: ReadonlyArray; -}; - -type Props = { - simpleItem: MultiItem; - children: (tree: { - renderers: SimpleItemRenderTree; - }) => React.ReactElement; - apiOptions?: APIOptions; -}; - -// Renders an assessment item (aka {_multi: ...} that conforms to the -// sample data simpleQuestionShape. -export const MultiItemRendererWithDebugUI = ({ - children, - simpleItem, - apiOptions, -}: Props): React.ReactElement => { - // @ts-expect-error - TS2530 - Cannot find namespace 'MultiItems'. - const ref = React.useRef(null); - const [state, setState] = React.useState(null); - - return ( - - - {(renderers) => { - return children(renderers); - }} - -
-
- - - - - - } - jsonObject={simpleItem} - /> - ); -}; From fcf5a7fe53dcc5177acbdaad0a0eb603385aacba Mon Sep 17 00:00:00 2001 From: Khan Actions Bot <56267880+khan-actions-bot@users.noreply.github.com> Date: Wed, 11 Dec 2024 07:58:45 -0500 Subject: [PATCH 5/7] Version Packages (#1980) Co-authored-by: github-actions[bot] --- .changeset/cool-taxis-clap.md | 6 ------ packages/perseus-editor/CHANGELOG.md | 11 +++++++++++ packages/perseus-editor/package.json | 4 ++-- packages/perseus/CHANGELOG.md | 6 ++++++ packages/perseus/package.json | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) delete mode 100644 .changeset/cool-taxis-clap.md diff --git a/.changeset/cool-taxis-clap.md b/.changeset/cool-taxis-clap.md deleted file mode 100644 index 5cf5a872e1..0000000000 --- a/.changeset/cool-taxis-clap.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": major -"@khanacademy/perseus-editor": major ---- - -Remove support for MultiRenderer diff --git a/packages/perseus-editor/CHANGELOG.md b/packages/perseus-editor/CHANGELOG.md index d87e89c266..bd18e648bb 100644 --- a/packages/perseus-editor/CHANGELOG.md +++ b/packages/perseus-editor/CHANGELOG.md @@ -1,5 +1,16 @@ # @khanacademy/perseus-editor +## 16.0.0 + +### Major Changes + +- [#1955](https://github.com/Khan/perseus/pull/1955) [`e7b4db0bf`](https://github.com/Khan/perseus/commit/e7b4db0bf193241a36508804dd6e58c729f0a3db) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove support for MultiRenderer + +### Patch Changes + +- Updated dependencies [[`e7b4db0bf`](https://github.com/Khan/perseus/commit/e7b4db0bf193241a36508804dd6e58c729f0a3db)]: + - @khanacademy/perseus@47.0.0 + ## 15.1.4 ### Patch Changes diff --git a/packages/perseus-editor/package.json b/packages/perseus-editor/package.json index bd710e8794..349b38cd08 100644 --- a/packages/perseus-editor/package.json +++ b/packages/perseus-editor/package.json @@ -3,7 +3,7 @@ "description": "Perseus editors", "author": "Khan Academy", "license": "MIT", - "version": "15.1.4", + "version": "16.0.0", "publishConfig": { "access": "public" }, @@ -38,7 +38,7 @@ "@khanacademy/keypad-context": "^1.0.4", "@khanacademy/kmath": "^0.1.16", "@khanacademy/math-input": "^21.1.6", - "@khanacademy/perseus": "^46.0.1", + "@khanacademy/perseus": "^47.0.0", "@khanacademy/perseus-core": "1.5.3", "@khanacademy/pure-markdown": "^0.3.13", "mafs": "^0.19.0" diff --git a/packages/perseus/CHANGELOG.md b/packages/perseus/CHANGELOG.md index 00b7002dc4..6d27ebfa82 100644 --- a/packages/perseus/CHANGELOG.md +++ b/packages/perseus/CHANGELOG.md @@ -1,5 +1,11 @@ # @khanacademy/perseus +## 47.0.0 + +### Major Changes + +- [#1955](https://github.com/Khan/perseus/pull/1955) [`e7b4db0bf`](https://github.com/Khan/perseus/commit/e7b4db0bf193241a36508804dd6e58c729f0a3db) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove support for MultiRenderer + ## 46.0.1 ### Patch Changes diff --git a/packages/perseus/package.json b/packages/perseus/package.json index 70a785efe4..c898f2ecb0 100644 --- a/packages/perseus/package.json +++ b/packages/perseus/package.json @@ -3,7 +3,7 @@ "description": "Core Perseus API (includes renderers and widgets)", "author": "Khan Academy", "license": "MIT", - "version": "46.0.1", + "version": "47.0.0", "publishConfig": { "access": "public" }, From acd8bd56664c6a0849bf3d532be8978115a97dfd Mon Sep 17 00:00:00 2001 From: daniellewhyte <30729058+daniellewhyte@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:59:03 -0600 Subject: [PATCH 6/7] [Dropdown] [BUGFIX] Update aria-label logic (#1953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: This PR contains two fixes for the aria-label in the dropdown widget: 1. Change the logic of the aria label Follow up from Mark's comment on #1845 This commit ensures that the visible label does not override the aria label. Motivation: There may be additional info for the screen reader in the aria label that may not be in the visible label. This changes also removes the aria-labelledby from the Dropdown which was causing the screen reader output to be garbled in Safari. 2. Pass in a space `(" ")` for empty placeholder. Wonder Blocks adds a nbsp when an empty string is passed in as one of the option items. This does not work well with screen readers. When testing with Voice Over on Safari and Firefox, it reads out the space and the aria label as a series of characters (instead of words). https://github.com/user-attachments/assets/6ed58cfe-e849-4c96-a661-7d8c25f25617 The root cause for this should be fixed in Wonder Blocks and I’ve started a discussion with the WB team [here](https://khanacademy.slack.com/archives/C4PE1QM5Y/p1733761447231789) (I haven't been able to find a solution in WB yet). Given that this bug exists on prod in webapp, it feels appropriate to go ahead and implement this fix in Perseus. Issue: LIT-1476, LIT-1482 ## Test plan: Test with the new [Dropdown with empty placeholder story](http://localhost:6007/?path=/story/perseus-widgets-dropdown--dropdown-with-empty-placeholder) - Dropdown displays inline when nothing is selected - Screen reader does not say "space" when no option is selected. And does not spell out aria-label https://github.com/user-attachments/assets/b4ee0524-50e7-4000-a66e-2bb0590d5c3f Author: daniellewhyte Reviewers: mark-fitzgerald, catandthemachines Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ .github/dependabot.yml Pull Request URL: https://github.com/Khan/perseus/pull/1953 --- .changeset/selfish-laws-knock.md | 5 +++ .../__snapshots__/renderer.test.tsx.snap | 9 ++---- .../__snapshots__/dropdown.test.ts.snap | 2 -- .../src/widgets/dropdown/dropdown.stories.tsx | 7 ++++ .../src/widgets/dropdown/dropdown.testdata.ts | 32 +++++++++++++++++++ .../perseus/src/widgets/dropdown/dropdown.tsx | 6 ++-- 6 files changed, 50 insertions(+), 11 deletions(-) create mode 100644 .changeset/selfish-laws-knock.md diff --git a/.changeset/selfish-laws-knock.md b/.changeset/selfish-laws-knock.md new file mode 100644 index 0000000000..3210f747ec --- /dev/null +++ b/.changeset/selfish-laws-knock.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +[Dropdown] Change logic for aria-label diff --git a/packages/perseus/src/__tests__/__snapshots__/renderer.test.tsx.snap b/packages/perseus/src/__tests__/__snapshots__/renderer.test.tsx.snap index 160e3f6b89..73f63e6741 100644 --- a/packages/perseus/src/__tests__/__snapshots__/renderer.test.tsx.snap +++ b/packages/perseus/src/__tests__/__snapshots__/renderer.test.tsx.snap @@ -350,7 +350,7 @@ exports[`renderer snapshots correct answer: correct answer 1`] = ` > @@ -370,7 +370,6 @@ exports[`renderer snapshots correct answer: correct answer 1`] = ` aria-expanded="false" aria-haspopup="listbox" aria-label="Test ARIA label" - aria-labelledby="uid-dropdown-widget-2-dropdown-label" class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y" id="uid-dropdown-widget-2-dropdown" role="combobox" @@ -429,7 +428,7 @@ exports[`renderer snapshots incorrect answer: incorrect answer 1`] = ` > @@ -449,7 +448,6 @@ exports[`renderer snapshots incorrect answer: incorrect answer 1`] = ` aria-expanded="false" aria-haspopup="listbox" aria-label="Test ARIA label" - aria-labelledby="uid-dropdown-widget-4-dropdown-label" class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y" id="uid-dropdown-widget-4-dropdown" role="combobox" @@ -508,7 +506,7 @@ exports[`renderer snapshots initial render: initial render 1`] = ` > @@ -528,7 +526,6 @@ exports[`renderer snapshots initial render: initial render 1`] = ` aria-expanded="false" aria-haspopup="listbox" aria-label="Test ARIA label" - aria-labelledby="uid-dropdown-widget-0-dropdown-label" class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y" id="uid-dropdown-widget-0-dropdown" role="combobox" diff --git a/packages/perseus/src/widgets/dropdown/__snapshots__/dropdown.test.ts.snap b/packages/perseus/src/widgets/dropdown/__snapshots__/dropdown.test.ts.snap index 915a4ac053..3b1723f09f 100644 --- a/packages/perseus/src/widgets/dropdown/__snapshots__/dropdown.test.ts.snap +++ b/packages/perseus/src/widgets/dropdown/__snapshots__/dropdown.test.ts.snap @@ -37,7 +37,6 @@ exports[`Dropdown widget should snapshot when opened: dropdown open 1`] = ` aria-expanded="true" aria-haspopup="listbox" aria-label="Select an answer" - aria-labelledby="uid-dropdown-widget-2-dropdown-label" class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y" id="uid-dropdown-widget-2-dropdown" role="combobox" @@ -110,7 +109,6 @@ exports[`Dropdown widget should snapshot: initial render 1`] = ` aria-expanded="false" aria-haspopup="listbox" aria-label="Select an answer" - aria-labelledby="uid-dropdown-widget-0-dropdown-label" class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y" id="uid-dropdown-widget-0-dropdown" role="combobox" diff --git a/packages/perseus/src/widgets/dropdown/dropdown.stories.tsx b/packages/perseus/src/widgets/dropdown/dropdown.stories.tsx index 6c994dad24..a2ff15c7c6 100644 --- a/packages/perseus/src/widgets/dropdown/dropdown.stories.tsx +++ b/packages/perseus/src/widgets/dropdown/dropdown.stories.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import {RendererWithDebugUI} from "../../../../../testing/renderer-with-debug-ui"; import { + dropdownWithEmptyPlaceholder, dropdownWithVisibleLabel, inlineDropdownWithVisibleLabel, question1, @@ -29,3 +30,9 @@ export const InlineDropdownWithVisibleLabel = ( ): React.ReactElement => { return ; }; + +export const DropdownWithEmptyPlaceholder = ( + args: StoryArgs, +): React.ReactElement => { + return ; +}; diff --git a/packages/perseus/src/widgets/dropdown/dropdown.testdata.ts b/packages/perseus/src/widgets/dropdown/dropdown.testdata.ts index 9f0ef14f25..b98d5cdbc4 100644 --- a/packages/perseus/src/widgets/dropdown/dropdown.testdata.ts +++ b/packages/perseus/src/widgets/dropdown/dropdown.testdata.ts @@ -150,3 +150,35 @@ export const inlineDropdownWithVisibleLabel: PerseusRenderer = { }, }, }; + +export const dropdownWithEmptyPlaceholder: PerseusRenderer = { + content: + "The total number of boxes the forklift can carry is [[☃ dropdown 1]] $60$.", + images: {}, + widgets: { + "dropdown 1": { + type: "dropdown", + alignment: "default", + static: false, + graded: true, + options: { + static: false, + placeholder: "", + choices: [ + { + content: "greater than or equal to", + correct: false, + }, + { + content: "less than or equal to", + correct: true, + }, + ], + }, + version: { + major: 0, + minor: 0, + }, + }, + }, +}; diff --git a/packages/perseus/src/widgets/dropdown/dropdown.tsx b/packages/perseus/src/widgets/dropdown/dropdown.tsx index 2fdaa3dae4..487ff0a364 100644 --- a/packages/perseus/src/widgets/dropdown/dropdown.tsx +++ b/packages/perseus/src/widgets/dropdown/dropdown.tsx @@ -74,7 +74,7 @@ class Dropdown extends React.Component implements Widget { key="placeholder" value="0" disabled - label={this.props.placeholder} + label={this.props.placeholder || " "} />, ...this.props.choices.map((choice, i) => ( implements Widget { {this.props.visibleLabel && ( {this.props.visibleLabel} @@ -116,9 +116,9 @@ class Dropdown extends React.Component implements Widget { disabled={this.props.apiOptions.readOnly} aria-label={ this.props.ariaLabel || + this.props.visibleLabel || this.context.strings.selectAnAnswer } - aria-labelledby={ids.get("dropdown-label")} // This is currently necessary for SRs to read the labels properly. // However, WB is working on a change to add the "combobox" role to // all dropdowns. From 335615bab18685aa6331c7628c1225bdecc54aab Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Wed, 11 Dec 2024 11:40:19 -0800 Subject: [PATCH 7/7] Document perseus-types.ts file (#1975) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: The `perseus-types.ts` file is special in the Perseus project. It represents the data that is saved out of editors and consumed by the Perseus rendering system. As a result, we need to be extremely careful when making changes to it. I've added a JSDoc comment to the top of the file that (hopefully) explains the file and some cautions to take into account when making changes. Issue: LEMS-2561 ## Test plan: Read the comments and let me know if they make sense and are clear. I'm happy to make further edits. :) Author: jeremywiebe Reviewers: jeremywiebe, nishasy, handeyeco, benchristel, SonicScrewdriver, mark-fitzgerald, Myranae, catandthemachines, anakaren-rojas Required Reviewers: Approved By: handeyeco Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/1975 --- .changeset/clever-kangaroos-mate.md | 5 ++++ packages/perseus/src/perseus-types.ts | 42 ++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 .changeset/clever-kangaroos-mate.md diff --git a/.changeset/clever-kangaroos-mate.md b/.changeset/clever-kangaroos-mate.md new file mode 100644 index 0000000000..1eb14b063c --- /dev/null +++ b/.changeset/clever-kangaroos-mate.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Make all types in `perseus-types.ts` originate from it (no longer import Mafs types) diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 8f922ea33f..630ef58abc 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -1,14 +1,40 @@ -// TODO(FEI-4010): Remove `Perseus` prefix for all types here -// TODO(FEI-4011): Use types generated by https://github.com/jaredly/generate-perseus-flowtypes +/** + * The Perseus "data schema" file. + * + * This file, and the types in it, represents the "data schema" that Perseus + * uses. The @khanacademy/perseus-editor package edits and produces objects + * that conform to the types in this file. Similarly, the top-level renderers + * in @khanacademy/perseus, consume objects that conform to these types. + * + * WARNING: This file should not import any types from elsewhere so that it is + * easy to reason about changes that alter the Perseus schema. This helps + * ensure that it is not changed accidentally when upgrading a dependant + * package or other part of Perseus code. Note that TypeScript does type + * checking via something called "structural typing". This means that as long + * as the shape of a type matches, the name it goes by doesn't matter. As a + * result, a `Coord` type that looks like this `[x: number, y: number]` is + * _identical_, in TypeScript's eyes, to this `Vector2` type `[x: number, y: + * number]`. Also, with tuples, the labels for each entry is ignored, so `[x: + * number, y: number]` is compatible with `[min: number, max: number]`. The + * labels are for humans, not TypeScript. :) + * + * If you make changes to types in this file, be very sure that: + * + * a) the changes are backwards compatible. If they are not, old data from + * previous versions of the "schema" could become unrenderable, or worse, + * introduce hard-to-diagnose bugs. + * b) the parsing code (`util/parse-perseus-json/`) is updated to handle + * the new format _as well as_ the old format. + */ -import type {Coord} from "./interactive2/types"; -import type {Interval, vec} from "mafs"; +// TODO(FEI-4010): Remove `Perseus` prefix for all types here -// Range is replaced within this file with Interval, but it is used elsewhere -// and exported from the package, so we need to keep it around. +export type Coord = [x: number, y: number]; +export type Interval = [min: number, max: number]; +export type Vector2 = Coord; // Same name as Mafs export type Range = Interval; -export type Size = [number, number]; -export type CollinearTuple = [vec.Vector2, vec.Vector2]; +export type Size = [width: number, height: number]; +export type CollinearTuple = [Vector2, Vector2]; export type ShowSolutions = "all" | "selected" | "none"; /**