From 56f33ae010390abd2050028db98c9a72fc604e1a Mon Sep 17 00:00:00 2001 From: Matthew Date: Thu, 26 Sep 2024 11:21:44 -0500 Subject: [PATCH 01/14] Add widgetIsOpen to the prop denylist (#1679) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: https://khanacademy.slack.com/archives/C3SLM30AW/p1727364716435589?thread_ts=1727349077.951369&cid=C3SLM30AW ## Test plan: I can't figure out how to test this, so I'm going to either try to spin up a ZND or YOLO it. Author: handeyeco Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, 🚫 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), 🚫 Cypress (ubuntu-latest, 20.x), 🚫 Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald, 🚫 Publish npm snapshot (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), 🚫 Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1679 --- .changeset/yellow-students-occur.md | 5 +++++ packages/perseus/src/mixins/widget-prop-denylist.ts | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changeset/yellow-students-occur.md diff --git a/.changeset/yellow-students-occur.md b/.changeset/yellow-students-occur.md new file mode 100644 index 0000000000..450d465fc0 --- /dev/null +++ b/.changeset/yellow-students-occur.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Don't serialize widgetIsOpen diff --git a/packages/perseus/src/mixins/widget-prop-denylist.ts b/packages/perseus/src/mixins/widget-prop-denylist.ts index 70893c2c82..8d8d63500a 100644 --- a/packages/perseus/src/mixins/widget-prop-denylist.ts +++ b/packages/perseus/src/mixins/widget-prop-denylist.ts @@ -21,6 +21,7 @@ const denylist = [ "onChange", "problemNum", "apiOptions", + "widgetIsOpen", "questionCompleted", "findWidgets", // added by src/editor.jsx, for widgets removing themselves From 213327df6f539fb5de22981310e8184aea78000e Mon Sep 17 00:00:00 2001 From: Khan Actions Bot <56267880+khan-actions-bot@users.noreply.github.com> Date: Thu, 26 Sep 2024 12:33:07 -0400 Subject: [PATCH 02/14] Version Packages (#1660) Co-authored-by: github-actions[bot] --- .changeset/angry-penguins-itch.md | 5 ---- .changeset/breezy-rockets-greet.md | 5 ---- .changeset/dry-parents-invite.md | 6 ---- .changeset/dull-kids-pretend.md | 5 ---- .changeset/early-mice-flash.md | 5 ---- .changeset/early-rivers-tell.md | 6 ---- .changeset/loud-parrots-run.md | 5 ---- .changeset/many-apricots-repair.md | 5 ---- .changeset/modern-sheep-bathe.md | 6 ---- .changeset/nasty-coats-sin.md | 2 -- .changeset/popular-drinks-sniff.md | 5 ---- .changeset/rude-lamps-warn.md | 6 ---- .changeset/spotty-days-burn.md | 6 ---- .changeset/strange-houses-breathe.md | 6 ---- .changeset/ten-seas-chew.md | 5 ---- .changeset/tough-snails-double.md | 6 ---- .changeset/violet-icons-breathe.md | 5 ---- .changeset/yellow-poets-poke.md | 6 ---- .changeset/yellow-students-occur.md | 5 ---- packages/perseus-editor/CHANGELOG.md | 27 ++++++++++++++++++ packages/perseus-editor/package.json | 4 +-- packages/perseus/CHANGELOG.md | 42 ++++++++++++++++++++++++++++ packages/perseus/package.json | 2 +- 23 files changed, 72 insertions(+), 103 deletions(-) delete mode 100644 .changeset/angry-penguins-itch.md delete mode 100644 .changeset/breezy-rockets-greet.md delete mode 100644 .changeset/dry-parents-invite.md delete mode 100644 .changeset/dull-kids-pretend.md delete mode 100644 .changeset/early-mice-flash.md delete mode 100644 .changeset/early-rivers-tell.md delete mode 100644 .changeset/loud-parrots-run.md delete mode 100644 .changeset/many-apricots-repair.md delete mode 100644 .changeset/modern-sheep-bathe.md delete mode 100644 .changeset/nasty-coats-sin.md delete mode 100644 .changeset/popular-drinks-sniff.md delete mode 100644 .changeset/rude-lamps-warn.md delete mode 100644 .changeset/spotty-days-burn.md delete mode 100644 .changeset/strange-houses-breathe.md delete mode 100644 .changeset/ten-seas-chew.md delete mode 100644 .changeset/tough-snails-double.md delete mode 100644 .changeset/violet-icons-breathe.md delete mode 100644 .changeset/yellow-poets-poke.md delete mode 100644 .changeset/yellow-students-occur.md diff --git a/.changeset/angry-penguins-itch.md b/.changeset/angry-penguins-itch.md deleted file mode 100644 index 3b9f2af702..0000000000 --- a/.changeset/angry-penguins-itch.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": major ---- - -Remove scoreWidgets from ServerItemRenderer diff --git a/.changeset/breezy-rockets-greet.md b/.changeset/breezy-rockets-greet.md deleted file mode 100644 index 9035aeb03e..0000000000 --- a/.changeset/breezy-rockets-greet.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -Updating protractor's default position within a graph. diff --git a/.changeset/dry-parents-invite.md b/.changeset/dry-parents-invite.md deleted file mode 100644 index 4d8d8f87a2..0000000000 --- a/.changeset/dry-parents-invite.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": minor -"@khanacademy/perseus-editor": minor ---- - -[Locked Figure Labels] Add/edit/delete locked ellipse labels diff --git a/.changeset/dull-kids-pretend.md b/.changeset/dull-kids-pretend.md deleted file mode 100644 index 3cc5052d5b..0000000000 --- a/.changeset/dull-kids-pretend.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": major ---- - -Refactor virtally all widget types and consolidate user input diff --git a/.changeset/early-mice-flash.md b/.changeset/early-mice-flash.md deleted file mode 100644 index 552dc3b37f..0000000000 --- a/.changeset/early-mice-flash.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -Split out validation logic in Radio diff --git a/.changeset/early-rivers-tell.md b/.changeset/early-rivers-tell.md deleted file mode 100644 index d7be8e50b0..0000000000 --- a/.changeset/early-rivers-tell.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": minor -"@khanacademy/perseus-editor": minor ---- - -[Locked Figure Labels] View locked ellipse labels diff --git a/.changeset/loud-parrots-run.md b/.changeset/loud-parrots-run.md deleted file mode 100644 index 72014a0c3f..0000000000 --- a/.changeset/loud-parrots-run.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -Move validation logic out of the sorter widget and add tests diff --git a/.changeset/many-apricots-repair.md b/.changeset/many-apricots-repair.md deleted file mode 100644 index 89cb0f95dc..0000000000 --- a/.changeset/many-apricots-repair.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -Translations and polish for unlimited point diff --git a/.changeset/modern-sheep-bathe.md b/.changeset/modern-sheep-bathe.md deleted file mode 100644 index 5f8f73a020..0000000000 --- a/.changeset/modern-sheep-bathe.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": minor -"@khanacademy/perseus-editor": minor ---- - -[Locked Figure Labels] Add/edit/delete locked vector labels diff --git a/.changeset/nasty-coats-sin.md b/.changeset/nasty-coats-sin.md deleted file mode 100644 index a845151cc8..0000000000 --- a/.changeset/nasty-coats-sin.md +++ /dev/null @@ -1,2 +0,0 @@ ---- ---- diff --git a/.changeset/popular-drinks-sniff.md b/.changeset/popular-drinks-sniff.md deleted file mode 100644 index a08b7a702b..0000000000 --- a/.changeset/popular-drinks-sniff.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -Split validation logic from LabelImage diff --git a/.changeset/rude-lamps-warn.md b/.changeset/rude-lamps-warn.md deleted file mode 100644 index a9a64b5636..0000000000 --- a/.changeset/rude-lamps-warn.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": minor -"@khanacademy/perseus-editor": minor ---- - -[Locked Figure Labels] Add/edit/delete locked function labels diff --git a/.changeset/spotty-days-burn.md b/.changeset/spotty-days-burn.md deleted file mode 100644 index 6bb4e910e4..0000000000 --- a/.changeset/spotty-days-burn.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": minor -"@khanacademy/perseus-editor": minor ---- - -[Locked Figure Labels] View locked function labels diff --git a/.changeset/strange-houses-breathe.md b/.changeset/strange-houses-breathe.md deleted file mode 100644 index 139c7d48c1..0000000000 --- a/.changeset/strange-houses-breathe.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": minor -"@khanacademy/perseus-editor": minor ---- - -[Locked Figure Labels] View locked vector labels diff --git a/.changeset/ten-seas-chew.md b/.changeset/ten-seas-chew.md deleted file mode 100644 index 2c77ec7d99..0000000000 --- a/.changeset/ten-seas-chew.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": patch ---- - -Internal: revert buggy change to interactive graphs (never shipped) diff --git a/.changeset/tough-snails-double.md b/.changeset/tough-snails-double.md deleted file mode 100644 index 5e6fa48952..0000000000 --- a/.changeset/tough-snails-double.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": major -"@khanacademy/perseus-editor": patch ---- - -Remove unused onInputError from APIOptions diff --git a/.changeset/violet-icons-breathe.md b/.changeset/violet-icons-breathe.md deleted file mode 100644 index 52b6b60301..0000000000 --- a/.changeset/violet-icons-breathe.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus-editor": patch ---- - -[Interactive Graph Editor] Save empty full graph aria label/description as undefined diff --git a/.changeset/yellow-poets-poke.md b/.changeset/yellow-poets-poke.md deleted file mode 100644 index f846e76ef7..0000000000 --- a/.changeset/yellow-poets-poke.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@khanacademy/perseus": patch -"@khanacademy/perseus-editor": patch ---- - -[Locked Figures Labels] Make labels optional to increase type safety diff --git a/.changeset/yellow-students-occur.md b/.changeset/yellow-students-occur.md deleted file mode 100644 index 450d465fc0..0000000000 --- a/.changeset/yellow-students-occur.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus": minor ---- - -Don't serialize widgetIsOpen diff --git a/packages/perseus-editor/CHANGELOG.md b/packages/perseus-editor/CHANGELOG.md index f94474ab44..471131b8c0 100644 --- a/packages/perseus-editor/CHANGELOG.md +++ b/packages/perseus-editor/CHANGELOG.md @@ -1,5 +1,32 @@ # @khanacademy/perseus-editor +## 14.5.0 + +### Minor Changes + +- [#1655](https://github.com/Khan/perseus/pull/1655) [`790e189a7`](https://github.com/Khan/perseus/commit/790e189a7fdcd215d78d1999879ab2fc7417e123) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked ellipse labels + +* [#1653](https://github.com/Khan/perseus/pull/1653) [`ca4be05ab`](https://github.com/Khan/perseus/commit/ca4be05ab7367007330784796ad2561e3f5bb1c8) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked ellipse labels + +- [#1652](https://github.com/Khan/perseus/pull/1652) [`1ed045583`](https://github.com/Khan/perseus/commit/1ed045583fec01be5baf5d4e86a8b582cbf782c2) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked vector labels + +* [#1659](https://github.com/Khan/perseus/pull/1659) [`3dcb1fdf2`](https://github.com/Khan/perseus/commit/3dcb1fdf247eda0f0b78966daf04a9e4278d4373) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked function labels + +- [#1658](https://github.com/Khan/perseus/pull/1658) [`20b3a2485`](https://github.com/Khan/perseus/commit/20b3a2485e2ba8deea798acc2732d9570c0dac45) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked function labels + +* [#1650](https://github.com/Khan/perseus/pull/1650) [`03cddb6c3`](https://github.com/Khan/perseus/commit/03cddb6c39570e87ff2437273eb1287ff1417eec) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked vector labels + +### Patch Changes + +- [#1661](https://github.com/Khan/perseus/pull/1661) [`391641acb`](https://github.com/Khan/perseus/commit/391641acb153d2d6c0f8c29f5026a392ac1b3a62) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove unused onInputError from APIOptions + +* [#1674](https://github.com/Khan/perseus/pull/1674) [`f38d104d5`](https://github.com/Khan/perseus/commit/f38d104d580775cd67a0586143eacf7b864e4814) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Save empty full graph aria label/description as undefined + +- [#1673](https://github.com/Khan/perseus/pull/1673) [`6f4702e41`](https://github.com/Khan/perseus/commit/6f4702e418ffdfaae01aa3f3a126b304b3250e34) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figures Labels] Make labels optional to increase type safety + +- Updated dependencies [[`063159313`](https://github.com/Khan/perseus/commit/063159313c8b146589912ce42c14f06aa23d3e51), [`13d79edb9`](https://github.com/Khan/perseus/commit/13d79edb94fd7009b18a176b5c93b43fb03fee72), [`790e189a7`](https://github.com/Khan/perseus/commit/790e189a7fdcd215d78d1999879ab2fc7417e123), [`ae51ccdb8`](https://github.com/Khan/perseus/commit/ae51ccdb820894f6fc5c1b23556823efdd4edba6), [`3a10f6b1f`](https://github.com/Khan/perseus/commit/3a10f6b1fe85d915fbf947434d7ebdc0b35607f5), [`ca4be05ab`](https://github.com/Khan/perseus/commit/ca4be05ab7367007330784796ad2561e3f5bb1c8), [`b9d1af181`](https://github.com/Khan/perseus/commit/b9d1af181efeb093407d59ba0a8efe8912524757), [`9f9d42c4e`](https://github.com/Khan/perseus/commit/9f9d42c4e2d041408cf508f5bfaeafe03dc2acbc), [`1ed045583`](https://github.com/Khan/perseus/commit/1ed045583fec01be5baf5d4e86a8b582cbf782c2), [`9efad87d0`](https://github.com/Khan/perseus/commit/9efad87d00c58f16c5a5a95c6c7148bde62fe71a), [`3dcb1fdf2`](https://github.com/Khan/perseus/commit/3dcb1fdf247eda0f0b78966daf04a9e4278d4373), [`20b3a2485`](https://github.com/Khan/perseus/commit/20b3a2485e2ba8deea798acc2732d9570c0dac45), [`03cddb6c3`](https://github.com/Khan/perseus/commit/03cddb6c39570e87ff2437273eb1287ff1417eec), [`1642ad9c0`](https://github.com/Khan/perseus/commit/1642ad9c0cadaf2e4db316e5e4cb38a5c9a9f5fe), [`391641acb`](https://github.com/Khan/perseus/commit/391641acb153d2d6c0f8c29f5026a392ac1b3a62), [`6f4702e41`](https://github.com/Khan/perseus/commit/6f4702e418ffdfaae01aa3f3a126b304b3250e34), [`56f33ae01`](https://github.com/Khan/perseus/commit/56f33ae010390abd2050028db98c9a72fc604e1a)]: + - @khanacademy/perseus@35.0.0 + ## 14.4.0 ### Minor Changes diff --git a/packages/perseus-editor/package.json b/packages/perseus-editor/package.json index 4c9ad4ef7b..80a047782c 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": "14.4.0", + "version": "14.5.0", "publishConfig": { "access": "public" }, @@ -38,7 +38,7 @@ "@khanacademy/keypad-context": "^1.0.1", "@khanacademy/kmath": "^0.1.13", "@khanacademy/math-input": "^21.0.2", - "@khanacademy/perseus": "^34.1.0", + "@khanacademy/perseus": "^35.0.0", "@khanacademy/perseus-core": "1.5.0", "mafs": "^0.19.0" }, diff --git a/packages/perseus/CHANGELOG.md b/packages/perseus/CHANGELOG.md index fc1c8cbbf6..699b2b4213 100644 --- a/packages/perseus/CHANGELOG.md +++ b/packages/perseus/CHANGELOG.md @@ -1,5 +1,47 @@ # @khanacademy/perseus +## 35.0.0 + +### Major Changes + +- [#1668](https://github.com/Khan/perseus/pull/1668) [`063159313`](https://github.com/Khan/perseus/commit/063159313c8b146589912ce42c14f06aa23d3e51) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove scoreWidgets from ServerItemRenderer + +* [#1639](https://github.com/Khan/perseus/pull/1639) [`ae51ccdb8`](https://github.com/Khan/perseus/commit/ae51ccdb820894f6fc5c1b23556823efdd4edba6) Thanks [@handeyeco](https://github.com/handeyeco)! - Refactor virtally all widget types and consolidate user input + +- [#1661](https://github.com/Khan/perseus/pull/1661) [`391641acb`](https://github.com/Khan/perseus/commit/391641acb153d2d6c0f8c29f5026a392ac1b3a62) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove unused onInputError from APIOptions + +### Minor Changes + +- [#1655](https://github.com/Khan/perseus/pull/1655) [`790e189a7`](https://github.com/Khan/perseus/commit/790e189a7fdcd215d78d1999879ab2fc7417e123) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked ellipse labels + +* [#1653](https://github.com/Khan/perseus/pull/1653) [`ca4be05ab`](https://github.com/Khan/perseus/commit/ca4be05ab7367007330784796ad2561e3f5bb1c8) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked ellipse labels + +- [#1652](https://github.com/Khan/perseus/pull/1652) [`1ed045583`](https://github.com/Khan/perseus/commit/1ed045583fec01be5baf5d4e86a8b582cbf782c2) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked vector labels + +* [#1659](https://github.com/Khan/perseus/pull/1659) [`3dcb1fdf2`](https://github.com/Khan/perseus/commit/3dcb1fdf247eda0f0b78966daf04a9e4278d4373) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked function labels + +- [#1658](https://github.com/Khan/perseus/pull/1658) [`20b3a2485`](https://github.com/Khan/perseus/commit/20b3a2485e2ba8deea798acc2732d9570c0dac45) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked function labels + +* [#1650](https://github.com/Khan/perseus/pull/1650) [`03cddb6c3`](https://github.com/Khan/perseus/commit/03cddb6c39570e87ff2437273eb1287ff1417eec) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked vector labels + +- [#1679](https://github.com/Khan/perseus/pull/1679) [`56f33ae01`](https://github.com/Khan/perseus/commit/56f33ae010390abd2050028db98c9a72fc604e1a) Thanks [@handeyeco](https://github.com/handeyeco)! - Don't serialize widgetIsOpen + +### Patch Changes + +- [#1669](https://github.com/Khan/perseus/pull/1669) [`13d79edb9`](https://github.com/Khan/perseus/commit/13d79edb94fd7009b18a176b5c93b43fb03fee72) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Updating protractor's default position within a graph. + +* [#1662](https://github.com/Khan/perseus/pull/1662) [`3a10f6b1f`](https://github.com/Khan/perseus/commit/3a10f6b1fe85d915fbf947434d7ebdc0b35607f5) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out validation logic in Radio + +- [#1656](https://github.com/Khan/perseus/pull/1656) [`b9d1af181`](https://github.com/Khan/perseus/commit/b9d1af181efeb093407d59ba0a8efe8912524757) Thanks [@Myranae](https://github.com/Myranae)! - Move validation logic out of the sorter widget and add tests + +* [#1665](https://github.com/Khan/perseus/pull/1665) [`9f9d42c4e`](https://github.com/Khan/perseus/commit/9f9d42c4e2d041408cf508f5bfaeafe03dc2acbc) Thanks [@nicolecomputer](https://github.com/nicolecomputer)! - Translations and polish for unlimited point + +- [#1667](https://github.com/Khan/perseus/pull/1667) [`9efad87d0`](https://github.com/Khan/perseus/commit/9efad87d00c58f16c5a5a95c6c7148bde62fe71a) Thanks [@handeyeco](https://github.com/handeyeco)! - Split validation logic from LabelImage + +* [#1663](https://github.com/Khan/perseus/pull/1663) [`1642ad9c0`](https://github.com/Khan/perseus/commit/1642ad9c0cadaf2e4db316e5e4cb38a5c9a9f5fe) Thanks [@benchristel](https://github.com/benchristel)! - Internal: revert buggy change to interactive graphs (never shipped) + +- [#1673](https://github.com/Khan/perseus/pull/1673) [`6f4702e41`](https://github.com/Khan/perseus/commit/6f4702e418ffdfaae01aa3f3a126b304b3250e34) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figures Labels] Make labels optional to increase type safety + ## 34.1.0 ### Minor Changes diff --git a/packages/perseus/package.json b/packages/perseus/package.json index 76259404dc..5c8135a441 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": "34.1.0", + "version": "35.0.0", "publishConfig": { "access": "public" }, From 463755970951a97db23baa5f73084549fe56c936 Mon Sep 17 00:00:00 2001 From: Matthew Date: Thu, 26 Sep 2024 11:48:22 -0500 Subject: [PATCH 03/14] Matrix: split out validator (#1670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Split out validation logic from Matrix Issue: LEMS-2363 ## Test plan: Nothing should change, just splitting out code Author: handeyeco Reviewers: catandthemachines, handeyeco, #perseus, jeremywiebe, Myranae Required Reviewers: Approved By: catandthemachines Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1670 --- .changeset/sour-spoons-rush.md | 5 + .../widgets/matrix/matrix-validator.test.ts | 176 ++++++++++++++++++ .../src/widgets/matrix/matrix-validator.ts | 85 +++++++++ .../perseus/src/widgets/matrix/matrix.tsx | 76 +------- 4 files changed, 272 insertions(+), 70 deletions(-) create mode 100644 .changeset/sour-spoons-rush.md create mode 100644 packages/perseus/src/widgets/matrix/matrix-validator.test.ts create mode 100644 packages/perseus/src/widgets/matrix/matrix-validator.ts diff --git a/.changeset/sour-spoons-rush.md b/.changeset/sour-spoons-rush.md new file mode 100644 index 0000000000..61dc3416f1 --- /dev/null +++ b/.changeset/sour-spoons-rush.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Split validation logic out of Matrix diff --git a/packages/perseus/src/widgets/matrix/matrix-validator.test.ts b/packages/perseus/src/widgets/matrix/matrix-validator.test.ts new file mode 100644 index 0000000000..5e4af99f0d --- /dev/null +++ b/packages/perseus/src/widgets/matrix/matrix-validator.test.ts @@ -0,0 +1,176 @@ +import {mockStrings} from "../../strings"; + +import matrixValidator from "./matrix-validator"; + +import type { + PerseusMatrixRubric, + PerseusMatrixUserInput, +} from "../../validation.types"; + +describe("matrixValidator", () => { + it("can be answered correctly", () => { + // Arrange + const rubric: PerseusMatrixRubric = { + prefix: "", + suffix: "", + answers: [ + [0, 1, 2], + [3, 4, 5], + [6, 7, 8], + ], + cursorPosition: [], + matrixBoardSize: [], + static: false, + }; + + const userInput: PerseusMatrixUserInput = { + answers: rubric.answers, + }; + + // Act + const result = matrixValidator(userInput, rubric, mockStrings); + + // Assert + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("can be answered incorrectly", () => { + // Arrange + const rubric: PerseusMatrixRubric = { + prefix: "", + suffix: "", + answers: [ + [0, 1, 2], + [3, 4, 5], + [6, 7, 8], + ], + cursorPosition: [], + matrixBoardSize: [], + static: false, + }; + + const userInput: PerseusMatrixUserInput = { + answers: [ + [0, 0, 0], + [0, 0, 0], + [0, 0, 0], + ], + }; + + // Act + const result = matrixValidator(userInput, rubric, mockStrings); + + // Assert + expect(result).toHaveBeenAnsweredIncorrectly(); + }); + + it("is invalid when there's an empty cell: null", () => { + // Arrange + const rubric: PerseusMatrixRubric = { + prefix: "", + suffix: "", + answers: [ + [0, 1, 2], + [3, 4, 5], + [6, 7, 8], + ], + cursorPosition: [], + matrixBoardSize: [], + static: false, + }; + + const userInput: PerseusMatrixUserInput = { + answers: [ + // TODO: this is either legacy logic or an incorrect type, + // but this is what the validator is checking for + // @ts-expect-error - TS(2322) - Type 'null' is not assignable to type 'number'. + [0, 0, null], + [0, 0, 0], + [0, 0, 0], + ], + }; + + // Act + const result = matrixValidator(userInput, rubric, mockStrings); + + // Assert + expect(result).toHaveInvalidInput(); + }); + + it("is invalid when there's an empty cell: empty string", () => { + // Arrange + const rubric: PerseusMatrixRubric = { + prefix: "", + suffix: "", + answers: [ + [0, 1, 2], + [3, 4, 5], + [6, 7, 8], + ], + cursorPosition: [], + matrixBoardSize: [], + static: false, + }; + + const userInput: PerseusMatrixUserInput = { + answers: [ + // TODO: this is either legacy logic or an incorrect type, + // but this is what the validator is checking for + // @ts-expect-error - TS(2322) - Type 'null' is not assignable to type 'number'. + [0, 0, ""], + [0, 0, 0], + [0, 0, 0], + ], + }; + + // Act + const result = matrixValidator(userInput, rubric, mockStrings); + + // Assert + expect(result).toHaveInvalidInput(); + }); + + it("is considered incorrect when the size is wrong", () => { + // Arrange + const rubric: PerseusMatrixRubric = { + prefix: "", + suffix: "", + answers: [ + [0, 1, 2], + [3, 4, 5], + [6, 7, 8], + ], + cursorPosition: [], + matrixBoardSize: [], + static: false, + }; + + const correctUserInput: PerseusMatrixUserInput = { + answers: rubric.answers, + }; + + const incorrectUserInput: PerseusMatrixUserInput = { + // Base the incorrect answer off of the correct answer. + // This is so we can check that it's considered incorrect + // if it has the wrong length, even though it otherwise + // would be a partial match. + answers: [...rubric.answers, [8, 6, 7]], + }; + + // Act + const correctResult = matrixValidator( + correctUserInput, + rubric, + mockStrings, + ); + const incorrectResult = matrixValidator( + incorrectUserInput, + rubric, + mockStrings, + ); + + // Assert + expect(correctResult).toHaveBeenAnsweredCorrectly(); + expect(incorrectResult).toHaveBeenAnsweredIncorrectly(); + }); +}); diff --git a/packages/perseus/src/widgets/matrix/matrix-validator.ts b/packages/perseus/src/widgets/matrix/matrix-validator.ts new file mode 100644 index 0000000000..599224d5f6 --- /dev/null +++ b/packages/perseus/src/widgets/matrix/matrix-validator.ts @@ -0,0 +1,85 @@ +import _ from "underscore"; + +import KhanAnswerTypes from "../../util/answer-types"; + +import {getMatrixSize} from "./matrix"; + +import type {PerseusStrings} from "../../strings"; +import type {PerseusScore} from "../../types"; +import type { + PerseusMatrixRubric, + PerseusMatrixUserInput, +} from "../../validation.types"; + +function matrixValidator( + state: PerseusMatrixUserInput, + rubric: PerseusMatrixRubric, + strings: PerseusStrings, +): PerseusScore { + const solution = rubric.answers; + const supplied = state.answers; + const solutionSize = getMatrixSize(solution); + const suppliedSize = getMatrixSize(supplied); + + const incorrectSize = + solutionSize[0] !== suppliedSize[0] || + solutionSize[1] !== suppliedSize[1]; + + const createValidator = KhanAnswerTypes.number.createValidatorFunctional; + let message = null; + let hasEmptyCell = false; + let incorrect = false; + _(suppliedSize[0]).times((row) => { + _(suppliedSize[1]).times((col) => { + if ( + supplied[row][col] == null || + supplied[row][col].toString().length === 0 + ) { + hasEmptyCell = true; + } + if (!incorrectSize) { + const validator = createValidator( + // @ts-expect-error - TS2345 - Argument of type 'number' is not assignable to parameter of type 'string'. + solution[row][col], + { + simplify: true, + }, + strings, + ); + const result = validator(supplied[row][col]); + if (result.message) { + // @ts-expect-error - TS2322 - Type 'string' is not assignable to type 'null'. + message = result.message; + } + if (!result.correct) { + incorrect = true; + } + } + }); + }); + + if (hasEmptyCell) { + return { + type: "invalid", + message: strings.fillAllCells, + }; + } + + if (incorrectSize) { + return { + type: "points", + earned: 0, + total: 1, + message: null, + }; + } + + return { + type: "points", + earned: incorrect ? 0 : 1, + total: 1, + message: message, + }; +} + +export default matrixValidator; diff --git a/packages/perseus/src/widgets/matrix/matrix.tsx b/packages/perseus/src/widgets/matrix/matrix.tsx index be9d5e515b..59e1edb629 100644 --- a/packages/perseus/src/widgets/matrix/matrix.tsx +++ b/packages/perseus/src/widgets/matrix/matrix.tsx @@ -13,9 +13,9 @@ import InteractiveUtil from "../../interactive2/interactive-util"; import {ApiOptions} from "../../perseus-api"; import Renderer from "../../renderer"; import Util from "../../util"; -import KhanAnswerTypes from "../../util/answer-types"; -// Type imports +import matrixValidator from "./matrix-validator"; + import type {PerseusMatrixWidgetOptions} from "../../perseus-types"; import type {PerseusStrings} from "../../strings"; import type {WidgetExports, WidgetProps, PerseusScore} from "../../types"; @@ -74,7 +74,7 @@ const getRefForPath = function (path) { return "answer" + row + "," + column; }; -const getMatrixSize = function (matrix: ReadonlyArray>) { +export function getMatrixSize(matrix: ReadonlyArray>) { const matrixSize = [1, 1]; // We need to find the widest row and tallest column to get the correct @@ -96,7 +96,7 @@ const getMatrixSize = function (matrix: ReadonlyArray>) { } }); return matrixSize; -}; +} type ExternalProps = WidgetProps< PerseusMatrixWidgetOptions, @@ -149,71 +149,7 @@ class Matrix extends React.Component { rubric: PerseusMatrixRubric, strings: PerseusStrings, ): PerseusScore { - const solution = rubric.answers; - const supplied = state.answers; - const solutionSize = getMatrixSize(solution); - const suppliedSize = getMatrixSize(supplied); - - const incorrectSize = - solutionSize[0] !== suppliedSize[0] || - solutionSize[1] !== suppliedSize[1]; - - const createValidator = - KhanAnswerTypes.number.createValidatorFunctional; - let message = null; - let hasEmptyCell = false; - let incorrect = false; - _(suppliedSize[0]).times((row) => { - _(suppliedSize[1]).times((col) => { - if ( - supplied[row][col] == null || - supplied[row][col].toString().length === 0 - ) { - hasEmptyCell = true; - } - if (!incorrectSize) { - const validator = createValidator( - // @ts-expect-error - TS2345 - Argument of type 'number' is not assignable to parameter of type 'string'. - solution[row][col], - { - simplify: true, - }, - strings, - ); - const result = validator(supplied[row][col]); - if (result.message) { - // @ts-expect-error - TS2322 - Type 'string' is not assignable to type 'null'. - message = result.message; - } - if (!result.correct) { - incorrect = true; - } - } - }); - }); - - if (hasEmptyCell) { - return { - type: "invalid", - message: strings.fillAllCells, - }; - } - - if (incorrectSize) { - return { - type: "points", - earned: 0, - total: 1, - message: null, - }; - } - - return { - type: "points", - earned: incorrect ? 0 : 1, - total: 1, - message: message, - }; + return matrixValidator(state, rubric, strings); } state: State = { @@ -397,7 +333,7 @@ class Matrix extends React.Component { } simpleValidate(rubric: PerseusMatrixRubric) { - return Matrix.validate( + return matrixValidator( this.getUserInput(), rubric, this.context.strings, From f326139ee6bf9680075eac6353b1e84c44fa3f77 Mon Sep 17 00:00:00 2001 From: Matthew Date: Fri, 27 Sep 2024 09:35:05 -0500 Subject: [PATCH 04/14] Grapher: move / test validator (#1671) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Grapher's validation logic was already split out, but this moves things around so it's consistent with other components (and adds tests) Issue: LEMS-2360 ## Test plan: Nothing should change, just moving coe Author: handeyeco Reviewers: Myranae, handeyeco, jeremywiebe Required Reviewers: Approved By: Myranae Checks: ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (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), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1671 --- .changeset/curly-beers-live.md | 5 + .../widgets/grapher/grapher-validator.test.ts | 196 ++++++++++++++++++ .../src/widgets/grapher/grapher-validator.ts | 61 ++++++ .../perseus/src/widgets/grapher/grapher.tsx | 6 +- packages/perseus/src/widgets/grapher/util.tsx | 49 ----- 5 files changed, 265 insertions(+), 52 deletions(-) create mode 100644 .changeset/curly-beers-live.md create mode 100644 packages/perseus/src/widgets/grapher/grapher-validator.test.ts create mode 100644 packages/perseus/src/widgets/grapher/grapher-validator.ts diff --git a/.changeset/curly-beers-live.md b/.changeset/curly-beers-live.md new file mode 100644 index 0000000000..e51475d965 --- /dev/null +++ b/.changeset/curly-beers-live.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Move and test Grapher's validator diff --git a/packages/perseus/src/widgets/grapher/grapher-validator.test.ts b/packages/perseus/src/widgets/grapher/grapher-validator.test.ts new file mode 100644 index 0000000000..70614e3932 --- /dev/null +++ b/packages/perseus/src/widgets/grapher/grapher-validator.test.ts @@ -0,0 +1,196 @@ +import grapherValidator from "./grapher-validator"; + +import type {Coord} from "../../interactive2/types"; +import type { + PerseusGrapherRubric, + PerseusGrapherUserInput, +} from "../../validation.types"; + +describe("grapherValidator", () => { + it("is incorrect when user input type doesn't match rubric type", () => { + const asymptote: [Coord, Coord] = [ + [-10, -10], + [10, 10], + ]; + const coords: [Coord, Coord] = [ + [-10, -10], + [10, 10], + ]; + + // Arrange + const userInput: PerseusGrapherUserInput = { + type: "exponential", + asymptote, + coords, + }; + + const rubric: PerseusGrapherRubric = { + availableTypes: ["exponential", "logarithm"], + correct: { + type: "logarithm", + asymptote, + coords, + }, + // The rubric type is probably wrong, + // the validator doesn't use graph + graph: {} as any, + }; + + // Act + const result = grapherValidator(userInput, rubric); + + // Assert + expect(result).toHaveBeenAnsweredIncorrectly(); + }); + + it("is invalid when user input doesn't have coords", () => { + const asymptote: [Coord, Coord] = [ + [-10, -10], + [10, 10], + ]; + const coords: [Coord, Coord] = [ + [-10, -10], + [10, 10], + ]; + + // Arrange + const userInput: PerseusGrapherUserInput = { + type: "exponential", + asymptote, + // TODO: either the types or logic is wrong, + // but the existing validator checks for null coords + // @ts-expect-error - TS(2322) - Type 'null' is not assignable to type 'readonly Coord[]'. + coords: null, + }; + + const rubric: PerseusGrapherRubric = { + availableTypes: ["exponential", "logarithm"], + correct: { + type: "exponential", + asymptote, + coords, + }, + // The rubric type is probably wrong, + // the validator doesn't use graph + graph: {} as any, + }; + + // Act + const result = grapherValidator(userInput, rubric); + + // Assert + expect(result).toHaveInvalidInput(); + }); + + it("is invalid when coefficients are unexpected", () => { + // I honestly don't understand what a coefficient is + // but this seems to get triggered when the type is "linear" + // and the points are in the same spot + const asymptote: [Coord, Coord] = [ + [-10, -10], + [-10, -10], + ]; + const coords: [Coord, Coord] = [ + [-10, -10], + [-10, -10], + ]; + + // Arrange + const userInput: PerseusGrapherUserInput = { + type: "linear", + asymptote, + coords, + }; + + const rubric: PerseusGrapherRubric = { + availableTypes: ["linear"], + correct: { + type: "linear", + coords, + }, + // The rubric type is probably wrong, + // the validator doesn't use graph + graph: {} as any, + }; + + // Act + const result = grapherValidator(userInput, rubric); + + // Assert + expect(result).toHaveInvalidInput(); + }); + + it("can be answered correctly", () => { + const asymptote: [Coord, Coord] = [ + [-10, -10], + [10, 10], + ]; + const coords: [Coord, Coord] = [ + [-10, -10], + [10, 10], + ]; + + // Arrange + const userInput: PerseusGrapherUserInput = { + type: "linear", + asymptote, + coords, + }; + + const rubric: PerseusGrapherRubric = { + availableTypes: ["linear"], + correct: { + type: "linear", + coords, + }, + // The rubric type is probably wrong, + // the validator doesn't use graph + graph: {} as any, + }; + + // Act + const result = grapherValidator(userInput, rubric); + + // Assert + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("can be answered incorrectly when user input and rubric coords don't match", () => { + // TODO: user input type is probably wrong, + // I don't think asymptote is needed for all types + const asymptote: [Coord, Coord] = [ + [10, 10], + [-10, -10], + ]; + + // Arrange + const userInput: PerseusGrapherUserInput = { + type: "linear", + asymptote, + coords: [ + [2, 3], + [-4, -5], + ], + }; + + const rubric: PerseusGrapherRubric = { + availableTypes: ["linear"], + correct: { + type: "linear", + coords: [ + [-10, -10], + [10, 10], + ], + }, + // The rubric type is probably wrong, + // the validator doesn't use graph + graph: {} as any, + }; + + // Act + const result = grapherValidator(userInput, rubric); + + // Assert + expect(result).toHaveBeenAnsweredIncorrectly(); + }); +}); diff --git a/packages/perseus/src/widgets/grapher/grapher-validator.ts b/packages/perseus/src/widgets/grapher/grapher-validator.ts new file mode 100644 index 0000000000..74e2e93600 --- /dev/null +++ b/packages/perseus/src/widgets/grapher/grapher-validator.ts @@ -0,0 +1,61 @@ +import {functionForType} from "./util"; + +import type {PerseusScore} from "../../types"; +import type { + PerseusGrapherRubric, + PerseusGrapherUserInput, +} from "../../validation.types"; + +function grapherValidator( + state: PerseusGrapherUserInput, + rubric: PerseusGrapherRubric, +): PerseusScore { + if (state.type !== rubric.correct.type) { + return { + type: "points", + earned: 0, + total: 1, + message: null, + }; + } + + // We haven't moved the coords + if (state.coords == null) { + return { + type: "invalid", + message: null, + }; + } + + // Get new function handler for grading + const grader = functionForType(state.type); + const guessCoeffs = grader.getCoefficients(state.coords, state.asymptote); + const correctCoeffs = grader.getCoefficients( + rubric.correct.coords, + // @ts-expect-error - TS(2339) - Property 'asymptote' does not exist on type '{ type: "absolute_value"; coords: [Coord, Coord]; }' + rubric.correct.asymptote, + ); + + if (guessCoeffs == null || correctCoeffs == null) { + return { + type: "invalid", + message: null, + }; + } + if (grader.areEqual(guessCoeffs, correctCoeffs)) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + return { + type: "points", + earned: 0, + total: 1, + message: null, + }; +} + +export default grapherValidator; diff --git a/packages/perseus/src/widgets/grapher/grapher.tsx b/packages/perseus/src/widgets/grapher/grapher.tsx index 8301ab5e63..587c5a58e3 100644 --- a/packages/perseus/src/widgets/grapher/grapher.tsx +++ b/packages/perseus/src/widgets/grapher/grapher.tsx @@ -19,6 +19,7 @@ import {getInteractiveBoxFromSizeClass} from "../../util/sizing-utils"; /* Graphie and relevant components. */ /* Mixins. */ +import grapherValidator from "./grapher-validator"; import { DEFAULT_GRAPHER_PROPS, chooseType, @@ -27,7 +28,6 @@ import { getGridAndSnapSteps, maybePointsFromNormalized, typeToButton, - validate as grapherValidate, } from "./util"; import type {Coord, Line} from "../../interactive2/types"; @@ -371,7 +371,7 @@ class Grapher extends React.Component { state: PerseusGrapherUserInput, rubric: PerseusGrapherRubric, ): PerseusScore { - return grapherValidate(state, rubric); + return grapherValidator(state, rubric); } static getUserInputFromProps(props: Props): PerseusGrapherUserInput { @@ -541,7 +541,7 @@ class Grapher extends React.Component { }; simpleValidate(rubric: PerseusGrapherRubric): PerseusScore { - return grapherValidate(this.getUserInput(), rubric); + return grapherValidator(this.getUserInput(), rubric); } getUserInput(): PerseusGrapherUserInput { diff --git a/packages/perseus/src/widgets/grapher/util.tsx b/packages/perseus/src/widgets/grapher/util.tsx index 9150dc0e43..3058555c9d 100644 --- a/packages/perseus/src/widgets/grapher/util.tsx +++ b/packages/perseus/src/widgets/grapher/util.tsx @@ -8,7 +8,6 @@ import {getDependencies} from "../../dependencies"; import Util from "../../util"; import type {Coord} from "../../interactive2/types"; -import type {PerseusScore} from "../../types"; // @ts-expect-error - TS2339 - Property 'Plot' does not exist on type 'typeof Graphie'. const Plot = Graphie.Plot; @@ -596,54 +595,6 @@ export function functionForType( return functionTypeMapping[type]; } -export const validate = (state: any, rubric: any): PerseusScore => { - if (state.type !== rubric.correct.type) { - return { - type: "points", - earned: 0, - total: 1, - message: null, - }; - } - - // We haven't moved the coords - if (state.coords == null) { - return { - type: "invalid", - message: null, - }; - } - - // Get new function handler for grading - const grader = functionForType(state.type); - const guessCoeffs = grader.getCoefficients(state.coords, state.asymptote); - const correctCoeffs = grader.getCoefficients( - rubric.correct.coords, - rubric.correct.asymptote, - ); - - if (guessCoeffs == null || correctCoeffs == null) { - return { - type: "invalid", - message: null, - }; - } - if (grader.areEqual(guessCoeffs, correctCoeffs)) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - return { - type: "points", - earned: 0, - total: 1, - message: null, - }; -}; - export const getEquationString = (props: any): string => { const plot = props.plot; if (plot.type && plot.coords) { From 49efaaff5235bea1b6499df6a9d05fca7d022cd2 Mon Sep 17 00:00:00 2001 From: Matthew Date: Fri, 27 Sep 2024 09:35:59 -0500 Subject: [PATCH 05/14] Port tests to custom matcher (#1678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: We made a custom matcher to make score tests easier to grok. This ports some existing tests over to that. Author: handeyeco Reviewers: Myranae Required Reviewers: Approved By: Myranae Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1678 --- .changeset/great-lions-switch.md | 5 ++++ .../src/widgets/definition/definition.test.ts | 7 +---- .../src/widgets/interactive-graph.test.tsx | 26 +++---------------- 3 files changed, 10 insertions(+), 28 deletions(-) create mode 100644 .changeset/great-lions-switch.md diff --git a/.changeset/great-lions-switch.md b/.changeset/great-lions-switch.md new file mode 100644 index 0000000000..5125f835af --- /dev/null +++ b/.changeset/great-lions-switch.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Port some tests to new custom matcher diff --git a/packages/perseus/src/widgets/definition/definition.test.ts b/packages/perseus/src/widgets/definition/definition.test.ts index ca931b4a60..499cc1f017 100644 --- a/packages/perseus/src/widgets/definition/definition.test.ts +++ b/packages/perseus/src/widgets/definition/definition.test.ts @@ -128,11 +128,6 @@ describe("Definition widget", () => { const result = renderer.scoreWidgets(); // Assert - expect(result["definition 1"]).toMatchObject({ - type: "points", - earned: 0, - total: 0, - message: null, - }); + expect(result["definition 1"]).toHaveBeenAnsweredCorrectly(); }); }); diff --git a/packages/perseus/src/widgets/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graph.test.tsx index 7f0973ca72..af489280d5 100644 --- a/packages/perseus/src/widgets/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graph.test.tsx @@ -30,10 +30,7 @@ describe("InteractiveGraph.validate on a segment question", () => { const result = InteractiveGraph.widget.validate(guess, rubric, null); - expect(result).toEqual({ - type: "invalid", - message: null, - }); + expect(result).toHaveInvalidInput(); }); it("does not award points if guess.coords is wrong", () => { @@ -58,12 +55,7 @@ describe("InteractiveGraph.validate on a segment question", () => { const result = InteractiveGraph.widget.validate(guess, rubric, null); - expect(result).toEqual({ - type: "points", - earned: 0, - total: 1, - message: null, - }); + expect(result).toHaveBeenAnsweredIncorrectly(); }); it("awards points if guess.coords is right", () => { @@ -88,12 +80,7 @@ describe("InteractiveGraph.validate on a segment question", () => { const result = InteractiveGraph.widget.validate(guess, rubric, null); - expect(result).toEqual({ - type: "points", - earned: 1, - total: 1, - message: null, - }); + expect(result).toHaveBeenAnsweredCorrectly(); }); it("allows points of a segment to be specified in reverse order", () => { @@ -118,12 +105,7 @@ describe("InteractiveGraph.validate on a segment question", () => { const result = InteractiveGraph.widget.validate(guess, rubric, null); - expect(result).toEqual({ - type: "points", - earned: 1, - total: 1, - message: null, - }); + expect(result).toHaveBeenAnsweredCorrectly(); }); it("does not modify the `guess` data", () => { From 10ce869258bc8506aba848c06ada8e5ae5fca4ff Mon Sep 17 00:00:00 2001 From: Tamara <60857422+Myranae@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:08:54 -0500 Subject: [PATCH 06/14] Split out validation logic for cs-program (#1688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: For the server-side scoring project, we are moving validation logic out of each widget component. This moves the validation logic for the cs-program widget and adds tests for the validator function. Issue: LEMS-2353 ## Test plan: - Confirm all tests pass - Nothing should change. Code is only being moved. Author: Myranae Reviewers: Myranae, handeyeco Required Reviewers: Approved By: handeyeco 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), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1688 --- .changeset/strong-numbers-pretend.md | 5 ++ .../cs-program/cs-program-validator.test.ts | 49 +++++++++++++++++++ .../cs-program/cs-program-validator.ts | 29 +++++++++++ .../src/widgets/cs-program/cs-program.tsx | 43 ++++------------ 4 files changed, 92 insertions(+), 34 deletions(-) create mode 100644 .changeset/strong-numbers-pretend.md create mode 100644 packages/perseus/src/widgets/cs-program/cs-program-validator.test.ts create mode 100644 packages/perseus/src/widgets/cs-program/cs-program-validator.ts diff --git a/.changeset/strong-numbers-pretend.md b/.changeset/strong-numbers-pretend.md new file mode 100644 index 0000000000..4ae9e6a808 --- /dev/null +++ b/.changeset/strong-numbers-pretend.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Move validation logic out of the cs-program widget and add tests diff --git a/packages/perseus/src/widgets/cs-program/cs-program-validator.test.ts b/packages/perseus/src/widgets/cs-program/cs-program-validator.test.ts new file mode 100644 index 0000000000..af61626dfd --- /dev/null +++ b/packages/perseus/src/widgets/cs-program/cs-program-validator.test.ts @@ -0,0 +1,49 @@ +import {csProgramValidator} from "./cs-program-validator"; + +import type {PerseusCSProgramUserInput} from "../../validation.types"; + +describe("csProgramValidator", () => { + it("is correct when the state from the iframe shows the status is correct", () => { + // Arrange + const state: PerseusCSProgramUserInput = { + status: "correct", + message: "Good job!", + }; + + // Act + const result = csProgramValidator(state); + + // Assert + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("is incorrect when the state from the iframe shows the status is incorrect", () => { + // Arrange + const state: PerseusCSProgramUserInput = { + status: "incorrect", + message: "Try again!", + }; + + // Act + const result = csProgramValidator(state); + + // Assert + expect(result).toHaveBeenAnsweredIncorrectly(); + }); + + // Note: It looks like the iframe only says if the answer is correct or + // incorrect, but status is set to "incomplete" by default. + it("should return invalid score before user interactions", () => { + // Arrange + const state: PerseusCSProgramUserInput = { + status: "incomplete", + message: null, + }; + + // Act + const result = csProgramValidator(state); + + // Assert + expect(result).toHaveInvalidInput("Keep going, you're not there yet!"); + }); +}); diff --git a/packages/perseus/src/widgets/cs-program/cs-program-validator.ts b/packages/perseus/src/widgets/cs-program/cs-program-validator.ts new file mode 100644 index 0000000000..7d1f1445f4 --- /dev/null +++ b/packages/perseus/src/widgets/cs-program/cs-program-validator.ts @@ -0,0 +1,29 @@ +import type {PerseusScore} from "../../types"; +import type {PerseusCSProgramUserInput} from "../../validation.types"; + +export function csProgramValidator( + state: PerseusCSProgramUserInput, +): PerseusScore { + // The iframe can tell us whether it's correct or incorrect, + // and pass an optional message + if (state.status === "correct") { + return { + type: "points", + earned: 1, + total: 1, + message: state.message || null, + }; + } + if (state.status === "incorrect") { + return { + type: "points", + earned: 0, + total: 1, + message: state.message || null, + }; + } + return { + type: "invalid", + message: "Keep going, you're not there yet!", + }; +} diff --git a/packages/perseus/src/widgets/cs-program/cs-program.tsx b/packages/perseus/src/widgets/cs-program/cs-program.tsx index 453819433e..64e4215eb7 100644 --- a/packages/perseus/src/widgets/cs-program/cs-program.tsx +++ b/packages/perseus/src/widgets/cs-program/cs-program.tsx @@ -14,6 +14,8 @@ import Util from "../../util"; import {isFileProtocol} from "../../util/mobile-native-utils"; import {toAbsoluteUrl} from "../../util/url-utils"; +import {csProgramValidator} from "./cs-program-validator"; + import type {PerseusCSProgramWidgetOptions} from "../../perseus-types"; import type {PerseusScore, WidgetExports, WidgetProps} from "../../types"; import type { @@ -67,32 +69,8 @@ class CSProgram extends React.Component { }; // The widget's grading function - static validate( - state: PerseusCSProgramUserInput, - rubric: any, - ): PerseusScore { - // The iframe can tell us whether it's correct or incorrect, - // and pass an optional message - if (state.status === "correct") { - return { - type: "points", - earned: 1, - total: 1, - message: state.message || null, - }; - } - if (state.status === "incorrect") { - return { - type: "points", - earned: 0, - total: 1, - message: state.message || null, - }; - } - return { - type: "invalid", - message: "Keep going, you're not there yet!", - }; + static validate(state: PerseusCSProgramUserInput): PerseusScore { + return csProgramValidator(state); } componentDidMount() { @@ -130,14 +108,11 @@ class CSProgram extends React.Component { return Changeable.change.apply(this, args); }; - simpleValidate(rubric): PerseusScore { - return CSProgram.validate( - { - status: this.props.status, - message: this.props.message, - }, - rubric, - ); + simpleValidate(): PerseusScore { + return csProgramValidator({ + status: this.props.status, + message: this.props.message, + }); } render(): React.ReactNode { From f5af2437133d68dcbaa42830850341c46d7affee Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Fri, 27 Sep 2024 12:40:07 -0700 Subject: [PATCH 07/14] Delete unused component param of InteractiveGraph.validate() (#1676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This parameter appeared to be used to score point graphs. It was passed to `getPointCoords()`. However, on closer inspection, there was no way this code could have worked: `getPointCoords` takes the interactive graph props as an argument, but we were passing it the entire component. This props param was only being used if the rubric contained no coords. Maybe that's a realistic case, or maybe it's not, but either way, it was throwing an error trying to access `props.range[0]`. Long story short, we lose nothing by deleting the `component` param and explicitly throwing an error if the rubric has no coords. Issue: LEMS-2448 ## Test plan: Run this script to copy point graph data to your clipboard: ``` grep -rl '"type":"point"' data/questions/ | xargs cat | pbcopy ``` Paste it into `http://localhost:5173#flipbook`. Solve some point graph questions. Author: benchristel Reviewers: handeyeco, benchristel Required Reviewers: Approved By: handeyeco Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (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), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1676 --- .changeset/honest-games-do.md | 5 + .../src/widgets/interactive-graph.test.tsx | 146 +++++++++++++++++- .../perseus/src/widgets/interactive-graph.tsx | 13 +- 3 files changed, 151 insertions(+), 13 deletions(-) create mode 100644 .changeset/honest-games-do.md diff --git a/.changeset/honest-games-do.md b/.changeset/honest-games-do.md new file mode 100644 index 0000000000..050ecdf11e --- /dev/null +++ b/.changeset/honest-games-do.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Internal: remove dead code from InteractiveGraph.validate() diff --git a/packages/perseus/src/widgets/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graph.test.tsx index af489280d5..aba4dae7b1 100644 --- a/packages/perseus/src/widgets/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graph.test.tsx @@ -1,5 +1,7 @@ import invariant from "tiny-invariant"; +import {clone} from "../../../../testing/object-utils"; + import InteractiveGraph, {shouldUseMafs} from "./interactive-graph"; import type { @@ -28,7 +30,7 @@ describe("InteractiveGraph.validate on a segment question", () => { ], }); - const result = InteractiveGraph.widget.validate(guess, rubric, null); + const result = InteractiveGraph.widget.validate(guess, rubric); expect(result).toHaveInvalidInput(); }); @@ -53,7 +55,7 @@ describe("InteractiveGraph.validate on a segment question", () => { ], }); - const result = InteractiveGraph.widget.validate(guess, rubric, null); + const result = InteractiveGraph.widget.validate(guess, rubric); expect(result).toHaveBeenAnsweredIncorrectly(); }); @@ -78,7 +80,7 @@ describe("InteractiveGraph.validate on a segment question", () => { ], }); - const result = InteractiveGraph.widget.validate(guess, rubric, null); + const result = InteractiveGraph.widget.validate(guess, rubric); expect(result).toHaveBeenAnsweredCorrectly(); }); @@ -103,7 +105,7 @@ describe("InteractiveGraph.validate on a segment question", () => { ], }); - const result = InteractiveGraph.widget.validate(guess, rubric, null); + const result = InteractiveGraph.widget.validate(guess, rubric); expect(result).toHaveBeenAnsweredCorrectly(); }); @@ -128,7 +130,7 @@ describe("InteractiveGraph.validate on a segment question", () => { ], }); - InteractiveGraph.widget.validate(guess, rubric, null); + InteractiveGraph.widget.validate(guess, rubric); expect(guess.coords).toEqual([ [ @@ -158,7 +160,7 @@ describe("InteractiveGraph.validate on a segment question", () => { ], }); - InteractiveGraph.widget.validate(guess, rubric, null); + InteractiveGraph.widget.validate(guess, rubric); // Narrow the type of `rubric.correct` to segment graph; otherwise TS // thinks it might not have a `coords` property. @@ -172,6 +174,138 @@ describe("InteractiveGraph.validate on a segment question", () => { }); }); +describe("InteractiveGraph.validate on a point question", () => { + it("marks the answer invalid if guess.coords is missing", () => { + const guess: PerseusGraphType = {type: "point"}; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [[0, 0]], + }); + + const result = InteractiveGraph.widget.validate(guess, rubric); + + expect(result).toHaveInvalidInput(); + }); + + it("throws an exception if correct.coords is missing", () => { + // Characterization test: this might not be desirable behavior, but + // it's the current behavior as of 2024-09-25. + const guess: PerseusGraphType = { + type: "point", + coords: [[0, 0]], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + }); + + expect(() => + InteractiveGraph.widget.validate(guess, rubric), + ).toThrowError(); + }); + + it("does not award points if guess.coords is wrong", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [[9, 9]], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [[0, 0]], + }); + + const result = InteractiveGraph.widget.validate(guess, rubric); + + expect(result).toHaveBeenAnsweredIncorrectly(); + }); + + it("awards points if guess.coords is right", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [[7, 8]], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [[7, 8]], + }); + + const result = InteractiveGraph.widget.validate(guess, rubric); + + expect(result).toEqual({ + type: "points", + earned: 1, + total: 1, + message: null, + }); + }); + + it("allows points to be specified in any order", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [ + [7, 8], + [5, 6], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [ + [5, 6], + [7, 8], + ], + }); + + const result = InteractiveGraph.widget.validate(guess, rubric); + + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("does not modify the `guess` data", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [ + [7, 8], + [5, 6], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [ + [5, 6], + [7, 8], + ], + }); + + const guessClone = clone(guess); + + InteractiveGraph.widget.validate(guess, rubric); + + expect(guess).toEqual(guessClone); + }); + + it("does not modify the `rubric` data", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [ + [7, 8], + [5, 6], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [ + [5, 6], + [7, 8], + ], + }); + + const rubricClone = clone(rubric); + + InteractiveGraph.widget.validate(guess, rubric); + + expect(rubric).toEqual(rubricClone); + }); +}); + describe("shouldUseMafs", () => { it("is false given no mafs flags", () => { const graph: PerseusGraphTypeLinear = { diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index 5086143532..962f64b8e4 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -1681,7 +1681,7 @@ class LegacyInteractiveGraph extends React.Component { } simpleValidate(rubric: PerseusInteractiveGraphRubric) { - return InteractiveGraph.validate(this.getUserInput(), rubric, this); + return InteractiveGraph.validate(this.getUserInput(), rubric); } focus: () => void = $.noop; @@ -1791,7 +1791,7 @@ class InteractiveGraph extends React.Component { } simpleValidate(rubric: PerseusInteractiveGraphRubric) { - return InteractiveGraph.validate(this.getUserInput(), rubric, this); + return InteractiveGraph.validate(this.getUserInput(), rubric); } render() { @@ -2363,7 +2363,6 @@ class InteractiveGraph extends React.Component { static validate( userInput: PerseusGraphType, rubric: PerseusInteractiveGraphRubric, - component: any, ): PerseusScore { // None-type graphs are not graded if (userInput.type === "none" && rubric.correct.type === "none") { @@ -2503,10 +2502,10 @@ class InteractiveGraph extends React.Component { rubric.correct.type === "point" && userInput.coords != null ) { - let correct = InteractiveGraph.getPointCoords( - rubric.correct, - component, - ); + let correct = rubric.correct.coords; + if (correct == null) { + throw new Error("Point graph rubric has null coords"); + } const guess = userInput.coords.slice(); correct = correct.slice(); // Everything's already rounded so we shouldn't need to do an From c41e4b2f35cd2036778c79e035d402dee7f12893 Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Fri, 27 Sep 2024 15:15:38 -0700 Subject: [PATCH 08/14] Reimplement mafsStateToInteractiveGraph (#1664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change was previously implemented in PR #1654, then reverted in #1663 due to a bug. This PR reimplements the change without the bug. The original PR summary follows. --- Previously, we were passing the entire Mafs graph state to onChange. Since the onChange callback data is used by the Interactive Graph widget editor to set the correct answer for the graph, a bunch of extra data like "hasBeenInteractedWith": true was getting saved in the correct field of the Widget data. This extra data is not used, so its presence in datastore is potentially confusing to future maintainers trying to understand the data. It might also cause bugs in the future - e.g. if some code merges it into another object in which those properties are meaningful. This PR ensures that we only save relevant data in the correct field of interactive graph widgets. Issue: none ## Test plan: `yarn dev; open http://localhost:5173` Play around with the graphs on the dev UI gallery page. In particular: - Unlimited point - Polygons with angle and side labels - Polygons with side snapping You should not observe any issues. Edit, save and publish an interactive graph widget in an exercise. As a learner, you should be able to complete the exercise successfully. Author: benchristel Reviewers: jeremywiebe, nicolecomputer Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1664 --- .changeset/eleven-elephants-trade.md | 5 + packages/perseus/src/types.ts | 4 +- .../perseus/src/widgets/interactive-graph.tsx | 1 - .../mafs-state-to-interactive-graph.test.ts | 418 ++++++++++++++++++ .../mafs-state-to-interactive-graph.ts | 97 ++++ .../stateful-mafs-graph.tsx | 26 +- 6 files changed, 526 insertions(+), 25 deletions(-) create mode 100644 .changeset/eleven-elephants-trade.md create mode 100644 packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts create mode 100644 packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.ts diff --git a/.changeset/eleven-elephants-trade.md b/.changeset/eleven-elephants-trade.md new file mode 100644 index 0000000000..9e79c16ece --- /dev/null +++ b/.changeset/eleven-elephants-trade.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Omit unused data from interactive graph onChange callback diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index effe982d52..764c9438de 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -3,12 +3,12 @@ import type {Item} from "./multi-items/item-types"; import type { Hint, PerseusAnswerArea, + PerseusGraphType, PerseusWidget, PerseusWidgetsMap, } from "./perseus-types"; import type {PerseusStrings} from "./strings"; import type {SizeClass} from "./util/sizing-utils"; -import type {InteractiveGraphState} from "./widgets/interactive-graphs/types"; import type {KeypadAPI} from "@khanacademy/math-input"; import type {AnalyticsEventHandlerFn} from "@khanacademy/perseus-core"; import type {LinterContextProps} from "@khanacademy/perseus-linter"; @@ -90,7 +90,7 @@ export type ChangeHandler = ( // perseus-all-package/widgets/grapher.jsx plot?: any; // Interactive Graph callback (see legacy: interactive-graph.tsx) - graph?: InteractiveGraphState; + graph?: PerseusGraphType; }, callback?: () => unknown | null | undefined, silent?: boolean, diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index 962f64b8e4..c7b3634553 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -1655,7 +1655,6 @@ class LegacyInteractiveGraph extends React.Component { $(this.angle).on("move", () => { this.onChange({ - // @ts-expect-error Type '{ coords: any; type: "angle"; showAngles?: boolean | undefined; allowReflexAngles?: boolean | undefined; angleOffsetDeg?: number | undefined; snapDegrees?: number | undefined; match?: "congruent" | undefined; }' is not assignable to type 'InteractiveGraphState | undefined'. graph: {...graph, coords: this.angle?.getClockwiseCoords()}, }); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts b/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts new file mode 100644 index 0000000000..d99412ea1b --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts @@ -0,0 +1,418 @@ +import {mafsStateToInteractiveGraph} from "./mafs-state-to-interactive-graph"; + +import type { + AngleGraphState, + CircleGraphState, + InteractiveGraphStateCommon, + LinearGraphState, + LinearSystemGraphState, + NoneGraphState, + PointGraphState, + PolygonGraphState, + QuadraticGraphState, + RayGraphState, + SegmentGraphState, + SinusoidGraphState, +} from "./types"; +import type {PerseusGraphType} from "../../perseus-types"; + +const commonGraphState: InteractiveGraphStateCommon = { + hasBeenInteractedWith: true, + range: [ + [-9, 9], + [-9, 9], + ], + snapStep: [9, 9], +}; + +describe("mafsStateToInteractiveGraph", () => { + it("converts the state of an angle graph", () => { + const graph: PerseusGraphType = { + type: "angle", + match: "congruent", + }; + const state: AngleGraphState = { + ...commonGraphState, + type: "angle", + showAngles: true, + allowReflexAngles: true, + angleOffsetDeg: 7, + snapDegrees: 8, + coords: [ + [9, 10], + [11, 12], + [13, 14], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "angle", + match: "congruent", + coords: [ + [9, 10], + [11, 12], + [13, 14], + ], + }); + }); + + it("converts the state of a circle graph", () => { + const graph: PerseusGraphType = { + type: "circle", + startCoords: { + radius: 3, + center: [4, 5], + }, + }; + const state: CircleGraphState = { + type: "circle", + center: [1, 2], + radiusPoint: [3, 2], + hasBeenInteractedWith: true, + range: [ + [-9, 9], + [-9, 9], + ], + snapStep: [9, 9], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "circle", + radius: 2, + center: [1, 2], + startCoords: { + radius: 3, + center: [4, 5], + }, + }); + }); + + it("converts the state of a segment graph", () => { + const graph: PerseusGraphType = { + type: "segment", + numSegments: 7, + }; + const state: SegmentGraphState = { + ...commonGraphState, + type: "segment", + coords: [ + [ + [1, 2], + [3, 4], + ], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "segment", + numSegments: 7, + coords: [ + [ + [1, 2], + [3, 4], + ], + ], + }); + }); + + it("converts the state of a linear graph", () => { + const graph: PerseusGraphType = { + type: "linear", + startCoords: [ + [5, 6], + [7, 8], + ], + }; + const state: LinearGraphState = { + ...commonGraphState, + type: "linear", + coords: [ + [1, 2], + [3, 4], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "linear", + coords: [ + [1, 2], + [3, 4], + ], + startCoords: [ + [5, 6], + [7, 8], + ], + }); + }); + + it("converts the state of a linear-system graph", () => { + const graph: PerseusGraphType = { + type: "linear-system", + startCoords: [ + [ + [9, 10], + [11, 12], + ], + ], + }; + const state: LinearSystemGraphState = { + ...commonGraphState, + type: "linear-system", + coords: [ + [ + [1, 2], + [3, 4], + ], + [ + [5, 6], + [7, 8], + ], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "linear-system", + coords: [ + [ + [1, 2], + [3, 4], + ], + [ + [5, 6], + [7, 8], + ], + ], + startCoords: [ + [ + [9, 10], + [11, 12], + ], + ], + }); + }); + + it("converts the state of a ray graph", () => { + const graph: PerseusGraphType = { + type: "ray", + startCoords: [ + [5, 6], + [7, 8], + ], + }; + const state: RayGraphState = { + ...commonGraphState, + type: "ray", + coords: [ + [1, 2], + [3, 4], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "ray", + coords: [ + [1, 2], + [3, 4], + ], + startCoords: [ + [5, 6], + [7, 8], + ], + }); + }); + + it("converts the state of a polygon graph", () => { + const graph: PerseusGraphType = { + type: "polygon", + match: "approx", + }; + const state: PolygonGraphState = { + ...commonGraphState, + type: "polygon", + showAngles: true, + showSides: true, + snapTo: "sides", + coords: [ + [1, 2], + [3, 4], + [5, 6], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "polygon", + match: "approx", + coords: [ + [1, 2], + [3, 4], + [5, 6], + ], + }); + }); + + it("converts the state of a point graph", () => { + const graph: PerseusGraphType = { + type: "point", + numPoints: "unlimited", + startCoords: [[7, 8]], + }; + const state: PointGraphState = { + ...commonGraphState, + type: "point", + numPoints: "unlimited", + focusedPointIndex: 99, + showRemovePointButton: true, + showKeyboardInteractionInvitation: true, + interactionMode: "mouse", + coords: [ + [1, 2], + [3, 4], + [5, 6], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "point", + numPoints: "unlimited", + coords: [ + [1, 2], + [3, 4], + [5, 6], + ], + startCoords: [[7, 8]], + }); + }); + + it("converts the state of a quadratic graph", () => { + const graph: PerseusGraphType = { + type: "quadratic", + startCoords: [ + [7, 8], + [9, 10], + [11, 12], + ], + }; + const state: QuadraticGraphState = { + ...commonGraphState, + type: "quadratic", + coords: [ + [1, 2], + [3, 4], + [5, 6], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "quadratic", + coords: [ + [1, 2], + [3, 4], + [5, 6], + ], + startCoords: [ + [7, 8], + [9, 10], + [11, 12], + ], + }); + }); + + it("converts the state of a sinusoid graph", () => { + const graph: PerseusGraphType = { + type: "sinusoid", + startCoords: [ + [5, 6], + [7, 8], + ], + }; + const state: SinusoidGraphState = { + ...commonGraphState, + type: "sinusoid", + coords: [ + [1, 2], + [3, 4], + ], + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "sinusoid", + coords: [ + [1, 2], + [3, 4], + ], + startCoords: [ + [5, 6], + [7, 8], + ], + }); + }); + + it("converts the state of a none-type graph", () => { + const graph: PerseusGraphType = { + type: "none", + }; + const state: NoneGraphState = { + ...commonGraphState, + type: "none", + }; + + const result: PerseusGraphType = mafsStateToInteractiveGraph( + state, + graph, + ); + + expect(result).toEqual({ + type: "none", + }); + }); +}); diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.ts b/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.ts new file mode 100644 index 0000000000..df7bf95806 --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.ts @@ -0,0 +1,97 @@ +import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core"; +import invariant from "tiny-invariant"; + +import {getRadius} from "./reducer/interactive-graph-state"; + +import type {InteractiveGraphState} from "./types"; +import type {PerseusGraphType} from "@khanacademy/perseus"; + +// Converts the state of a StatefulMafsGraph back to the format used to +// represent graph state in the widget JSON. +// +// Rather than be tightly bound to how data was structured in +// the legacy interactive graph, this lets us store state +// however we want and we just transform it before handing it off +// the the parent InteractiveGraph. +// +// The transformed data is used in the interactive graph widget editor, to +// set the `correct` field of the graph options. In the learner-facing UI, the +// data is also stored by the Renderer, and passed back down to the graph +// widget via the `graph` prop. Because the data returned by this function +// completely replaces the Renderer's representation of the widget, rather than +// being merged into it, we take it upon ourselves to merge in the original +// widget data here. If we didn't do this merging, the graph's configuration +// would reset to the defaults when the learner interacted with it. +export function mafsStateToInteractiveGraph( + state: InteractiveGraphState, + originalGraph: PerseusGraphType, +): PerseusGraphType { + switch (state.type) { + case "angle": + invariant(originalGraph.type === "angle"); + return { + ...originalGraph, + coords: state.coords, + }; + case "quadratic": + invariant(originalGraph.type === "quadratic"); + return { + ...originalGraph, + coords: state.coords, + }; + case "circle": + invariant(originalGraph.type === "circle"); + return { + ...originalGraph, + center: state.center, + radius: getRadius(state), + }; + case "linear": + invariant(originalGraph.type === "linear"); + return { + ...originalGraph, + coords: state.coords, + }; + case "ray": + invariant(originalGraph.type === "ray"); + return { + ...originalGraph, + coords: state.coords, + }; + case "sinusoid": + invariant(originalGraph.type === "sinusoid"); + return { + ...originalGraph, + coords: state.coords, + }; + case "segment": + invariant(originalGraph.type === "segment"); + return { + ...originalGraph, + coords: state.coords, + }; + case "linear-system": + invariant(originalGraph.type === "linear-system"); + return { + ...originalGraph, + coords: state.coords, + }; + case "polygon": + invariant(originalGraph.type === "polygon"); + return { + ...originalGraph, + coords: state.coords, + }; + case "point": + invariant(originalGraph.type === "point"); + return { + ...originalGraph, + coords: state.coords, + }; + case "none": + invariant(originalGraph.type === "none"); + return {...originalGraph}; + default: + throw new UnreachableCaseError(state); + } +} diff --git a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx index 148f2277cb..446305e0dd 100644 --- a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import {useEffect, useImperativeHandle, useRef} from "react"; import {MafsGraph} from "./mafs-graph"; +import {mafsStateToInteractiveGraph} from "./mafs-state-to-interactive-graph"; import {initializeGraphState} from "./reducer/initialize-graph-state"; import { changeRange, @@ -10,7 +11,7 @@ import { reinitialize, } from "./reducer/interactive-graph-action"; import {interactiveGraphReducer} from "./reducer/interactive-graph-reducer"; -import {getGradableGraph, getRadius} from "./reducer/interactive-graph-state"; +import {getGradableGraph} from "./reducer/interactive-graph-state"; import type {InteractiveGraphProps, InteractiveGraphState} from "./types"; import type {PerseusGraphType} from "../../perseus-types"; @@ -45,25 +46,6 @@ export type StatefulMafsGraphType = { getUserInput: () => PerseusInteractiveGraphUserInput; }; -// Rather than be tightly bound to how data was structured in -// the legacy interactive graph, this lets us store state -// however we want and we just transform it before handing it off -// the the parent InteractiveGraph -function mafsStateToInteractiveGraph(state: {graph: InteractiveGraphState}) { - if (state.graph.type === "circle") { - return { - ...state, - graph: { - ...state.graph, - radius: getRadius(state.graph), - }, - }; - } - return { - ...state, - }; -} - export const StatefulMafsGraph = React.forwardRef< StatefulMafsGraphType, StatefulMafsGraphProps @@ -84,10 +66,10 @@ export const StatefulMafsGraph = React.forwardRef< useEffect(() => { if (prevState.current !== state) { - onChange(mafsStateToInteractiveGraph({graph: state})); + onChange({graph: mafsStateToInteractiveGraph(state, graph)}); } prevState.current = state; - }, [onChange, state]); + }, [onChange, state, graph]); // Destructuring first to keep useEffect from making excess calls const [xSnap, ySnap] = props.snapStep; From 3c73f4aa40b4912affa7ec3a71ce21a9cbf11af5 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Fri, 27 Sep 2024 15:47:05 -0700 Subject: [PATCH 09/14] [Locked Figure Aria] Implement locked point aria label behavior on graph (#1677) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Add `locked-figures-aria` flag - Add `ariaLabel` field to LockedPointType - Add the aria label behavior to the LockedPoint on the mafs graph - Minor updates to the screen reader behvaior Issue: https://khanacademy.atlassian.net/browse/LEMS-2375 ## Test plan: `yarn jest packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx` Storybook - Go to http://localhost:6006/iframe.html?args=&id=perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags&viewMode=story - Use a screen reader to navigate through the preview graph - Using tab should _only_ navigate to the interactive elements, NOT the locked point - Using the screenreader controls (VO + arrow keys) should be able to navigate to the locked point. It should read out as "Point A". https://github.com/user-attachments/assets/939ae929-8569-410b-a25b-8b09e16ec97b Author: nishasy Reviewers: catandthemachines, nishasy, #perseus, benchristel, mark-fitzgerald, anakaren-rojas Required Reviewers: Approved By: catandthemachines 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), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1677 --- .changeset/hot-geese-look.md | 6 +++ .../src/__stories__/flags-for-api-options.ts | 1 + .../interactive-graph-editor.stories.tsx | 1 + packages/perseus/src/perseus-types.ts | 1 + packages/perseus/src/types.ts | 5 ++ .../backgrounds/axis-ticks.tsx | 4 +- .../interactive-graphs/graph-locked-layer.tsx | 10 +++- ...interactive-graph-question-builder.test.ts | 2 + .../interactive-graph-question-builder.ts | 3 ++ .../interactive-graph.test.tsx | 46 +++++++++++++++++++ .../interactive-graph.testdata.ts | 2 +- .../locked-figures/locked-label.tsx | 1 + .../locked-figures/locked-point.tsx | 43 +++++++++++------ .../widgets/interactive-graphs/mafs-graph.tsx | 1 + 14 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 .changeset/hot-geese-look.md diff --git a/.changeset/hot-geese-look.md b/.changeset/hot-geese-look.md new file mode 100644 index 0000000000..b6974ef51d --- /dev/null +++ b/.changeset/hot-geese-look.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Locked Figure Aria] Implement locked point aria label behavior on graph diff --git a/packages/perseus-editor/src/__stories__/flags-for-api-options.ts b/packages/perseus-editor/src/__stories__/flags-for-api-options.ts index a371423c34..05c64ba39e 100644 --- a/packages/perseus-editor/src/__stories__/flags-for-api-options.ts +++ b/packages/perseus-editor/src/__stories__/flags-for-api-options.ts @@ -16,6 +16,7 @@ export const flags = { // Locked figures flags "interactive-graph-locked-features-labels": true, + "locked-figures-aria": true, "locked-point-labels": true, "locked-line-labels": true, "locked-vector-labels": true, diff --git a/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx b/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx index faec15bca0..aa6838b6cf 100644 --- a/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx +++ b/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx @@ -153,6 +153,7 @@ export const MafsWithLockedFiguresCurrent = (): React.ReactElement => { mafs: { ...flags.mafs, "interactive-graph-locked-features-labels": false, + "locked-figures-aria": false, "locked-point-labels": false, "locked-line-labels": false, "locked-vector-labels": false, diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index f4ab252168..de7930fc6e 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -707,6 +707,7 @@ export type LockedPointType = { color: LockedFigureColor; filled: boolean; labels?: LockedLabelType[]; + ariaLabel?: string; }; export type LockedLineType = { diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index 764c9438de..ecdbb54a08 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -154,6 +154,11 @@ export const InteractiveGraphLockedFeaturesFlags = [ * widget (locked labels). */ "interactive-graph-locked-features-labels", + /** + * Enables/disables the aria labels associated with specific locked + * figures in the updated Interactive Graph widget. + */ + "locked-figures-aria", /** * Enables/disables the labels associated with locked points in the diff --git a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx index e54450897e..0e12c55072 100644 --- a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx @@ -48,7 +48,7 @@ const YGridTick = ({y, range}: {y: number; range: [Interval, Interval]}) => { const showLabel = shouldShowLabel(y, range); return ( - + {showLabel && ( { const yPositionText = yPosition + yAdjustment; return ( - + { ; range: [x: Interval, y: Interval]; }; const GraphLockedLayer = (props: Props) => { - const {lockedFigures} = props; + const {flags, lockedFigures} = props; return ( <> {lockedFigures.map((figure, index) => { switch (figure.type) { case "point": return ( - + ); case "line": return ( diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts index 3f8b777631..372a7c4a0c 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts @@ -771,6 +771,7 @@ describe("InteractiveGraphQuestionBuilder", () => { color: "green", filled: false, labels: [{text: "a label"}], + ariaLabel: "an aria label", }) .build(); const graph = question.widgets["interactive-graph 1"]; @@ -790,6 +791,7 @@ describe("InteractiveGraphQuestionBuilder", () => { size: "medium", }, ], + ariaLabel: "an aria label", }, ]); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts index 2939139214..07aff6e25f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts @@ -298,6 +298,7 @@ class InteractiveGraphQuestionBuilder { color?: LockedFigureColor; filled?: boolean; labels?: LockedFigureLabelOptions[]; + ariaLabel?: string; }, ): InteractiveGraphQuestionBuilder { this.addLockedFigure(this.createLockedPoint(x, y, options)); @@ -482,6 +483,7 @@ class InteractiveGraphQuestionBuilder { color?: LockedFigureColor; filled?: boolean; labels?: LockedFigureLabelOptions[]; + ariaLabel?: string; }, ): LockedPointType { return { @@ -496,6 +498,7 @@ class InteractiveGraphQuestionBuilder { color: options?.color ?? "grayH", size: label.size ?? "medium", })), + ariaLabel: options?.ariaLabel, }; } diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx index e8e667a3d5..5653d22331 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx @@ -542,6 +542,52 @@ describe("locked layer", () => { }); }); + it("should render locked point with aria label when one is provided", () => { + // Arrange + const lockedPointWithAriaLabelQuestion = + interactiveGraphQuestionBuilder() + .addLockedPointAt(0, 0, { + ariaLabel: "Point A", + }) + .build(); + const {container} = renderQuestion(lockedPointWithAriaLabelQuestion, { + flags: { + mafs: { + segment: true, + "locked-figures-aria": true, + }, + }, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const point = container.querySelector(".locked-point"); + + // Assert + expect(point).toHaveAttribute("aria-label", "Point A"); + }); + + it("should render locked points without aria label by default", () => { + // Arrange + const simpleLockedPointQuestion = interactiveGraphQuestionBuilder() + .addLockedPointAt(0, 0) + .build(); + const {container} = renderQuestion(simpleLockedPointQuestion, { + flags: { + mafs: { + segment: true, + }, + }, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const point = container.querySelector(".locked-point"); + + // Assert + expect(point).not.toHaveAttribute("aria-label"); + }); + it("should render locked lines", () => { // Arrange const {container} = renderQuestion(segmentWithLockedLineQuestion, { diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts index 11466c9b4a..c98b171003 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts @@ -830,7 +830,7 @@ export const segmentWithLockedLabels: PerseusRenderer = export const segmentWithLockedFigures: PerseusRenderer = interactiveGraphQuestionBuilder() - .addLockedPointAt(-7, -7, {labels: [{text: "A"}]}) + .addLockedPointAt(-7, -7, {labels: [{text: "A"}], ariaLabel: "Point A"}) .addLockedLine([-7, -5], [2, -3], { showPoint1: true, showPoint2: true, diff --git a/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-label.tsx b/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-label.tsx index b84469a3fc..d3853f6e30 100644 --- a/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-label.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-label.tsx @@ -25,6 +25,7 @@ export default function LockedLabel(props: LockedLabelType) { fontSize: font.size[size], backgroundColor: "rgba(255, 255, 255, 0.8)", }} + aria-hidden={true} > {text} diff --git a/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-point.tsx b/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-point.tsx index c7c1046dbe..fd854d67b3 100644 --- a/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-point.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-point.tsx @@ -4,21 +4,38 @@ import * as React from "react"; import {lockedFigureColors, type LockedPointType} from "../../../perseus-types"; -const LockedPoint = (props: LockedPointType) => { - const {color, coord, filled} = props; +import type {APIOptions} from "../../../types"; + +type Props = LockedPointType & { + flags?: APIOptions["flags"]; +}; + +const LockedPoint = (props: Props) => { + const {flags, color, coord, filled, ariaLabel} = props; const [x, y] = coord; + + const hasAria = ariaLabel && flags?.["mafs"]?.["locked-figures-aria"]; + return ( - + + + ); }; diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx index 7bf493c5a3..dbd6ca3491 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx @@ -230,6 +230,7 @@ export const MafsGraph = (props: MafsGraphProps) => { {/* Locked figures layer */} {props.lockedFigures && ( From 764b9ecad0928d70e97998e8962037912524ed8e Mon Sep 17 00:00:00 2001 From: Khan Actions Bot <56267880+khan-actions-bot@users.noreply.github.com> Date: Fri, 27 Sep 2024 19:57:04 -0400 Subject: [PATCH 10/14] Update browserslist (#1623) 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: somewhatabstract, jeresig, #perseus Required Reviewers: Approved By: somewhatabstract, jeresig Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ 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), ✅ Jest Coverage (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), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1623 --- yarn.lock | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index 1a6d4443d8..f21ec054e3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6146,9 +6146,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: - version "1.0.30001659" - resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001659.tgz" - integrity sha512-Qxxyfv3RdHAfJcXelgf0hU4DFUVXBGTjqrBUZLUh8AtlGnsDo+CnncYtTd95+ZKfnANUOzxyIQCuU/UeBZBYoA== + version "1.0.30001663" + resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001663.tgz" + integrity sha512-o9C3X27GLKbLeTYZ6HBOLU1tsAcBZsLis28wrVzddShCS16RujjHp9GDHKZqrB3meE0YjhawvMFsGb/igqiPzA== caseless@~0.12.0: version "0.12.0" @@ -15078,7 +15078,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0": version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -15096,6 +15096,15 @@ string-width@^1.0.1: is-fullwidth-code-point "^1.0.0" strip-ansi "^3.0.0" +"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + string-width@^5.0.1, string-width@^5.1.2: version "5.1.2" resolved "https://registry.yarnpkg.com/string-width/-/string-width-5.1.2.tgz#14f8daec6d81e7221d2a357e668cab73bdbca794" @@ -15180,7 +15189,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -15194,6 +15203,13 @@ strip-ansi@^3.0.0, strip-ansi@^3.0.1: dependencies: ansi-regex "^2.0.0" +strip-ansi@^6.0.0, strip-ansi@^6.0.1: + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + strip-ansi@^7.0.1: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -16412,7 +16428,7 @@ wordwrap@^1.0.0: resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-1.0.0.tgz#27584810891456a4171c8d0226441ade90cbcaeb" integrity sha512-gvVzJFlPycKc5dZN4yPkP8w7Dc37BtP1yczEneOb4uq34pXZcvrtRTmWV8W+Ume+XCxKgbjM+nevkyFPMybd4Q== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -16430,6 +16446,15 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" +wrap-ansi@^7.0.0: + version "7.0.0" + resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" + integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== + dependencies: + ansi-styles "^4.0.0" + string-width "^4.1.0" + strip-ansi "^6.0.0" + wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 493715e3d9a8892ae6f7e052b830e4d88367cd19 Mon Sep 17 00:00:00 2001 From: Matthew Date: Mon, 30 Sep 2024 13:37:09 -0500 Subject: [PATCH 11/14] Split out InteractiveGraph validator (#1700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Split out the validation logic from the InteractiveGraph widget Issue: LEMS-2356 ## Test plan: Nothing should change, mostly just moving code around Author: handeyeco Reviewers: benchristel, handeyeco Required Reviewers: Approved By: benchristel Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1700 --- .changeset/nervous-otters-fly.md | 5 + .../src/widgets/interactive-graph.test.tsx | 301 +------------ .../perseus/src/widgets/interactive-graph.tsx | 397 +++--------------- .../interactive-graph-validator.test.ts | 299 +++++++++++++ .../interactive-graph-validator.ts | 313 ++++++++++++++ 5 files changed, 675 insertions(+), 640 deletions(-) create mode 100644 .changeset/nervous-otters-fly.md create mode 100644 packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.test.ts create mode 100644 packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.ts diff --git a/.changeset/nervous-otters-fly.md b/.changeset/nervous-otters-fly.md new file mode 100644 index 0000000000..818680d6ad --- /dev/null +++ b/.changeset/nervous-otters-fly.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Split out InteractiveGraph validator diff --git a/packages/perseus/src/widgets/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graph.test.tsx index aba4dae7b1..8185210ae0 100644 --- a/packages/perseus/src/widgets/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graph.test.tsx @@ -1,310 +1,11 @@ -import invariant from "tiny-invariant"; - -import {clone} from "../../../../testing/object-utils"; - -import InteractiveGraph, {shouldUseMafs} from "./interactive-graph"; +import {shouldUseMafs} from "./interactive-graph"; import type { PerseusGraphTypeLinear, PerseusGraphTypePoint, PerseusGraphTypePolygon, - PerseusGraphType, PerseusGraphTypeNone, } from "../perseus-types"; -import type {PerseusInteractiveGraphRubric} from "../validation.types"; - -function createRubric(graph: PerseusGraphType): PerseusInteractiveGraphRubric { - return {graph, correct: graph}; -} - -describe("InteractiveGraph.validate on a segment question", () => { - it("marks the answer invalid if guess.coords is missing", () => { - const guess: PerseusGraphType = {type: "segment"}; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "segment", - coords: [ - [ - [0, 0], - [1, 1], - ], - ], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveInvalidInput(); - }); - - it("does not award points if guess.coords is wrong", () => { - const guess: PerseusGraphType = { - type: "segment", - coords: [ - [ - [99, 0], - [1, 1], - ], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "segment", - coords: [ - [ - [0, 0], - [1, 1], - ], - ], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveBeenAnsweredIncorrectly(); - }); - - it("awards points if guess.coords is right", () => { - const guess: PerseusGraphType = { - type: "segment", - coords: [ - [ - [0, 0], - [1, 1], - ], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "segment", - coords: [ - [ - [0, 0], - [1, 1], - ], - ], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveBeenAnsweredCorrectly(); - }); - - it("allows points of a segment to be specified in reverse order", () => { - const guess: PerseusGraphType = { - type: "segment", - coords: [ - [ - [1, 1], - [0, 0], - ], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "segment", - coords: [ - [ - [0, 0], - [1, 1], - ], - ], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveBeenAnsweredCorrectly(); - }); - - it("does not modify the `guess` data", () => { - const guess: PerseusGraphType = { - type: "segment", - coords: [ - [ - [1, 1], - [0, 0], - ], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "segment", - coords: [ - [ - [0, 0], - [1, 1], - ], - ], - }); - - InteractiveGraph.widget.validate(guess, rubric); - - expect(guess.coords).toEqual([ - [ - [1, 1], - [0, 0], - ], - ]); - }); - - it("does not modify the `rubric` data", () => { - const guess: PerseusGraphType = { - type: "segment", - coords: [ - [ - [1, 1], - [0, 0], - ], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "segment", - coords: [ - [ - [1, 1], - [0, 0], - ], - ], - }); - - InteractiveGraph.widget.validate(guess, rubric); - - // Narrow the type of `rubric.correct` to segment graph; otherwise TS - // thinks it might not have a `coords` property. - invariant(rubric.correct.type === "segment"); - expect(rubric.correct.coords).toEqual([ - [ - [1, 1], - [0, 0], - ], - ]); - }); -}); - -describe("InteractiveGraph.validate on a point question", () => { - it("marks the answer invalid if guess.coords is missing", () => { - const guess: PerseusGraphType = {type: "point"}; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - coords: [[0, 0]], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveInvalidInput(); - }); - - it("throws an exception if correct.coords is missing", () => { - // Characterization test: this might not be desirable behavior, but - // it's the current behavior as of 2024-09-25. - const guess: PerseusGraphType = { - type: "point", - coords: [[0, 0]], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - }); - - expect(() => - InteractiveGraph.widget.validate(guess, rubric), - ).toThrowError(); - }); - - it("does not award points if guess.coords is wrong", () => { - const guess: PerseusGraphType = { - type: "point", - coords: [[9, 9]], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - coords: [[0, 0]], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveBeenAnsweredIncorrectly(); - }); - - it("awards points if guess.coords is right", () => { - const guess: PerseusGraphType = { - type: "point", - coords: [[7, 8]], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - coords: [[7, 8]], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toEqual({ - type: "points", - earned: 1, - total: 1, - message: null, - }); - }); - - it("allows points to be specified in any order", () => { - const guess: PerseusGraphType = { - type: "point", - coords: [ - [7, 8], - [5, 6], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - coords: [ - [5, 6], - [7, 8], - ], - }); - - const result = InteractiveGraph.widget.validate(guess, rubric); - - expect(result).toHaveBeenAnsweredCorrectly(); - }); - - it("does not modify the `guess` data", () => { - const guess: PerseusGraphType = { - type: "point", - coords: [ - [7, 8], - [5, 6], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - coords: [ - [5, 6], - [7, 8], - ], - }); - - const guessClone = clone(guess); - - InteractiveGraph.widget.validate(guess, rubric); - - expect(guess).toEqual(guessClone); - }); - - it("does not modify the `rubric` data", () => { - const guess: PerseusGraphType = { - type: "point", - coords: [ - [7, 8], - [5, 6], - ], - }; - const rubric: PerseusInteractiveGraphRubric = createRubric({ - type: "point", - coords: [ - [5, 6], - [7, 8], - ], - }); - - const rubricClone = clone(rubric); - - InteractiveGraph.widget.validate(guess, rubric); - - expect(rubric).toEqual(rubricClone); - }); -}); describe("shouldUseMafs", () => { it("is false given no mafs flags", () => { diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index c7b3634553..8886012c28 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -14,7 +14,6 @@ import Util from "../util"; import KhanColors from "../util/colors"; import { angleMeasures, - canonicalSineCoefficients, ccw, collinear, getLineEquation, @@ -24,7 +23,6 @@ import { magnitude, rotate, sign, - similar, vector, } from "../util/geometry"; import GraphUtils from "../util/graph-utils"; @@ -32,6 +30,7 @@ import {polar} from "../util/graphie"; import {getInteractiveBoxFromSizeClass} from "../util/sizing-utils"; import {StatefulMafsGraph} from "./interactive-graphs"; +import interactiveGraphValidator from "./interactive-graphs/interactive-graph-validator"; import type {StatefulMafsGraphType} from "./interactive-graphs/stateful-mafs-graph"; import type {QuadraticGraphState} from "./interactive-graphs/types"; @@ -68,7 +67,6 @@ const defaultBackgroundImage = { }; const eq = Util.eq; -const deepEq = Util.deepEq; const UNLIMITED = "unlimited" as const; @@ -135,6 +133,57 @@ type DefaultProps = { graph: Props["graph"]; }; +// TODO: there's another, very similar getSinusoidCoefficients function +// they should probably be merged +export function getSinusoidCoefficients( + coords: ReadonlyArray, +): SineCoefficient { + // It's assumed that p1 is the root and p2 is the first peak + const p1 = coords[0]; + const p2 = coords[1]; + + // Resulting coefficients are canonical for this sine curve + const amplitude = p2[1] - p1[1]; + const angularFrequency = Math.PI / (2 * (p2[0] - p1[0])); + const phase = p1[0] * angularFrequency; + const verticalOffset = p1[1]; + + return [amplitude, angularFrequency, phase, verticalOffset]; +} + +// TODO: there's another, very similar getQuadraticCoefficients function +// they should probably be merged +export function getQuadraticCoefficients( + coords: ReadonlyArray, +): QuadraticCoefficient { + const p1 = coords[0]; + const p2 = coords[1]; + const p3 = coords[2]; + + const denom = (p1[0] - p2[0]) * (p1[0] - p3[0]) * (p2[0] - p3[0]); + if (denom === 0) { + // Many of the callers assume that the return value is always defined. + // @ts-expect-error - TS2322 - Type 'undefined' is not assignable to type 'QuadraticCoefficient'. + return; + } + const a = + (p3[0] * (p2[1] - p1[1]) + + p2[0] * (p1[1] - p3[1]) + + p1[0] * (p3[1] - p2[1])) / + denom; + const b = + (p3[0] * p3[0] * (p1[1] - p2[1]) + + p2[0] * p2[0] * (p3[1] - p1[1]) + + p1[0] * p1[0] * (p2[1] - p3[1])) / + denom; + const c = + (p2[0] * p3[0] * (p2[0] - p3[0]) * p1[1] + + p3[0] * p1[0] * (p3[0] - p1[0]) * p2[1] + + p1[0] * p2[0] * (p1[0] - p2[0]) * p3[1]) / + denom; + return [a, b, c]; +} + // (LEMS-2190): Move the Mafs Angle Graph coordinate reversal logic in interactive-graph-state.ts // to this file when we remove the legacy graph. This logic allows us to support bi-directional angles // for the new (non-reflexive) Mafs graphs, while maintaining the same scoring behaviour as the legacy graph. @@ -1680,7 +1729,7 @@ class LegacyInteractiveGraph extends React.Component { } simpleValidate(rubric: PerseusInteractiveGraphRubric) { - return InteractiveGraph.validate(this.getUserInput(), rubric); + return interactiveGraphValidator(this.getUserInput(), rubric); } focus: () => void = $.noop; @@ -1790,7 +1839,7 @@ class InteractiveGraph extends React.Component { } simpleValidate(rubric: PerseusInteractiveGraphRubric) { - return InteractiveGraph.validate(this.getUserInput(), rubric); + return interactiveGraphValidator(this.getUserInput(), rubric); } render() { @@ -1824,53 +1873,6 @@ class InteractiveGraph extends React.Component { ); } - static getQuadraticCoefficients( - coords: ReadonlyArray, - ): QuadraticCoefficient { - const p1 = coords[0]; - const p2 = coords[1]; - const p3 = coords[2]; - - const denom = (p1[0] - p2[0]) * (p1[0] - p3[0]) * (p2[0] - p3[0]); - if (denom === 0) { - // Many of the callers assume that the return value is always defined. - // @ts-expect-error - TS2322 - Type 'undefined' is not assignable to type 'QuadraticCoefficient'. - return; - } - const a = - (p3[0] * (p2[1] - p1[1]) + - p2[0] * (p1[1] - p3[1]) + - p1[0] * (p3[1] - p2[1])) / - denom; - const b = - (p3[0] * p3[0] * (p1[1] - p2[1]) + - p2[0] * p2[0] * (p3[1] - p1[1]) + - p1[0] * p1[0] * (p2[1] - p3[1])) / - denom; - const c = - (p2[0] * p3[0] * (p2[0] - p3[0]) * p1[1] + - p3[0] * p1[0] * (p3[0] - p1[0]) * p2[1] + - p1[0] * p2[0] * (p1[0] - p2[0]) * p3[1]) / - denom; - return [a, b, c]; - } - - static getSinusoidCoefficients( - coords: ReadonlyArray, - ): SineCoefficient { - // It's assumed that p1 is the root and p2 is the first peak - const p1 = coords[0]; - const p2 = coords[1]; - - // Resulting coefficients are canonical for this sine curve - const amplitude = p2[1] - p1[1]; - const angularFrequency = Math.PI / (2 * (p2[0] - p1[0])); - const phase = p1[0] * angularFrequency; - const verticalOffset = p1[1]; - - return [amplitude, angularFrequency, phase, verticalOffset]; - } - /** * @param {object} graph Like props.graph or props.correct * @param {object} props of an InteractiveGraph instance @@ -2198,7 +2200,7 @@ class InteractiveGraph extends React.Component { // @ts-expect-error - TS2339 - Property 'coords' does not exist on type 'PerseusGraphType'. props.graph.coords || InteractiveGraph.defaultQuadraticCoords(props); - return InteractiveGraph.getQuadraticCoefficients(coords); + return getQuadraticCoefficients(coords); } static defaultQuadraticCoords(props: Props): QuadraticGraphState["coords"] { @@ -2227,7 +2229,7 @@ class InteractiveGraph extends React.Component { const coords = // @ts-expect-error - TS2339 - Property 'coords' does not exist on type 'PerseusGraphType'. props.graph.coords || InteractiveGraph.defaultSinusoidCoords(props); - return InteractiveGraph.getSinusoidCoefficients(coords); + return getSinusoidCoefficients(coords); } static defaultSinusoidCoords(props: Props): ReadonlyArray { @@ -2363,292 +2365,7 @@ class InteractiveGraph extends React.Component { userInput: PerseusGraphType, rubric: PerseusInteractiveGraphRubric, ): PerseusScore { - // None-type graphs are not graded - if (userInput.type === "none" && rubric.correct.type === "none") { - return { - type: "points", - earned: 0, - total: 0, - message: null, - }; - } - - // When nothing has moved, there will neither be coords nor the - // circle's center/radius fields. When those fields are absent, skip - // all these checks; just go mark the answer as empty. - const hasValue = Boolean( - // @ts-expect-error - TS2339 - Property 'coords' does not exist on type 'PerseusGraphType'. - userInput.coords || - // @ts-expect-error - TS2339 - Property 'center' does not exist on type 'PerseusGraphType'. | TS2339 - Property 'radius' does not exist on type 'PerseusGraphType'. - (userInput.center && userInput.radius), - ); - - if (userInput.type === rubric.correct.type && hasValue) { - if ( - userInput.type === "linear" && - rubric.correct.type === "linear" && - userInput.coords != null - ) { - const guess = userInput.coords; - const correct = rubric.correct.coords; - - // If both of the guess points are on the correct line, it's - // correct. - if ( - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. - collinear(correct[0], correct[1], guess[0]) && - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. - collinear(correct[0], correct[1], guess[1]) - ) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "linear-system" && - rubric.correct.type === "linear-system" && - userInput.coords != null - ) { - const guess = userInput.coords; - const correct = rubric.correct.coords as ReadonlyArray< - ReadonlyArray - >; - - if ( - (collinear(correct[0][0], correct[0][1], guess[0][0]) && - collinear(correct[0][0], correct[0][1], guess[0][1]) && - collinear(correct[1][0], correct[1][1], guess[1][0]) && - collinear(correct[1][0], correct[1][1], guess[1][1])) || - (collinear(correct[0][0], correct[0][1], guess[1][0]) && - collinear(correct[0][0], correct[0][1], guess[1][1]) && - collinear(correct[1][0], correct[1][1], guess[0][0]) && - collinear(correct[1][0], correct[1][1], guess[0][1])) - ) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "quadratic" && - rubric.correct.type === "quadratic" && - userInput.coords != null - ) { - // If the parabola coefficients match, it's correct. - const guessCoeffs = this.getQuadraticCoefficients( - userInput.coords, - ); - const correctCoeffs = this.getQuadraticCoefficients( - // @ts-expect-error - TS2345 - Argument of type 'readonly Coord[] | undefined' is not assignable to parameter of type 'readonly Coord[]'. - rubric.correct.coords, - ); - if (deepEq(guessCoeffs, correctCoeffs)) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "sinusoid" && - rubric.correct.type === "sinusoid" && - userInput.coords != null - ) { - const guessCoeffs = this.getSinusoidCoefficients( - userInput.coords, - ); - const correctCoeffs = this.getSinusoidCoefficients( - // @ts-expect-error - TS2345 - Argument of type 'readonly Coord[] | undefined' is not assignable to parameter of type 'readonly Coord[]'. - rubric.correct.coords, - ); - - const canonicalGuessCoeffs = - canonicalSineCoefficients(guessCoeffs); - const canonicalCorrectCoeffs = - canonicalSineCoefficients(correctCoeffs); - // If the canonical coefficients match, it's correct. - if (deepEq(canonicalGuessCoeffs, canonicalCorrectCoeffs)) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "circle" && - rubric.correct.type === "circle" - ) { - if ( - deepEq(userInput.center, rubric.correct.center) && - eq(userInput.radius, rubric.correct.radius) - ) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "point" && - rubric.correct.type === "point" && - userInput.coords != null - ) { - let correct = rubric.correct.coords; - if (correct == null) { - throw new Error("Point graph rubric has null coords"); - } - const guess = userInput.coords.slice(); - correct = correct.slice(); - // Everything's already rounded so we shouldn't need to do an - // eq() comparison but _.isEqual(0, -0) is false, so we'll use - // eq() anyway. The sort should be fine because it'll stringify - // it and -0 converted to a string is "0" - guess?.sort(); - // @ts-expect-error - TS2339 - Property 'sort' does not exist on type 'readonly Coord[]'. - correct.sort(); - if (deepEq(guess, correct)) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "polygon" && - rubric.correct.type === "polygon" && - userInput.coords != null - ) { - const guess: Array = userInput.coords?.slice(); - // @ts-expect-error - TS2322 - Type 'Coord[] | undefined' is not assignable to type 'Coord[]'. - const correct: Array = rubric.correct.coords?.slice(); - - let match; - if (rubric.correct.match === "similar") { - match = similar(guess, correct, Number.POSITIVE_INFINITY); - } else if (rubric.correct.match === "congruent") { - match = similar(guess, correct, knumber.DEFAULT_TOLERANCE); - } else if (rubric.correct.match === "approx") { - match = similar(guess, correct, 0.1); - } else { - /* exact */ - guess.sort(); - correct.sort(); - match = deepEq(guess, correct); - } - - if (match) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "segment" && - rubric.correct.type === "segment" && - userInput.coords != null - ) { - let guess = Util.deepClone(userInput.coords); - let correct = Util.deepClone(rubric.correct?.coords); - guess = _.invoke(guess, "sort").sort(); - // @ts-expect-error - TS2345 - Argument of type '(readonly Coord[])[] | undefined' is not assignable to parameter of type 'Collection'. - correct = _.invoke(correct, "sort").sort(); - if (deepEq(guess, correct)) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "ray" && - rubric.correct.type === "ray" && - userInput.coords != null - ) { - const guess = userInput.coords; - const correct = rubric.correct.coords; - if ( - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - deepEq(guess[0], correct[0]) && - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. - collinear(correct[0], correct[1], guess[1]) - ) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } else if ( - userInput.type === "angle" && - rubric.correct.type === "angle" - ) { - const guess = userInput.coords; - const correct = rubric.correct.coords; - - let match; - if (rubric.correct.match === "congruent") { - const angles = _.map([guess, correct], function (coords) { - const angle = GraphUtils.findAngle( - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - coords[2], - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - coords[0], - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. - coords[1], - ); - return (angle + 360) % 360; - }); - // @ts-expect-error - TS2556 - A spread argument must either have a tuple type or be passed to a rest parameter. - match = eq(...angles); - } else { - /* exact */ - match = // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. - deepEq(guess[1], correct[1]) && - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. - collinear(correct[1], correct[0], guess[0]) && - // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. - collinear(correct[1], correct[2], guess[2]); - } - - if (match) { - return { - type: "points", - earned: 1, - total: 1, - message: null, - }; - } - } - } - - // The input wasn't correct, so check if it's a blank input or if it's - // actually just wrong - if (!hasValue || _.isEqual(userInput, rubric.graph)) { - // We're where we started. - return { - type: "invalid", - message: null, - }; - } - return { - type: "points", - earned: 0, - total: 1, - message: null, - }; + return interactiveGraphValidator(userInput, rubric); } static getUserInputFromProps(props: Props): PerseusGraphType { diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.test.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.test.ts new file mode 100644 index 0000000000..22fb8046cc --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.test.ts @@ -0,0 +1,299 @@ +import invariant from "tiny-invariant"; + +import {clone} from "../../../../../testing/object-utils"; + +import interactiveGraphValidator from "./interactive-graph-validator"; + +import type {PerseusGraphType} from "../../perseus-types"; +import type {PerseusInteractiveGraphRubric} from "../../validation.types"; + +function createRubric(graph: PerseusGraphType): PerseusInteractiveGraphRubric { + return {graph, correct: graph}; +} + +describe("InteractiveGraph.validate on a segment question", () => { + it("marks the answer invalid if guess.coords is missing", () => { + const guess: PerseusGraphType = {type: "segment"}; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "segment", + coords: [ + [ + [0, 0], + [1, 1], + ], + ], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveInvalidInput(); + }); + + it("does not award points if guess.coords is wrong", () => { + const guess: PerseusGraphType = { + type: "segment", + coords: [ + [ + [99, 0], + [1, 1], + ], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "segment", + coords: [ + [ + [0, 0], + [1, 1], + ], + ], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveBeenAnsweredIncorrectly(); + }); + + it("awards points if guess.coords is right", () => { + const guess: PerseusGraphType = { + type: "segment", + coords: [ + [ + [0, 0], + [1, 1], + ], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "segment", + coords: [ + [ + [0, 0], + [1, 1], + ], + ], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("allows points of a segment to be specified in reverse order", () => { + const guess: PerseusGraphType = { + type: "segment", + coords: [ + [ + [1, 1], + [0, 0], + ], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "segment", + coords: [ + [ + [0, 0], + [1, 1], + ], + ], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("does not modify the `guess` data", () => { + const guess: PerseusGraphType = { + type: "segment", + coords: [ + [ + [1, 1], + [0, 0], + ], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "segment", + coords: [ + [ + [0, 0], + [1, 1], + ], + ], + }); + + interactiveGraphValidator(guess, rubric); + + expect(guess.coords).toEqual([ + [ + [1, 1], + [0, 0], + ], + ]); + }); + + it("does not modify the `rubric` data", () => { + const guess: PerseusGraphType = { + type: "segment", + coords: [ + [ + [1, 1], + [0, 0], + ], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "segment", + coords: [ + [ + [1, 1], + [0, 0], + ], + ], + }); + + interactiveGraphValidator(guess, rubric); + + // Narrow the type of `rubric.correct` to segment graph; otherwise TS + // thinks it might not have a `coords` property. + invariant(rubric.correct.type === "segment"); + expect(rubric.correct.coords).toEqual([ + [ + [1, 1], + [0, 0], + ], + ]); + }); +}); + +describe("InteractiveGraph.validate on a point question", () => { + it("marks the answer invalid if guess.coords is missing", () => { + const guess: PerseusGraphType = {type: "point"}; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [[0, 0]], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveInvalidInput(); + }); + + it("throws an exception if correct.coords is missing", () => { + // Characterization test: this might not be desirable behavior, but + // it's the current behavior as of 2024-09-25. + const guess: PerseusGraphType = { + type: "point", + coords: [[0, 0]], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + }); + + expect(() => interactiveGraphValidator(guess, rubric)).toThrowError(); + }); + + it("does not award points if guess.coords is wrong", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [[9, 9]], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [[0, 0]], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveBeenAnsweredIncorrectly(); + }); + + it("awards points if guess.coords is right", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [[7, 8]], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [[7, 8]], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toEqual({ + type: "points", + earned: 1, + total: 1, + message: null, + }); + }); + + it("allows points to be specified in any order", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [ + [7, 8], + [5, 6], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [ + [5, 6], + [7, 8], + ], + }); + + const result = interactiveGraphValidator(guess, rubric); + + expect(result).toHaveBeenAnsweredCorrectly(); + }); + + it("does not modify the `guess` data", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [ + [7, 8], + [5, 6], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [ + [5, 6], + [7, 8], + ], + }); + + const guessClone = clone(guess); + + interactiveGraphValidator(guess, rubric); + + expect(guess).toEqual(guessClone); + }); + + it("does not modify the `rubric` data", () => { + const guess: PerseusGraphType = { + type: "point", + coords: [ + [7, 8], + [5, 6], + ], + }; + const rubric: PerseusInteractiveGraphRubric = createRubric({ + type: "point", + coords: [ + [5, 6], + [7, 8], + ], + }); + + const rubricClone = clone(rubric); + + interactiveGraphValidator(guess, rubric); + + expect(rubric).toEqual(rubricClone); + }); +}); diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.ts new file mode 100644 index 0000000000..24ac5f90c9 --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-validator.ts @@ -0,0 +1,313 @@ +import {number as knumber} from "@khanacademy/kmath"; +import _ from "underscore"; + +import Util from "../../util"; +import { + canonicalSineCoefficients, + collinear, + similar, +} from "../../util/geometry"; +import GraphUtils from "../../util/graph-utils"; +import { + getQuadraticCoefficients, + getSinusoidCoefficients, +} from "../interactive-graph"; + +import type {Coord} from "../../interactive2/types"; +import type {PerseusScore} from "../../types"; +import type { + PerseusInteractiveGraphRubric, + PerseusInteractiveGraphUserInput, +} from "../../validation.types"; + +const eq = Util.eq; +const deepEq = Util.deepEq; + +function interactiveGraphValidator( + userInput: PerseusInteractiveGraphUserInput, + rubric: PerseusInteractiveGraphRubric, +): PerseusScore { + // None-type graphs are not graded + if (userInput.type === "none" && rubric.correct.type === "none") { + return { + type: "points", + earned: 0, + total: 0, + message: null, + }; + } + + // When nothing has moved, there will neither be coords nor the + // circle's center/radius fields. When those fields are absent, skip + // all these checks; just go mark the answer as empty. + const hasValue = Boolean( + // @ts-expect-error - TS2339 - Property 'coords' does not exist on type 'PerseusGraphType'. + userInput.coords || + // @ts-expect-error - TS2339 - Property 'center' does not exist on type 'PerseusGraphType'. | TS2339 - Property 'radius' does not exist on type 'PerseusGraphType'. + (userInput.center && userInput.radius), + ); + + if (userInput.type === rubric.correct.type && hasValue) { + if ( + userInput.type === "linear" && + rubric.correct.type === "linear" && + userInput.coords != null + ) { + const guess = userInput.coords; + const correct = rubric.correct.coords; + + // If both of the guess points are on the correct line, it's + // correct. + if ( + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. + collinear(correct[0], correct[1], guess[0]) && + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. + collinear(correct[0], correct[1], guess[1]) + ) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "linear-system" && + rubric.correct.type === "linear-system" && + userInput.coords != null + ) { + const guess = userInput.coords; + const correct = rubric.correct.coords as ReadonlyArray< + ReadonlyArray + >; + + if ( + (collinear(correct[0][0], correct[0][1], guess[0][0]) && + collinear(correct[0][0], correct[0][1], guess[0][1]) && + collinear(correct[1][0], correct[1][1], guess[1][0]) && + collinear(correct[1][0], correct[1][1], guess[1][1])) || + (collinear(correct[0][0], correct[0][1], guess[1][0]) && + collinear(correct[0][0], correct[0][1], guess[1][1]) && + collinear(correct[1][0], correct[1][1], guess[0][0]) && + collinear(correct[1][0], correct[1][1], guess[0][1])) + ) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "quadratic" && + rubric.correct.type === "quadratic" && + userInput.coords != null + ) { + // If the parabola coefficients match, it's correct. + const guessCoeffs = getQuadraticCoefficients(userInput.coords); + const correctCoeffs = getQuadraticCoefficients( + // @ts-expect-error - TS2345 - Argument of type 'readonly Coord[] | undefined' is not assignable to parameter of type 'readonly Coord[]'. + rubric.correct.coords, + ); + if (deepEq(guessCoeffs, correctCoeffs)) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "sinusoid" && + rubric.correct.type === "sinusoid" && + userInput.coords != null + ) { + const guessCoeffs = getSinusoidCoefficients(userInput.coords); + const correctCoeffs = getSinusoidCoefficients( + // @ts-expect-error - TS2345 - Argument of type 'readonly Coord[] | undefined' is not assignable to parameter of type 'readonly Coord[]'. + rubric.correct.coords, + ); + + const canonicalGuessCoeffs = canonicalSineCoefficients(guessCoeffs); + const canonicalCorrectCoeffs = + canonicalSineCoefficients(correctCoeffs); + // If the canonical coefficients match, it's correct. + if (deepEq(canonicalGuessCoeffs, canonicalCorrectCoeffs)) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "circle" && + rubric.correct.type === "circle" + ) { + if ( + deepEq(userInput.center, rubric.correct.center) && + eq(userInput.radius, rubric.correct.radius) + ) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "point" && + rubric.correct.type === "point" && + userInput.coords != null + ) { + let correct = rubric.correct.coords; + if (correct == null) { + throw new Error("Point graph rubric has null coords"); + } + const guess = userInput.coords.slice(); + correct = correct.slice(); + // Everything's already rounded so we shouldn't need to do an + // eq() comparison but _.isEqual(0, -0) is false, so we'll use + // eq() anyway. The sort should be fine because it'll stringify + // it and -0 converted to a string is "0" + guess?.sort(); + // @ts-expect-error - TS2339 - Property 'sort' does not exist on type 'readonly Coord[]'. + correct.sort(); + if (deepEq(guess, correct)) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "polygon" && + rubric.correct.type === "polygon" && + userInput.coords != null + ) { + const guess: Array = userInput.coords?.slice(); + // @ts-expect-error - TS2322 - Type 'Coord[] | undefined' is not assignable to type 'Coord[]'. + const correct: Array = rubric.correct.coords?.slice(); + + let match; + if (rubric.correct.match === "similar") { + match = similar(guess, correct, Number.POSITIVE_INFINITY); + } else if (rubric.correct.match === "congruent") { + match = similar(guess, correct, knumber.DEFAULT_TOLERANCE); + } else if (rubric.correct.match === "approx") { + match = similar(guess, correct, 0.1); + } else { + /* exact */ + guess.sort(); + correct.sort(); + match = deepEq(guess, correct); + } + + if (match) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "segment" && + rubric.correct.type === "segment" && + userInput.coords != null + ) { + let guess = Util.deepClone(userInput.coords); + let correct = Util.deepClone(rubric.correct?.coords); + guess = _.invoke(guess, "sort").sort(); + // @ts-expect-error - TS2345 - Argument of type '(readonly Coord[])[] | undefined' is not assignable to parameter of type 'Collection'. + correct = _.invoke(correct, "sort").sort(); + if (deepEq(guess, correct)) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "ray" && + rubric.correct.type === "ray" && + userInput.coords != null + ) { + const guess = userInput.coords; + const correct = rubric.correct.coords; + if ( + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. + deepEq(guess[0], correct[0]) && + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. + collinear(correct[0], correct[1], guess[1]) + ) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } else if ( + userInput.type === "angle" && + rubric.correct.type === "angle" + ) { + const guess = userInput.coords; + const correct = rubric.correct.coords; + + let match; + if (rubric.correct.match === "congruent") { + const angles = _.map([guess, correct], function (coords) { + const angle = GraphUtils.findAngle( + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. + coords[2], + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. + coords[0], + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. + coords[1], + ); + return (angle + 360) % 360; + }); + // @ts-expect-error - TS2556 - A spread argument must either have a tuple type or be passed to a rest parameter. + match = eq(...angles); + } else { + /* exact */ + match = // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. + deepEq(guess[1], correct[1]) && + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. + collinear(correct[1], correct[0], guess[0]) && + // @ts-expect-error - TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. | TS2532 - Object is possibly 'undefined'. + collinear(correct[1], correct[2], guess[2]); + } + + if (match) { + return { + type: "points", + earned: 1, + total: 1, + message: null, + }; + } + } + } + + // The input wasn't correct, so check if it's a blank input or if it's + // actually just wrong + if (!hasValue || _.isEqual(userInput, rubric.graph)) { + // We're where we started. + return { + type: "invalid", + message: null, + }; + } + return { + type: "points", + earned: 0, + total: 1, + message: null, + }; +} + +export default interactiveGraphValidator; From 039e0a360e56044ce2b4a1decfd82e6c01841ea9 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Mon, 30 Sep 2024 14:48:37 -0700 Subject: [PATCH 12/14] [Locked Figure Aria] Locked point aria label editor UI (#1682) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Create a new `locked-figure-aria.tsx` file that can be reused for each locked figure's settings. - Use this `locked-figure-aria.tsx` component within LockedPointSettings. - Write the function to auto-generate the locked point aria label with its coordinates and visible labels. Issue: https://khanacademy.atlassian.net/browse/LEMS-2375 ## Test plan: - `yarn jest packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.test.tsx` - `yarn jest packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.test.tsx` Storybook - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags - Confirm that the locked point aria label field is already populated with "Point A" (passed in via builder) - Confirm that pressing "Auto-generate" works with no labels, one label, and multiple labels - Confirm that updateing the aria label in the editor also updates the aria label on the point - Check the web inspector to confirm the aria-label text is updated - Use a screen reader to confirm the new label is read out image Author: nishasy Reviewers: catandthemachines, #perseus, benchristel, anakaren-rojas Required Reviewers: Approved By: catandthemachines 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), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1682 --- .changeset/strong-spoons-talk.md | 6 + .../interactive-graph-description.tsx | 2 +- .../locked-figure-aria.test.tsx | 109 +++++++++++++++ .../locked-figures/locked-figure-aria.tsx | 88 ++++++++++++ .../locked-point-settings.test.tsx | 130 ++++++++++++++++++ .../locked-figures/locked-point-settings.tsx | 56 +++++++- 6 files changed, 389 insertions(+), 2 deletions(-) create mode 100644 .changeset/strong-spoons-talk.md create mode 100644 packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.test.tsx create mode 100644 packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.tsx diff --git a/.changeset/strong-spoons-talk.md b/.changeset/strong-spoons-talk.md new file mode 100644 index 0000000000..c0ee97b4ba --- /dev/null +++ b/.changeset/strong-spoons-talk.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Locked Figure Aria] Locked point aria label editor UI diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-description.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-description.tsx index 8678c18158..34611bb0e8 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-description.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-description.tsx @@ -34,7 +34,7 @@ export default function InteractiveGraphDescription(props: Props) { Use these fields to describe the graph as a whole. These are used by screen readers to describe content to users - who are visually impaired. + who may be visually impaired. Title diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.test.tsx new file mode 100644 index 0000000000..dc5d60275d --- /dev/null +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.test.tsx @@ -0,0 +1,109 @@ +import {render, screen} from "@testing-library/react"; +import {userEvent as userEventLib} from "@testing-library/user-event"; +import * as React from "react"; + +import LockedFigureAria from "./locked-figure-aria"; + +import type {UserEvent} from "@testing-library/user-event"; + +describe("LockedFigureAria", () => { + let userEvent: UserEvent; + beforeEach(() => { + userEvent = userEventLib.setup({ + advanceTimers: jest.advanceTimersByTime, + }); + }); + + test("renders", () => { + // Arrange + + // Act + render( + {}} + />, + ); + + const titleText = screen.getByText("Aria label"); + const descriptionText = screen.getByText( + "The figure is hidden from screen readers if this field is left blank.", + ); + const input = screen.getByRole("textbox"); + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + + // Assert + expect(titleText).toBeInTheDocument(); + expect(descriptionText).toBeInTheDocument(); + expect(input).toBeInTheDocument(); + expect(input).toHaveValue(""); + expect(autoGenButton).toBeInTheDocument(); + }); + + test("renders with aria label", () => { + // Arrange + + // Act + render( + {}} + />, + ); + + const input = screen.getByRole("textbox"); + + // Assert + expect(input).toHaveValue("Point at (x, y)"); + }); + + test("auto-generate button calls onChange with the prepopulated label", async () => { + // Arrange + const onChangeProps = jest.fn(); + + // Act + render( + , + ); + + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Pre-populated aria label", + }); + }); + + test("calls onChange with undefined when input is cleared", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + ); + + // Act + const input = screen.getByRole("textbox"); + await userEvent.clear(input); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: undefined, + }); + }); +}); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.tsx new file mode 100644 index 0000000000..9d33e494bb --- /dev/null +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-figure-aria.tsx @@ -0,0 +1,88 @@ +import {components} from "@khanacademy/perseus"; +import Button from "@khanacademy/wonder-blocks-button"; +import {View} from "@khanacademy/wonder-blocks-core"; +import {LabeledTextField} from "@khanacademy/wonder-blocks-form"; +import {Spring} from "@khanacademy/wonder-blocks-layout"; +import {spacing} from "@khanacademy/wonder-blocks-tokens"; +import {LabelMedium} from "@khanacademy/wonder-blocks-typography"; +import pencilCircle from "@phosphor-icons/core/regular/pencil-circle.svg"; +import {StyleSheet} from "aphrodite"; +import * as React from "react"; + +const {InfoTip} = components; + +type Props = { + ariaLabel: string | undefined; + prePopulatedAriaLabel: string; + onChangeProps: (props: {ariaLabel?: string | undefined}) => void; +}; + +function LockedFigureAria(props: Props) { + const {ariaLabel, prePopulatedAriaLabel, onChangeProps} = props; + + return ( + + + Aria label + + + Aria label is used by screen readers to describe + content to users who may be visually impaired.{" "} +
+
+ Populating this field will make it so that users can + use a screen reader to navigate to this point and + hear the description. +
+
+ If you leave this field blank, the point will be + hidden from screen readers. Users will not be able + to navigate to this point using a screen reader. + + + } + description={`The figure is hidden from screen readers + if this field is left blank.`} + value={ariaLabel ?? ""} + onChange={(newValue) => { + onChangeProps({ + // Save as undefined if the field is empty. + ariaLabel: newValue || undefined, + }); + }} + placeholder="Ex. Point at (x, y)" + style={styles.ariaLabelTextField} + /> + + + + ); +} + +const styles = StyleSheet.create({ + row: { + flexDirection: "row", + alignItems: "center", + }, + button: { + alignSelf: "start", + }, + ariaLabelTextField: { + marginTop: spacing.xSmall_8, + }, +}); + +export default LockedFigureAria; 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 75767f18fe..5aceb30f69 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 @@ -347,4 +347,134 @@ describe("LockedPointSettings", () => { ], }); }); + + test("Renders with aria label", () => { + // Arrange + + // Act + render( + , + {wrapper: RenderStateRoot}, + ); + + const input = screen.getByRole("textbox", {name: "Aria label"}); + + // Assert + expect(input).toHaveValue("Point at (x, y)"); + }); + + test("calls onChangeProps when the aria label is updated", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const input = screen.getByRole("textbox", {name: "Aria label"}); + await userEvent.clear(input); + await userEvent.type(input, "A"); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "A", + }); + }); + + test("aria label auto-generates (no labels)", async () => { + // Arrange + const onChangeProps = jest.fn(); + + // Act + render( + , + {wrapper: RenderStateRoot}, + ); + + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Point at (0, 0)", + }); + }); + + test("aria label auto-generates (one label)", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Point at (0, 0) with label A", + }); + }); + + test("aria label auto-generates (multiple labels)", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Point at (0, 0) with labels A, B", + }); + }); }); 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 ba445d94e9..07f09eaea2 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 @@ -8,7 +8,7 @@ import Button from "@khanacademy/wonder-blocks-button"; import {View} from "@khanacademy/wonder-blocks-core"; import {Strut} from "@khanacademy/wonder-blocks-layout"; import {spacing, color as wbColor} from "@khanacademy/wonder-blocks-tokens"; -import {LabelLarge} from "@khanacademy/wonder-blocks-typography"; +import {LabelLarge, LabelMedium} from "@khanacademy/wonder-blocks-typography"; import plusCircle from "@phosphor-icons/core/regular/plus-circle.svg"; import {StyleSheet} from "aphrodite"; import * as React from "react"; @@ -19,6 +19,7 @@ import PerseusEditorAccordion from "../../../components/perseus-editor-accordion import ColorSelect from "./color-select"; import ColorSwatch from "./color-swatch"; import LabeledSwitch from "./labeled-switch"; +import LockedFigureAria from "./locked-figure-aria"; import LockedFigureSettingsActions from "./locked-figure-settings-actions"; import LockedLabelSettings from "./locked-label-settings"; import {getDefaultFigureForType} from "./util"; @@ -87,6 +88,7 @@ const LockedPointSettings = (props: Props) => { color: pointColor, filled = true, labels, + ariaLabel, onChangeProps, onMove, onRemove, @@ -99,6 +101,33 @@ const LockedPointSettings = (props: Props) => { const isDefiningPoint = !onMove && !onRemove; + /** + * Get a prepopulated aria label for the point. + * + * If the point has no labels, the aria label will just be + * "Point at (x, y)". + * + * If the point has labels, the aria label will be + * "Point at (x, y) with label1, label2, label3". + */ + function getPrepopulatedAriaLabel() { + let str = `Point at (${coord[0]}, ${coord[1]})`; + + if (labels && labels.length > 0) { + str += " with label"; + // Make it "with labels" instead of "with label" if there are + // multiple labels. + if (labels.length > 1) { + str += "s"; + } + + // Separate additional labels with commas. + str += ` ${labels.map((l) => l.text).join(", ")}`; + } + + return str; + } + function handleColorChange(newValue) { const newProps: Partial = { color: newValue, @@ -212,10 +241,31 @@ const LockedPointSettings = (props: Props) => { )} + {!isDefiningPoint && flags?.["mafs"]?.["locked-figures-aria"] && ( + <> + + + + { + onChangeProps(newProps); + }} + /> + + )} + {((!isDefiningPoint && flags?.["mafs"]?.["locked-point-labels"]) || (isDefiningPoint && flags?.["mafs"]?.["locked-line-labels"])) && ( <> + + + + + Visible labels + {labels?.map((label, labelIndex) => ( Date: Mon, 30 Sep 2024 14:57:15 -0700 Subject: [PATCH 13/14] [Locked Figure Aria] Implement locked line aria label behavior on graph (#1683) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Add `ariaLabel` field to LockedLineType - Add the aria label behavior to the LockedLine on the mafs graph Issue: https://khanacademy.atlassian.net/browse/LEMS-2376 ## Test plan: `yarn jest packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx` Storybook - Go to http://localhost:6006/iframe.html?args=&id=perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags&viewMode=story - Use a screen reader to navigate through the preview graph - Using the screenreader controls (VO + arrow keys) should be able to navigate to the locked line. It should read out as "Point PQ". https://github.com/user-attachments/assets/a6fafd5b-9d3e-4b1a-94ca-7404bb60e7ab Author: nishasy Reviewers: benchristel, catandthemachines, anakaren-rojas Required Reviewers: Approved By: benchristel Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1683 --- .changeset/fresh-sheep-invent.md | 6 +++ packages/perseus/src/perseus-types.ts | 1 + .../interactive-graphs/graph-locked-layer.tsx | 1 + ...interactive-graph-question-builder.test.ts | 2 + .../interactive-graph-question-builder.ts | 2 + .../interactive-graph.test.tsx | 46 +++++++++++++++++++ .../interactive-graph.testdata.ts | 1 + .../locked-figures/locked-line.tsx | 23 ++++++++-- 8 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 .changeset/fresh-sheep-invent.md diff --git a/.changeset/fresh-sheep-invent.md b/.changeset/fresh-sheep-invent.md new file mode 100644 index 0000000000..18a346681f --- /dev/null +++ b/.changeset/fresh-sheep-invent.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Locked Figure Aria] Implement locked line aria label behavior on graph diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index de7930fc6e..99b7dcafb8 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -719,6 +719,7 @@ export type LockedLineType = { showPoint1: boolean; showPoint2: boolean; labels?: LockedLabelType[]; + ariaLabel?: string; }; export type LockedVectorType = { diff --git a/packages/perseus/src/widgets/interactive-graphs/graph-locked-layer.tsx b/packages/perseus/src/widgets/interactive-graphs/graph-locked-layer.tsx index b883cc4216..134ceb7dd0 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graph-locked-layer.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graph-locked-layer.tsx @@ -38,6 +38,7 @@ const GraphLockedLayer = (props: Props) => { key={`line-${index}`} range={props.range} {...figure} + flags={flags} /> ); case "vector": diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts index 372a7c4a0c..ca00988816 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts @@ -867,6 +867,7 @@ describe("InteractiveGraphQuestionBuilder", () => { showPoint1: true, showPoint2: true, labels: [{text: "a label"}], + ariaLabel: "an aria label", }) .build(); const graph = question.widgets["interactive-graph 1"]; @@ -902,6 +903,7 @@ describe("InteractiveGraphQuestionBuilder", () => { size: "medium", }, ], + ariaLabel: "an aria label", }, ]); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts index 07aff6e25f..fe5b57ca50 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts @@ -316,6 +316,7 @@ class InteractiveGraphQuestionBuilder { showPoint1?: boolean; showPoint2?: boolean; labels?: LockedFigureLabelOptions[]; + ariaLabel?: string; }, ): InteractiveGraphQuestionBuilder { const line: LockedLineType = { @@ -332,6 +333,7 @@ class InteractiveGraphQuestionBuilder { color: options?.color ?? "grayH", size: label.size ?? "medium", })), + ariaLabel: options?.ariaLabel, points: [ { ...this.createLockedPoint(...point1, { diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx index 5653d22331..73c150db45 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx @@ -673,6 +673,52 @@ describe("locked layer", () => { }); }); + it("should render locked line with aria label when one is provided", () => { + // Arrange + const lockedLineWithAriaLabelQuestion = + interactiveGraphQuestionBuilder() + .addLockedLine([0, 0], [2, 2], { + ariaLabel: "Line A", + }) + .build(); + const {container} = renderQuestion(lockedLineWithAriaLabelQuestion, { + flags: { + mafs: { + segment: true, + "locked-figures-aria": true, + }, + }, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const point = container.querySelector(".locked-line"); + + // Assert + expect(point).toHaveAttribute("aria-label", "Line A"); + }); + + it("should render locked line without aria label by default", () => { + // Arrange + const simpleLockedLinequestion = interactiveGraphQuestionBuilder() + .addLockedLine([0, 0], [2, 2]) + .build(); + const {container} = renderQuestion(simpleLockedLinequestion, { + flags: { + mafs: { + segment: true, + }, + }, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const point = container.querySelector(".locked-line"); + + // Assert + expect(point).not.toHaveAttribute("aria-label"); + }); + it("should render locked vectors", async () => { // Arrange const {container} = renderQuestion(segmentWithLockedVectors, { diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts index c98b171003..09b66bd77f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts @@ -835,6 +835,7 @@ export const segmentWithLockedFigures: PerseusRenderer = showPoint1: true, showPoint2: true, labels: [{text: "B"}], + ariaLabel: "Line PQ", }) .addLockedVector([0, 0], [8, 2], { color: "purple", diff --git a/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-line.tsx b/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-line.tsx index a7d8bfceff..42114fff3b 100644 --- a/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-line.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/locked-figures/locked-line.tsx @@ -10,17 +10,30 @@ import {getIntersectionOfRayWithBox} from "../graphs/utils"; import {X, Y, calculateAngleInDegrees} from "../math"; import type {LockedLineType} from "../../../perseus-types"; +import type {APIOptions} from "../../../types"; import type {Interval} from "mafs"; type Props = LockedLineType & { + flags?: APIOptions["flags"]; range: [Interval, Interval]; }; const LockedLine = (props: Props) => { - const {color, lineStyle, kind, points, showPoint1, showPoint2, range} = - props; + const { + color, + lineStyle, + kind, + points, + showPoint1, + showPoint2, + ariaLabel, + flags, + range, + } = props; const [point1, point2] = points; + const hasAria = ariaLabel && flags?.["mafs"]?.["locked-figures-aria"]; + let line; if (kind === "ray") { @@ -101,7 +114,11 @@ const LockedLine = (props: Props) => { } return ( - + {line} {showPoint1 && ( Date: Mon, 30 Sep 2024 15:04:40 -0700 Subject: [PATCH 14/14] [Locked Figure Aria] Locked line aria label editor UI (#1684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Use `locked-figure-aria.tsx` within LockedLineSettings. - Write the function to auto-generate the locked line aria label with its coordinates and visible labels. Issue: https://khanacademy.atlassian.net/browse/LEMS-2376 ## Test plan: `yarn jest packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.test.tsx` Storybook - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags - Confirm that the locked line aria label field is already populated with "Line PQ" (passed in via builder) - Confirm that pressing "Auto-generate" works with no labels, one label, and multiple labels - Confirm that updating the aria label in the editor also updates the aria label on the line - Check the web inspector to confirm the aria-label text is updated - Use a screen reader to confirm the new label is read out image https://github.com/user-attachments/assets/36738b4f-6524-4034-adbb-52f6cc53cb94 Author: nishasy Reviewers: benchristel, nishasy, catandthemachines, anakaren-rojas Required Reviewers: Approved By: benchristel Checks: ✅ Publish npm snapshot (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), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1684 --- .changeset/itchy-pears-march.md | 6 + .../locked-line-settings.test.tsx | 157 ++++++++++++++++++ .../locked-figures/locked-line-settings.tsx | 41 ++++- 3 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 .changeset/itchy-pears-march.md diff --git a/.changeset/itchy-pears-march.md b/.changeset/itchy-pears-march.md new file mode 100644 index 0000000000..3b7781655a --- /dev/null +++ b/.changeset/itchy-pears-march.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Locked Figure Aria] Locked line aria label editor UI 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 92ed774f76..d5b5e345cc 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 @@ -550,4 +550,161 @@ describe("LockedLineSettings", () => { }); }); }); + + describe("Aria label", () => { + test("Renders with aria label", () => { + // Arrange + + // Act + render( + , + {wrapper: RenderStateRoot}, + ); + + const input = screen.getByRole("textbox", {name: "Aria label"}); + + // Assert + expect(input).toHaveValue("Line at (x, y)"); + }); + + test("calls onChangeProps when the aria label is updated", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const input = screen.getByRole("textbox", {name: "Aria label"}); + await userEvent.clear(input); + await userEvent.type(input, "A"); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "A", + }); + }); + + test("aria label auto-generates with different kind", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Segment from (0, 0) to (2, 2)", + }); + }); + + test("aria label auto-generates (no labels)", async () => { + // Arrange + const onChangeProps = jest.fn(); + + // Act + render( + , + {wrapper: RenderStateRoot}, + ); + + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Line from (0, 0) to (2, 2)", + }); + }); + + test("aria label auto-generates (one label)", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Line from (0, 0) to (2, 2) with label A", + }); + }); + + test("aria label auto-generates (multiple labels)", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const autoGenButton = screen.getByRole("button", { + name: "Auto-generate", + }); + await userEvent.click(autoGenButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + ariaLabel: "Line from (0, 0) to (2, 2) with labels A, B", + }); + }); + }); }); 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 563e82be4f..5bf69ad6ac 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 @@ -21,6 +21,7 @@ import PerseusEditorAccordion from "../../../components/perseus-editor-accordion import ColorSelect from "./color-select"; import LineStrokeSelect from "./line-stroke-select"; import LineSwatch from "./line-swatch"; +import LockedFigureAria from "./locked-figure-aria"; import LockedFigureSettingsActions from "./locked-figure-settings-actions"; import LockedLabelSettings from "./locked-label-settings"; import LockedPointSettings from "./locked-point-settings"; @@ -56,6 +57,7 @@ const LockedLineSettings = (props: Props) => { showPoint1, showPoint2, labels, + ariaLabel, onChangeProps, onMove, onRemove, @@ -69,6 +71,24 @@ const LockedLineSettings = (props: Props) => { // Check if the line has length 0. const isInvalid = kvector.equal(point1.coord, point2.coord); + function getPrepopulatedAriaLabel() { + let str = `${capitalizeKind} from (${point1.coord[0]}, ${point1.coord[1]}) to (${point2.coord[0]}, ${point2.coord[1]})`; + + if (labels && labels.length > 0) { + str += " with label"; + // Make it "with labels" instead of "with label" if there are + // multiple labels. + if (labels.length > 1) { + str += "s"; + } + + // Separate additional labels with commas. + str += ` ${labels.map((l) => l.text).join(", ")}`; + } + + return str; + } + function handleChangePoint( newPointProps: Partial, index: 0 | 1, @@ -243,9 +263,28 @@ const LockedLineSettings = (props: Props) => { onChangeProps={(newProps) => handleChangePoint(newProps, 1)} /> + {flags?.["mafs"]?.["locked-figures-aria"] && ( + <> + + + + { + onChangeProps(newProps); + }} + /> + + )} + {flags?.["mafs"]?.["locked-line-labels"] && ( <> + + + + Visible labels {labels?.map((label, labelIndex) => (