From 0a5599de399d24ac6445e60b6d54da4b8fb1c507 Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Wed, 29 Jan 2025 14:27:23 -0800 Subject: [PATCH 1/4] Remove deprecated exports from the `perseus` package (#2164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: They have been moved to `perseus-core`. Issue: none Test plan: CI should be green. Author: benchristel Reviewers: Myranae, jeremywiebe, handeyeco Required Reviewers: Approved By: Myranae, jeremywiebe 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), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2164 --- .changeset/poor-masks-return.md | 5 +++++ packages/perseus/src/index.ts | 36 --------------------------------- 2 files changed, 5 insertions(+), 36 deletions(-) create mode 100644 .changeset/poor-masks-return.md diff --git a/.changeset/poor-masks-return.md b/.changeset/poor-masks-return.md new file mode 100644 index 0000000000..54d6811699 --- /dev/null +++ b/.changeset/poor-masks-return.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": major +--- + +Removes the following deprecated exports from `@khanacademy/perseus`: `parsePerseusItem`, `parseAndMigratePerseusItem`, `parseAndMigratePerseusArticle`, `isSuccess`, `isFailure`, `Result`, `Success`, `Failure`. Clients should import these from `@khanacademy/perseus-core` instead. diff --git a/packages/perseus/src/index.ts b/packages/perseus/src/index.ts index 864e277717..b890bdb999 100644 --- a/packages/perseus/src/index.ts +++ b/packages/perseus/src/index.ts @@ -108,28 +108,6 @@ export { getAnswerFromUserInput, getImagesWithoutAltData, } from "./util/extract-perseus-data"; -export { - /** - * @deprecated - import this function from perseus-core instead - */ - parsePerseusItem, - /** - * @deprecated - import this function from perseus-core instead - */ - parseAndMigratePerseusItem, - /** - * @deprecated - import this function from perseus-core instead - */ - parseAndMigratePerseusArticle, - /** - * @deprecated - import this function from perseus-core instead - */ - isSuccess, - /** - * @deprecated - import this function from perseus-core instead - */ - isFailure, -} from "@khanacademy/perseus-core"; export { generateTestPerseusItem, @@ -203,20 +181,6 @@ export type { SharedRendererProps, } from "./types"; export type {ParsedValue} from "./util"; -export type { - /** - * @deprecated - import this function from perseus-core instead - */ - Result, - /** - * @deprecated - import this function from perseus-core instead - */ - Success, - /** - * @deprecated - import this function from perseus-core instead - */ - Failure, -} from "@khanacademy/perseus-core"; export type {Coord} from "./interactive2/types"; export type { RendererPromptJSON, From a470c799eb53c87e08fb2f829b27e114ca80f63f Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Wed, 29 Jan 2025 14:28:34 -0800 Subject: [PATCH 2/4] Create a function to get the public options for LabelImage (#2171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: LEMS-2769 ## Test plan: `yarn test` Author: benchristel Reviewers: Myranae, benchristel, handeyeco, jeremywiebe Required Reviewers: Approved By: Myranae 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) Pull Request URL: https://github.com/Khan/perseus/pull/2171 --- .changeset/forty-meals-teach.md | 6 +++ packages/perseus-core/src/index.ts | 1 + .../label-image/label-image-util.test.ts | 46 +++++++++++++++++++ .../widgets/label-image/label-image-util.ts | 41 +++++++++++++++++ packages/perseus/src/types.ts | 4 +- .../src/widgets/label-image/label-image.tsx | 2 + 6 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 .changeset/forty-meals-teach.md create mode 100644 packages/perseus-core/src/widgets/label-image/label-image-util.test.ts create mode 100644 packages/perseus-core/src/widgets/label-image/label-image-util.ts diff --git a/.changeset/forty-meals-teach.md b/.changeset/forty-meals-teach.md new file mode 100644 index 0000000000..712fead8a2 --- /dev/null +++ b/.changeset/forty-meals-teach.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-core": minor +--- + +Create a function to get the public options for the LabelImage widget diff --git a/packages/perseus-core/src/index.ts b/packages/perseus-core/src/index.ts index 00b19d8a91..68f76a4bc3 100644 --- a/packages/perseus-core/src/index.ts +++ b/packages/perseus-core/src/index.ts @@ -115,3 +115,4 @@ export type * from "./widgets/logic-export.types"; export {default as getOrdererPublicWidgetOptions} from "./widgets/orderer/orderer-util"; export {default as getCategorizerPublicWidgetOptions} from "./widgets/categorizer/categorizer-util"; export {default as getExpressionPublicWidgetOptions} from "./widgets/expression/expression-util"; +export {default as getLabelImagePublicWidgetOptions} from "./widgets/label-image/label-image-util"; diff --git a/packages/perseus-core/src/widgets/label-image/label-image-util.test.ts b/packages/perseus-core/src/widgets/label-image/label-image-util.test.ts new file mode 100644 index 0000000000..c4a66d4e64 --- /dev/null +++ b/packages/perseus-core/src/widgets/label-image/label-image-util.test.ts @@ -0,0 +1,46 @@ +import getLabelImagePublicWidgetOptions from "./label-image-util"; + +import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core"; + +describe("getLabelImagePublicWidgetOptions", () => { + it("removes private fields", () => { + const options: PerseusLabelImageWidgetOptions = { + choices: ["right", "wrong"], + markers: [ + { + answers: ["right"], + label: "", + x: 0, + y: 0, + }, + ], + imageUrl: "", + imageHeight: 0, + imageWidth: 0, + imageAlt: "", + hideChoicesFromInstructions: false, + multipleAnswers: false, + static: false, + }; + + const publicWidgetOptions = getLabelImagePublicWidgetOptions(options); + + expect(publicWidgetOptions).toEqual({ + choices: ["right", "wrong"], + markers: [ + { + label: "", + x: 0, + y: 0, + }, + ], + imageUrl: "", + imageHeight: 0, + imageWidth: 0, + imageAlt: "", + hideChoicesFromInstructions: false, + multipleAnswers: false, + static: false, + }); + }); +}); diff --git a/packages/perseus-core/src/widgets/label-image/label-image-util.ts b/packages/perseus-core/src/widgets/label-image/label-image-util.ts new file mode 100644 index 0000000000..94ab284369 --- /dev/null +++ b/packages/perseus-core/src/widgets/label-image/label-image-util.ts @@ -0,0 +1,41 @@ +import type { + PerseusLabelImageMarker, + PerseusLabelImageWidgetOptions, +} from "@khanacademy/perseus-core"; + +/** + * For details on the individual options, see the + * PerseusLabelImageWidgetOptions type + */ +type LabelImagePublicWidgetOptions = { + choices: PerseusLabelImageWidgetOptions["choices"]; + imageUrl: PerseusLabelImageWidgetOptions["imageUrl"]; + imageAlt: PerseusLabelImageWidgetOptions["imageAlt"]; + imageHeight: PerseusLabelImageWidgetOptions["imageHeight"]; + imageWidth: PerseusLabelImageWidgetOptions["imageWidth"]; + markers: ReadonlyArray; + hideChoicesFromInstructions: PerseusLabelImageWidgetOptions["hideChoicesFromInstructions"]; + multipleAnswers: PerseusLabelImageWidgetOptions["multipleAnswers"]; + static: PerseusLabelImageWidgetOptions["static"]; +}; + +type LabelImageMarkerPublicData = Pick< + PerseusLabelImageMarker, + "x" | "y" | "label" +>; + +export default function getLabelImagePublicWidgetOptions( + options: PerseusLabelImageWidgetOptions, +): LabelImagePublicWidgetOptions { + return { + ...options, + markers: options.markers.map(getLabelImageMarkerPublicData), + }; +} + +function getLabelImageMarkerPublicData( + marker: PerseusLabelImageMarker, +): LabelImageMarkerPublicData { + const {answers: _, ...publicData} = marker; + return publicData; +} diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index 174d756969..728d1cc22d 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -4,6 +4,7 @@ import type {SizeClass} from "./util/sizing-utils"; import type {WidgetPromptJSON} from "./widget-ai-utils/prompt-types"; import type {KeypadAPI} from "@khanacademy/math-input"; import type { + getLabelImagePublicWidgetOptions, Hint, PerseusAnswerArea, PerseusGraphType, @@ -543,7 +544,8 @@ export type WidgetScorerFunction = ( export type PublicWidgetOptionsFunction = | typeof getCategorizerPublicWidgetOptions | typeof getOrdererPublicWidgetOptions - | typeof getExpressionPublicWidgetOptions; + | typeof getExpressionPublicWidgetOptions + | typeof getLabelImagePublicWidgetOptions; export type WidgetExports< T extends React.ComponentType & Widget = React.ComponentType, diff --git a/packages/perseus/src/widgets/label-image/label-image.tsx b/packages/perseus/src/widgets/label-image/label-image.tsx index 34a5db8eb9..f598f94d0d 100644 --- a/packages/perseus/src/widgets/label-image/label-image.tsx +++ b/packages/perseus/src/widgets/label-image/label-image.tsx @@ -6,6 +6,7 @@ * knowledge by directly interacting with the image. */ +import {getLabelImagePublicWidgetOptions} from "@khanacademy/perseus-core"; import { scoreLabelImageMarker, scoreLabelImage, @@ -761,4 +762,5 @@ export default { // TODO(LEMS-2656): remove TS suppression // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusLabelImageUserInput'. scorer: scoreLabelImage, + getPublicWidgetOptions: getLabelImagePublicWidgetOptions, } satisfies WidgetExports; From 781834332921f839028aa5cb3c5c867121859e02 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Wed, 29 Jan 2025 15:58:51 -0800 Subject: [PATCH 3/4] Fix 'repository' field in kmath's package.json (#2162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: While working on some automation, I noticed that this package still points to its old repository (before we moved everything into this monorepo) Issue: "none" ## Test plan: Review the change. Hpoe it's correct! Author: jeremywiebe Reviewers: mark-fitzgerald, catandthemachines Required Reviewers: Approved By: mark-fitzgerald, catandthemachines 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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2162 --- .changeset/curly-bikes-nail.md | 5 +++++ packages/kmath/package.json | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 .changeset/curly-bikes-nail.md diff --git a/.changeset/curly-bikes-nail.md b/.changeset/curly-bikes-nail.md new file mode 100644 index 0000000000..66fa10774b --- /dev/null +++ b/.changeset/curly-bikes-nail.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/kmath": patch +--- + +Update repository information diff --git a/packages/kmath/package.json b/packages/kmath/package.json index d368f43e28..c17846e89a 100644 --- a/packages/kmath/package.json +++ b/packages/kmath/package.json @@ -9,7 +9,8 @@ }, "repository": { "type": "git", - "url": "https://github.com/Khan/kmath.git" + "url": "https://github.com/Khan/perseus.git", + "directory": "packages/kmath" }, "bugs": { "url": "https://github.com/Khan/perseus/issues" @@ -35,4 +36,4 @@ "underscore": "1.4.4" }, "keywords": [] -} \ No newline at end of file +} From c72166c3046aa7c0fcd4e3348d604248e8565c2e Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Wed, 29 Jan 2025 16:07:37 -0800 Subject: [PATCH 4/4] [editor-pi] [Interactive Graph] Support tick labels that are multiples of pi (#2167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Currently, if a graph has tick labels that are multiples of pi, such as in a sinusoid graph, content authors have to set the graph markings to "none" and load in a graphie background image that has the graph with the desired tick markings. This is not only inconvenient for content authors, it also means that the screen reader will not read the correct coordinates for these interactive graphs. It might say "2" when it should be saying "2 pi." Implementing a fix for this to improve the lives of our content authors and our screen reader users. - Editor - Check if the value loaded into NumberInput is a multiple of pi, and not already written in the "pi" format. - If so, and if the `allowPiTrunacation` prop is true, divide this number by pi and append "π" to the value. - This will make it so that the value shown in the text input will show as pi format even if loaded in without it. (6.28... --> "2π") - Mafs graph - In the AxisTicks component, check if the tick step is a multiple of pi. - If so, divide all the ticks by pi and append pi to the value (special cases for 0, 1π, and -1π). - This will result in the ticks showing as integers * pi. Issue: https://khanacademy.atlassian.net/browse/LEMS-2059 ## Test plan: `yarn jest packages/perseus/src/components/__tests__/number-input.test.tsx` `yarn jest packages/perseus/src/util/math-utils.test.ts` `yarn jest packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.test.ts` `yarn jest packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx` Storybook - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-sinusoid image Author: nishasy Reviewers: SonicScrewdriver, nishasy, catandthemachines Required Reviewers: Approved By: SonicScrewdriver Checks: ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (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), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2167 --- .changeset/honest-parents-press.md | 6 ++ .../components/interactive-graph-settings.tsx | 5 ++ .../__tests__/number-input.test.tsx | 43 ++++++++++++ .../perseus/src/components/number-input.tsx | 14 ++++ .../perseus/src/components/range-input.tsx | 3 + packages/perseus/src/util/math-utils.test.ts | 32 +++++++++ packages/perseus/src/util/math-utils.ts | 15 +++++ .../backgrounds/axis-ticks.test.ts | 22 +++++++ .../backgrounds/axis-ticks.tsx | 45 ++++++++++++- .../interactive-graph-regression.stories.tsx | 8 +++ .../interactive-graph.test.tsx | 66 +++++++++++++++++++ .../interactive-graph.testdata.ts | 16 ++++- 12 files changed, 271 insertions(+), 4 deletions(-) create mode 100644 .changeset/honest-parents-press.md create mode 100644 packages/perseus/src/util/math-utils.test.ts create mode 100644 packages/perseus/src/util/math-utils.ts diff --git a/.changeset/honest-parents-press.md b/.changeset/honest-parents-press.md new file mode 100644 index 0000000000..ee9f58be49 --- /dev/null +++ b/.changeset/honest-parents-press.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Interactive Graph] Allow axis tick labels to be multiples of pi diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx index 8e0158aaaa..baa32fbbb4 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx @@ -495,6 +495,7 @@ class InteractiveGraphSettings extends React.Component { onChange={(vals) => this.changeRange(0, vals) } + allowPiTruncation={true} /> @@ -505,6 +506,7 @@ class InteractiveGraphSettings extends React.Component { onChange={(vals) => this.changeRange(1, vals) } + allowPiTruncation={true} /> @@ -515,6 +517,7 @@ class InteractiveGraphSettings extends React.Component { @@ -523,6 +526,7 @@ class InteractiveGraphSettings extends React.Component { @@ -533,6 +537,7 @@ class InteractiveGraphSettings extends React.Component { diff --git a/packages/perseus/src/components/__tests__/number-input.test.tsx b/packages/perseus/src/components/__tests__/number-input.test.tsx index 45e26d6269..88583db66c 100644 --- a/packages/perseus/src/components/__tests__/number-input.test.tsx +++ b/packages/perseus/src/components/__tests__/number-input.test.tsx @@ -111,4 +111,47 @@ describe("NumberInput", function () { expect(onChange).not.toHaveBeenCalled(); }); + + it.each` + value | expected + ${Math.PI} | ${"π"} + ${Math.PI * 2} | ${"2π"} + ${Math.PI * 0.5} | ${"π/2"} + `( + "allowPiTruncation: loads $value as $expected on mount", + async function ({value, expected}) { + // Arrange + + // Act + render( + , + ); + const input = screen.getByRole("textbox"); + + // Assert + expect(input).toHaveValue(expected); + }, + ); + + it("does not auto-convert number to pi format when allowPiTruncation is false", async function () { + // Arrange + + // Act + render( + , + ); + const input = screen.getByRole("textbox"); + + // Assert + expect(input).not.toHaveValue("π"); + expect(input).toHaveValue(Math.PI.toString()); + }); }); diff --git a/packages/perseus/src/components/number-input.tsx b/packages/perseus/src/components/number-input.tsx index b618ad1b07..b336d6c346 100644 --- a/packages/perseus/src/components/number-input.tsx +++ b/packages/perseus/src/components/number-input.tsx @@ -6,6 +6,7 @@ import * as React from "react"; import _ from "underscore"; import Util from "../util"; +import {isPiMultiple} from "../util/math-utils"; import {PerseusI18nContext} from "./i18n-context"; @@ -48,6 +49,7 @@ class NumberInput extends React.Component { checkValidity: PropTypes.func, size: PropTypes.oneOf(["mini", "small", "normal"]), label: PropTypes.oneOf(["put your labels outside your inputs!"]), + allowPiTruncation: PropTypes.bool, }; static defaultProps: any = { @@ -63,6 +65,18 @@ class NumberInput extends React.Component { format: this.props.format, }; + componentDidMount() { + // If the value is a multiple of pi, but it is not in the pi format, + // then convert it to the pi format and show it as a multiple of pi. + const value = this.getValue(); + if (this.props.allowPiTruncation && value !== null && value !== 0) { + if (this.state.format !== "pi" && isPiMultiple(value)) { + this._setValue(value / Math.PI, "pi"); + this.setState({format: "pi"}); + } + } + } + componentDidUpdate(prevProps: any) { if (!knumber.equal(this.getValue(), this.props.value)) { this._setValue(this.props.value, this.state.format); diff --git a/packages/perseus/src/components/range-input.tsx b/packages/perseus/src/components/range-input.tsx index b60dcad131..cb0c565332 100644 --- a/packages/perseus/src/components/range-input.tsx +++ b/packages/perseus/src/components/range-input.tsx @@ -15,6 +15,7 @@ class RangeInput extends React.Component { onChange: PropTypes.func.isRequired, placeholder: PropTypes.array, checkValidity: PropTypes.func, + allowPiTruncation: PropTypes.bool, }; static defaultProps: any = { @@ -43,6 +44,7 @@ class RangeInput extends React.Component { // eslint-disable-next-line react/jsx-no-bind onChange={this.onChange.bind(this, 0)} placeholder={this.props.placeholder[0]} + allowPiTruncation={this.props.allowPiTruncation} /> { // eslint-disable-next-line react/jsx-no-bind onChange={this.onChange.bind(this, 1)} placeholder={this.props.placeholder[1]} + allowPiTruncation={this.props.allowPiTruncation} /> ); diff --git a/packages/perseus/src/util/math-utils.test.ts b/packages/perseus/src/util/math-utils.test.ts new file mode 100644 index 0000000000..4313e3d543 --- /dev/null +++ b/packages/perseus/src/util/math-utils.test.ts @@ -0,0 +1,32 @@ +import {isPiMultiple} from "./math-utils"; + +describe("isPiMultiple", () => { + test.each` + case | number + ${"π"} | ${Math.PI} + ${"2π"} | ${Math.PI * 2} + ${"3π"} | ${Math.PI * 3} + ${"-π"} | ${Math.PI * -1} + ${"-2π"} | ${Math.PI * -2} + ${"π/2"} | ${Math.PI / 2} + ${"π/3"} | ${Math.PI / 3} + ${"π/4"} | ${Math.PI / 4} + ${"π/6"} | ${Math.PI / 6} + ${"2π/3"} | ${(Math.PI * 2) / 3} + `("should return true for $case", ({number}) => { + expect(isPiMultiple(number)).toBe(true); + }); + + test.each` + case | number + ${"0"} | ${0} + ${"1"} | ${1} + ${"-1"} | ${-1} + ${"3.14"} | ${3.14} + ${"3.14159"} | ${3.14159} + ${"2.5"} | ${2.5} + ${"-1.5"} | ${-1.5} + `("should return false for $case", ({number}) => { + expect(isPiMultiple(number)).toBe(false); + }); +}); diff --git a/packages/perseus/src/util/math-utils.ts b/packages/perseus/src/util/math-utils.ts new file mode 100644 index 0000000000..890c06e460 --- /dev/null +++ b/packages/perseus/src/util/math-utils.ts @@ -0,0 +1,15 @@ +export function isPiMultiple(value: number): boolean { + // Early return for integers that clearly aren't multiples + // of pi to save some computation. + if (Number.isInteger(value)) { + return false; + } + + return ( + value % Math.PI === 0 || + value % (Math.PI / 2) === 0 || + value % (Math.PI / 3) === 0 || + value % (Math.PI / 4) === 0 || + value % (Math.PI / 6) === 0 + ); +} diff --git a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.test.ts b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.test.ts index a59b289b3a..72accd4779 100644 --- a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.test.ts @@ -2,6 +2,7 @@ import { generateTickLocations, shouldShowLabel, countSignificantDecimals, + divideByAndShowPi, } from "./axis-ticks"; import type {Interval} from "mafs"; @@ -76,3 +77,24 @@ describe("countSignificantDecimals", () => { }, ); }); + +describe("divideByAndShowPi", () => { + it.each` + value | expected + ${Math.PI} | ${"π"} + ${-1 * Math.PI} | ${"-π"} + ${0} | ${"0"} + ${Math.PI * 2} | ${"2π"} + ${Math.PI * -2} | ${"-2π"} + ${Math.PI / 2} | ${"0.5π"} + ${Math.PI / -2} | ${"-0.5π"} + `("should display $expected as a multiple of pi", ({value, expected}) => { + // Arrange + + // Act + const result = divideByAndShowPi(value); + + // Assert + expect(result).toBe(expected); + }); +}); 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 40757be70f..a17a1a14b6 100644 --- a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx @@ -14,10 +14,13 @@ const YGridTick = ({ y, range, tickStep, + showPi, }: { y: number; range: [Interval, Interval]; tickStep: number; + // Whether to show the tick label as a multiple of pi + showPi: boolean; }) => { // If the graph requires out-of-bounds labels, we want to make sure to set the // coordinates to the edge of the visible range of the graph. Otherwise, @@ -55,6 +58,8 @@ const YGridTick = ({ // to hide the label at -1 on the y-axis to prevent overlap with the x-axis label const showLabel = shouldShowLabel(y, range, tickStep); + const yLabel = showPi ? divideByAndShowPi(y) : y.toString(); + return ( @@ -66,14 +71,23 @@ const YGridTick = ({ x={xPositionText} y={yPositionText} > - {y.toString()} + {yLabel} )} ); }; -const XGridTick = ({x, range}: {x: number; range: [Interval, Interval]}) => { +const XGridTick = ({ + x, + range, + showPi, +}: { + x: number; + range: [Interval, Interval]; + // Whether to show the tick label as a multiple of pi + showPi: boolean; +}) => { // If the graph requires out-of-bounds labels, we want to make sure to set the // coordinates to the edge of the visible range of the graph. Otherwise, // the ticks and labels would render outside of the clipping-mask. @@ -113,6 +127,8 @@ const XGridTick = ({x, range}: {x: number; range: [Interval, Interval]}) => { const xPositionText = xPosition + xAdjustment; const yPositionText = yPosition + yAdjustment; + const xLabel = showPi ? divideByAndShowPi(x) : x.toString(); + return ( @@ -124,7 +140,7 @@ const XGridTick = ({x, range}: {x: number; range: [Interval, Interval]}) => { x={xPositionText} y={yPositionText} > - {x.toString()} + {xLabel} } @@ -190,6 +206,23 @@ export const countSignificantDecimals = (number: number): number => { return numStr.split(".")[1].length; }; +// Show the given value as a multiple of pi (already assumed to be +// a multiple of pi). Exported for testing +export function divideByAndShowPi(value: number): string { + const dividedValue = value / Math.PI; + + switch (dividedValue) { + case 1: + return "π"; + case -1: + return "-π"; + case 0: + return "0"; + default: + return dividedValue + "π"; + } +} + export const AxisTicks = () => { const {tickStep, range} = useGraphConfig(); const [[xMin, xMax], [yMin, yMax]] = range; @@ -209,6 +242,9 @@ export const AxisTicks = () => { key={`y-grid-tick-${y}`} range={range} tickStep={tickStep[Y]} + // Show the tick labels as multiples of pi + // if the tick step is a multiple of pi. + showPi={tickStep[Y] % Math.PI === 0} /> ); })} @@ -220,6 +256,9 @@ export const AxisTicks = () => { x={x} key={`x-grid-tick-${x}`} range={range} + // Show the tick labels as multiples of pi + // if the tick step is a multiple of pi. + showPi={tickStep[X] % Math.PI === 0} /> ); })} diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-regression.stories.tsx b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-regression.stories.tsx index 2d6276cd84..245271e6ca 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-regression.stories.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-regression.stories.tsx @@ -4,6 +4,7 @@ import Renderer from "../../renderer"; import {mockStrings} from "../../strings"; import {interactiveGraphQuestionBuilder} from "./interactive-graph-question-builder"; +import {sinusoidWithPiTicks} from "./interactive-graph.testdata"; import type {PerseusRenderer} from "@khanacademy/perseus-core"; import type {Meta, StoryObj} from "@storybook/react"; @@ -249,6 +250,12 @@ export const MafsWithProtractor: Story = { }, }; +export const MafsWithPiTicks: Story = { + args: { + question: sinusoidWithPiTicks, + }, +}; + function MafsQuestionRenderer(props: {question: PerseusRenderer}) { const {question} = props; return ( @@ -263,6 +270,7 @@ function MafsQuestionRenderer(props: {question: PerseusRenderer}) { segment: true, circle: true, linear: true, + sinusoid: true, }, }, }} 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 3ab059b049..d7882f127e 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx @@ -57,6 +57,7 @@ import { segmentWithLockedPolygonWhite, segmentWithLockedVectors, sinusoidQuestionWithDefaultCorrect, + sinusoidWithPiTicks, } from "./interactive-graph.testdata"; import {trueForAllMafsSupportedGraphTypes} from "./mafs-supported-graph-types"; @@ -1394,4 +1395,69 @@ describe("Interactive Graph", function () { expect(graph).not.toHaveAttribute("aria-describedby"); }); }); + + describe("axis labels", () => { + test("should render x axis labels as multiples of pi if the tick step is a multiple of pi", () => { + // Arrange + const {container} = renderQuestion(sinusoidWithPiTicks, { + flags: {mafs: {sinusoid: true}}, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const xAxisLabels = container.querySelectorAll( + ".x-axis-ticks > .tick", + ); + + // Assert + // The first label is π + expect(xAxisLabels[0]).toHaveTextContent("π"); + + // There are four more labels in the positive direction + for (let i = 0; i < 4; i++) { + expect(xAxisLabels[i + 1]).toHaveTextContent(`${i + 2}π`); + } + + // First label in the negative direction is -π + expect(xAxisLabels[5]).toHaveTextContent("-π"); + + // There are four more labels in the negative direction + for (let i = 0; i < 4; i++) { + expect(xAxisLabels[i + 6]).toHaveTextContent(`-${i + 2}π`); + } + }); + + test("should render y axis labels as multiples of pi if the tick step is a multiple of pi", () => { + // Arrange + const {container} = renderQuestion(sinusoidWithPiTicks, { + flags: {mafs: {sinusoid: true}}, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const yAxisLabels = container.querySelectorAll( + ".y-axis-ticks > .tick", + ); + + // Assert + // The first label is 2π + expect(yAxisLabels[0]).toHaveTextContent("2π"); + + // There are three more labels in the positive direction, + // increasing by 2π each time. + for (let i = 1; i < 4; i++) { + expect(yAxisLabels[i]).toHaveTextContent(`${2 + 2 * i}π`); + } + + // First label in the negative direction is empty + // to make room for the x axis labels (in place of -2π). + expect(yAxisLabels[4]).toHaveTextContent(""); + + // There are three more labels in the negative direction, + // decreasing by 2π each time. + for (let i = 1; i < 4; i++) { + expect(yAxisLabels[i + 4]).toHaveTextContent(`-${2 + 2 * i}π`); + } + }); + }); }); 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 1ffc49f6a5..32adc957c6 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts @@ -459,10 +459,14 @@ export const quadraticWithStartingCoordsQuestion: PerseusRenderer = export const sinusoidWithStartingCoordsQuestion: PerseusRenderer = interactiveGraphQuestionBuilder() + .withXRange(-6 * Math.PI, 6 * Math.PI) + .withTickStep(Math.PI, 1) + .withGridStep(Math.PI, 1) + .withSnapStep(Math.PI / 2, 1) .withSinusoid({ startCoords: [ [0, 0], - [1, -1], + [3 * Math.PI, -3], ], }) .build(); @@ -1064,3 +1068,13 @@ export const graphWithLabeledFunction: PerseusRenderer = labels: [{text: "F"}], }) .build(); + +export const sinusoidWithPiTicks: PerseusRenderer = + interactiveGraphQuestionBuilder() + .withXRange(-6 * Math.PI, 6 * Math.PI) + .withYRange(-10 * Math.PI, 10 * Math.PI) + .withTickStep(Math.PI, 2 * Math.PI) + .withGridStep(Math.PI, 2 * Math.PI) + .withSnapStep(Math.PI / 2, Math.PI) + .withSinusoid() + .build();