[editor-pi] [Interactive Graph] Support tick labels that are multiple…

…s of pi (#2167)

## 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


<img width="847" alt="image" src="https://github.com/user-attachments/assets/cb2044df-d8c2-496e-b78c-2ee7c6643b7d" />

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: #2167

.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

packages/perseus-editor/src/widgets/interactive-graph-editor/components/interactive-graph-settings.tsx

@@ -495,6 +495,7 @@ class InteractiveGraphSettings extends React.Component<Props, State> {
                                             onChange={(vals) =>
                                                 this.changeRange(0, vals)
                                             }
+                                            allowPiTruncation={true}
                                         />
                                     </LabeledRow>
                                 </div>
@@ -505,6 +506,7 @@ class InteractiveGraphSettings extends React.Component<Props, State> {
                                             onChange={(vals) =>
                                                 this.changeRange(1, vals)
                                             }
+                                            allowPiTruncation={true}
                                         />
                                     </LabeledRow>
                                 </div>
@@ -515,6 +517,7 @@ class InteractiveGraphSettings extends React.Component<Props, State> {
                                         <RangeInput
                                             value={this.state.stepTextbox}
                                             onChange={this.changeStep}
+                                            allowPiTruncation={true}
                                         />
                                     </LabeledRow>
                                 </div>
@@ -523,6 +526,7 @@ class InteractiveGraphSettings extends React.Component<Props, State> {
                                         <RangeInput
                                             value={this.state.gridStepTextbox}
                                             onChange={this.changeGridStep}
+                                            allowPiTruncation={true}
                                         />
                                     </LabeledRow>
                                 </div>
@@ -533,6 +537,7 @@ class InteractiveGraphSettings extends React.Component<Props, State> {
                                         <RangeInput
                                             value={this.state.snapStepTextbox}
                                             onChange={this.changeSnapStep}
+                                            allowPiTruncation={true}
                                         />
                                     </LabeledRow>
                                 </div>

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(
+                <NumberInput
+                    value={value}
+                    onChange={jest.fn()}
+                    allowPiTruncation={true}
+                />,
+            );
+            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(
+            <NumberInput
+                value={Math.PI}
+                onChange={jest.fn()}
+                allowPiTruncation={false}
+            />,
+        );
+        const input = screen.getByRole("textbox");
+
+        // Assert
+        expect(input).not.toHaveValue("π");
+        expect(input).toHaveValue(Math.PI.toString());
+    });
 });

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<any, any> {
         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<any, any> {
         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);

packages/perseus/src/components/range-input.tsx

@@ -15,6 +15,7 @@ class RangeInput extends React.Component<any> {
         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<any> {
                     // eslint-disable-next-line react/jsx-no-bind
                     onChange={this.onChange.bind(this, 0)}
                     placeholder={this.props.placeholder[0]}
+                    allowPiTruncation={this.props.allowPiTruncation}
                 />
                 <NumberInput
                     {...this.props}
@@ -51,6 +53,7 @@ class RangeInput extends React.Component<any> {
                     // eslint-disable-next-line react/jsx-no-bind
                     onChange={this.onChange.bind(this, 1)}
                     placeholder={this.props.placeholder[1]}
+                    allowPiTruncation={this.props.allowPiTruncation}
                 />
             </div>
         );

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);
+    });
+});

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
+    );
+}

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);
+    });
+});

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 (
         <g className="tick" aria-hidden={true}>
             <line x1={x1} y1={y1} x2={x2} y2={y2} className="axis-tick" />
@@ -66,14 +71,23 @@ const YGridTick = ({
                     x={xPositionText}
                     y={yPositionText}
                 >
-                    {y.toString()}
+                    {yLabel}
                 </text>
             )}
         </g>
     );
 };
 
-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 (
         <g className="tick" aria-hidden={true}>
             <line x1={x1} y1={y1} x2={x2} y2={y2} className="axis-tick" />
@@ -124,7 +140,7 @@ const XGridTick = ({x, range}: {x: number; range: [Interval, Interval]}) => {
                     x={xPositionText}
                     y={yPositionText}
                 >
-                    {x.toString()}
+                    {xLabel}
                 </text>
             }
         </g>
@@ -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}
                         />
                     );
                 })}

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,
                     },
                 },
             }}