docs(changeset): Interactive Graph code cleaup and bug fix for edge-based label positioning.

.changeset/silent-news-change.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": minor
+---
+
+Interactive Graph code cleaup and bug fix for edge-based label positioning.

packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.test.ts

@@ -1,9 +1,9 @@
 import {
-    getLabelPosition,
-    fontSize,
     clampLabelPosition,
+    fontSize,
+    getLabelPosition,
     getLabelTransform,
-} from "./axis-labels";
+} from "./utils";
 
 import type {GraphDimensions} from "../types";
 import type {vec} from "mafs";
@@ -24,7 +24,9 @@ describe("getLabelPosition", () => {
             [200, -2 * fontSize], // Y Label at [Horizontal center of the graph, 2x fontSize above the top edge]
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
     it("should return the correct position for the default graph without a labelLocation", () => {
         const graphInfo: GraphDimensions = {
@@ -41,7 +43,9 @@ describe("getLabelPosition", () => {
             [200, -2 * fontSize], // Y Label at [Horizontal center of the graph, 2x fontSize above the top edge]
         ];
 
-        expect(getLabelPosition(graphInfo, undefined)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, undefined, [1, 1])).toEqual(
+            expected,
+        );
     });
 
     it("should return the correct position for a graph with high positive min-ranges", () => {
@@ -55,11 +59,13 @@ describe("getLabelPosition", () => {
         };
         const labelLocation = "onAxis";
         const expected = [
-            [400, 400 + 1.25 * fontSize], // X Label at [Right edge of the graph, vertical center of the graph]
-            [-1.5 * fontSize, -2 * fontSize], // Y Label at [Horizontal center of the graph, 2x fontSize above the top edge]
+            [400, 400 + 1.25 * fontSize],
+            [-1.5 * fontSize, -2 * fontSize],
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
 
     it("should return the correct position for a graph with low negative max-ranges", () => {
@@ -73,11 +79,13 @@ describe("getLabelPosition", () => {
         };
         const labelLocation = "onAxis";
         const expected = [
-            [400, -2 * fontSize], // X Label at [Right edge of the graph, vertical center of the graph]
-            [400 + 1.25 * fontSize, -2 * fontSize], // Y Label at [Horizontal center of the graph, 2x fontSize above the top edge]
+            [400, -2 * fontSize],
+            [400 + 1.25 * fontSize, -2 * fontSize],
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
 
     it("should return the correct position for labels set to alongEdge", () => {
@@ -91,11 +99,13 @@ describe("getLabelPosition", () => {
         };
         const labelLocation = "alongEdge";
         const expected = [
-            [200, 400 + fontSize], // X Label at [Horizontal center of the graph, 1x fontSize below the bottom edge]
-            [-fontSize, 200 - fontSize], // Y label at [1x fontSize to the left of the left edge, vertical center of the graph]
+            [200, 400 + fontSize * 1.5],
+            [-fontSize * 1.25, 200 - fontSize],
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
 
     it("should return the correct position for labels set to alongEdge with wholly negative ranges", () => {
@@ -109,11 +119,13 @@ describe("getLabelPosition", () => {
         };
         const labelLocation = "alongEdge";
         const expected = [
-            [200, 400 + fontSize], // X Label at [Horizontal center of the graph, 1x fontSize below the bottom edge]
-            [-fontSize, 200 - fontSize], // Y label at [1x fontSize to the left of the left edge, vertical center of the graph]
+            [200, 400 + fontSize * 1.5],
+            [-fontSize * 1.25, 200 - fontSize],
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
 
     it("should return the correct position for labels set to alongEdge with wholly positive ranges", () => {
@@ -127,11 +139,13 @@ describe("getLabelPosition", () => {
         };
         const labelLocation = "alongEdge";
         const expected = [
-            [200, 400 + 3 * fontSize], // X Label at [Horizontal center of the graph, 3x fontSize below the bottom edge]
-            [-3 * fontSize, 200 - fontSize], // Y label at [3x fontSize to the left of the left edge, vertical center of the graph]
+            [200, 400 + 3 * fontSize],
+            [-2.75 * fontSize, 200 - fontSize],
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
 
     it("should return the correct position for labels set to alongEdge with min ranges at 0", () => {
@@ -146,11 +160,13 @@ describe("getLabelPosition", () => {
         };
         const labelLocation = "alongEdge";
         const expected = [
-            [200, 400 + 3 * fontSize], // X Label at [Horizontal center of the graph, 3x fontSize below the bottom edge]
-            [-3 * fontSize, 200 - fontSize], // Y label at [3x fontSize to the left of the left edge, vertical center of the graph]
+            [200, 400 + 3 * fontSize],
+            [-2.75 * fontSize, 200 - fontSize],
         ];
 
-        expect(getLabelPosition(graphInfo, labelLocation)).toEqual(expected);
+        expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
+            expected,
+        );
     });
 });
 describe("getLabelTransform", () => {

packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.tsx

@@ -1,21 +1,18 @@
-import {vec} from "mafs";
 import React from "react";
 
 import {getDependencies} from "../../../dependencies";
-import {pointToPixel} from "../graphs/use-transform";
-import {MAX, MIN, X, Y} from "../math";
+import {X, Y} from "../math";
 import useGraphConfig from "../reducer/use-graph-config";
 import {replaceOutsideTeX} from "../utils";
 
+import {fontSize, getLabelPosition, getLabelTransform} from "./utils";
+
 import type {I18nContextType} from "../../../components/i18n-context";
-import type {GraphConfig} from "../reducer/use-graph-config";
 import type {GraphDimensions} from "../types";
 
-// Exported for testing purposes
-export const fontSize = 14;
-
 export default function AxisLabels({i18n}: {i18n: I18nContextType}) {
-    const {range, labels, width, height, labelLocation} = useGraphConfig();
+    const {range, labels, width, height, labelLocation, tickStep} =
+        useGraphConfig();
 
     const graphInfo: GraphDimensions = {
         range,
@@ -27,6 +24,7 @@ export default function AxisLabels({i18n}: {i18n: I18nContextType}) {
     const [xAxisLabelLocation, yAxisLabelLocation] = getLabelPosition(
         graphInfo,
         labelLocation,
+        tickStep,
     );
 
     const [xAxisLabelText, yAxisLabelText] = labels;
@@ -73,110 +71,3 @@ export default function AxisLabels({i18n}: {i18n: I18nContextType}) {
         </>
     );
 }
-
-/* Get the transform for the labels based on the labelLocation
- *  Exported for testing purposes.
- */
-export const getLabelTransform = (
-    labelLocation: GraphConfig["labelLocation"],
-): {xLabelTransform: string; yLabelTransform: string} => {
-    // onAxis is the default label location
-    const isOnAxis = labelLocation === undefined || labelLocation === "onAxis";
-
-    const xLabelTransform = isOnAxis
-        ? "translate(7px, -50%)"
-        : "translate(-50%, -50%)";
-
-    const yLabelTransform = isOnAxis
-        ? "translate(-50%, 0px)"
-        : "translate(-50%, 0px) rotate(-90deg)";
-
-    return {xLabelTransform, yLabelTransform};
-};
-
-/* Calculate the position of the main axis labels based on the labelLocation
- * and the ranges of the graph. Exported for testing purposes.
- */
-// This function clamps the label position to ensure that the labels do not go too far
-// outside the graph bounds. This is only required when the labelLocations are set to "onAxis".
-export const clampLabelPosition = (
-    labelPosition: vec.Vector2,
-    graphInfo: GraphDimensions,
-): vec.Vector2 => {
-    // Clamp the label position to ensure that the labels do not go too far outside of the graph bounds.
-    // Unfortuantely, this logic is a little complex as we have to account for both the positive and negative
-    // ranges of the graph, and the variable position of the axis tick labels.
-    const x = Math.max(
-        // The maximum x value is the width of the graph + 1.25 font sizes, which aligns the label with the axis ticks
-        // when the x-axis is out of bounds to the right of the graph.
-        Math.min(labelPosition[X], graphInfo.width + fontSize * 1.25),
-        // The minimum x value is -1.5 font sizes, as this aligns the label with the axis ticks
-        // when the y-axis is out of bounds to the left of the graph.
-        -fontSize * 1.5,
-    );
-    const y = Math.max(
-        // The maximum y value is the height of the graph + 1.25 font sizes, which aligns the label with the axis ticks
-        // when the y-axis is out of bounds below the graph.
-        Math.min(labelPosition[Y], graphInfo.height + fontSize * 1.25),
-        // The minimum y value is -2 font sizes, which aligns the label with the axis ticks
-        // when the y-axis is out of bounds above the graph.
-        -fontSize * 2,
-    );
-    return [x, y];
-};
-export const getLabelPosition = (
-    graphInfo: GraphDimensions,
-    labelLocation: GraphConfig["labelLocation"],
-): vec.Vector2[] => {
-    // If the labels are placed along the edge of the graph, we need to place them at the
-    // center of the graph, which is the average of the min and max values of the axes.
-    if (labelLocation === "alongEdge") {
-        // Offset the labels by a certain amount based on the range of the graph, to ensure that
-        // the labels do not overlap with the axis tick if they are out of the graph bounds.
-        const xAxisLabelOffset: [number, number] =
-            graphInfo.range[Y][MIN] >= 0
-                ? [0, fontSize * 3] // Move the label down by 3 font sizes if the y-axis min is positive
-                : [0, fontSize]; // Move the label down by 1 font size if the y-axis min is negative
-        const yAxisLabelOffset: [number, number] =
-            graphInfo.range[X][MIN] >= 0
-                ? [-fontSize * 3, -fontSize] // Move the label left by 3 font sizes if the x-axis min is positive
-                : [-fontSize, -fontSize]; // Move the label left by 1 font size if the x-axis min is negative
-
-        // Calculate the location of the labels to be halfway between the min and max values of the axes
-        const xAxisLabelLocation: vec.Vector2 = [
-            (graphInfo.range[X][MIN] + graphInfo.range[X][MAX]) / 2,
-            graphInfo.range[Y][MIN],
-        ];
-        const yAxisLabelLocation: vec.Vector2 = [
-            graphInfo.range[X][MIN],
-            (graphInfo.range[Y][MIN] + graphInfo.range[Y][MAX]) / 2,
-        ];
-
-        // Convert the Vector2 coordinates to pixel coordinates and add the offsets
-        const xLabel = vec.add(
-            pointToPixel(xAxisLabelLocation, graphInfo),
-            xAxisLabelOffset,
-        );
-        const yLabel = vec.add(
-            pointToPixel(yAxisLabelLocation, graphInfo),
-            yAxisLabelOffset,
-        );
-
-        return [xLabel, yLabel];
-    }
-
-    // Otherwise, the labels are placed on the axes (default), and we need to
-    // place them at the end of the axis, which is the maximum value of the axis.
-    const xLabelInitial: vec.Vector2 = [graphInfo.range[X][MAX], 0];
-    const yLabelInitial: vec.Vector2 = [0, graphInfo.range[Y][MAX]];
-    const yLabelOffset: vec.Vector2 = [0, -fontSize * 2]; // Move the y-axis label up by 2 font sizes
-
-    let xLabel = pointToPixel(xLabelInitial, graphInfo);
-    let yLabel = vec.add(pointToPixel(yLabelInitial, graphInfo), yLabelOffset);
-
-    // Clamp the label positions to ensure that the labels do not go too far outside of the graph bounds.
-    xLabel = clampLabelPosition(xLabel, graphInfo);
-    yLabel = clampLabelPosition(yLabel, graphInfo);
-
-    return [xLabel, yLabel];
-};

packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.test.ts

@@ -3,7 +3,7 @@ import {
     shouldShowLabel,
     countSignificantDecimals,
     divideByAndShowPi,
-} from "./axis-ticks";
+} from "./utils";
 
 import type {Interval} from "mafs";
 

packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx

@@ -4,6 +4,12 @@ import {useTransformVectorsToPixels} from "../graphs/use-transform";
 import {MAX, MIN, X, Y} from "../math";
 import useGraphConfig from "../reducer/use-graph-config";
 
+import {
+    divideByAndShowPi,
+    generateTickLocations,
+    shouldShowLabel,
+} from "./utils";
+
 import type {Interval, vec} from "mafs";
 
 // The size of the ticks and labels in pixels
@@ -147,82 +153,6 @@ const XGridTick = ({
     );
 };
 
-// Determines whether to show the label for the given tick
-// Currently, the only condition is to hide the label at -tickStep
-// on the y-axis when the y-axis is within the graph bounds
-export const shouldShowLabel = (
-    currentTick: number,
-    range: [Interval, Interval],
-    tickStep: number,
-) => {
-    let showLabel = true;
-
-    // If the y-axis is within the graph and currentTick equals -tickStep, hide the label
-    if (
-        range[X][MIN] < -tickStep &&
-        range[X][MAX] > 0 &&
-        currentTick === -tickStep
-    ) {
-        showLabel = false;
-    }
-
-    return showLabel;
-};
-
-export function generateTickLocations(
-    tickStep: number,
-    min: number,
-    max: number,
-): number[] {
-    const ticks: number[] = [];
-
-    // Calculate the number of significant decimals in the tick step so
-    // that we can match the desired precision when generating ticks.
-    const decimalSigFigs: number = countSignificantDecimals(tickStep);
-
-    // Add ticks in the positive direction
-    const start = Math.max(min, 0);
-    for (let i = start + tickStep; i < max; i += tickStep) {
-        // Match to the same number of decimal places as the tick step
-        // to avoid floating point errors when working with small numbers
-        ticks.push(parseFloat(i.toFixed(decimalSigFigs)));
-    }
-
-    // Add ticks in the negative direction
-    // Start at the first tick after 0 or the maximum value if it is negative
-    let i = Math.min(max, 0) - tickStep;
-    for (i; i > min; i -= tickStep) {
-        ticks.push(i);
-    }
-    return ticks;
-}
-
-// Count the number of significant digits after the decimal point
-export const countSignificantDecimals = (number: number): number => {
-    const numStr = number.toString();
-    if (!numStr.includes(".")) {
-        return 0;
-    }
-    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;