diff --git a/.changeset/silent-news-change.md b/.changeset/silent-news-change.md new file mode 100644 index 00000000000..ed903ce3c9e --- /dev/null +++ b/.changeset/silent-news-change.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Interactive Graph code cleaup and bug fix for edge-based label positioning. diff --git a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.test.ts b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.test.ts index 7d6fc4325bd..664c9f5929c 100644 --- a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.test.ts +++ b/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", () => { diff --git a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.tsx b/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.tsx index 5254d6d166e..e7acb40ec4a 100644 --- a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.tsx +++ b/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]; -}; 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 72accd4779b..45bb79bc287 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 @@ -3,7 +3,7 @@ import { shouldShowLabel, countSignificantDecimals, divideByAndShowPi, -} from "./axis-ticks"; +} from "./utils"; import type {Interval} from "mafs"; 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 a17a1a14b65..9be6624cadf 100644 --- a/packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-ticks.tsx +++ b/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; diff --git a/packages/perseus/src/widgets/interactive-graphs/backgrounds/utils.ts b/packages/perseus/src/widgets/interactive-graphs/backgrounds/utils.ts new file mode 100644 index 00000000000..cc15c5563c8 --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/backgrounds/utils.ts @@ -0,0 +1,255 @@ +import {vec} from "mafs"; + +import {pointToPixel} from "../graphs/use-transform"; +import {X, Y, MIN, MAX} from "../math"; + +import type {GraphConfig} from "../reducer/use-graph-config"; +import type {GraphDimensions} from "../types"; +import type {Interval} from "mafs"; + +// Exported for testing purposes +export const fontSize = 14; + +/* 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]; +}; + +/* 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 maximum number of digits needed to display tick labels on the y-axis. + * This accounts for both the integer part of the range values and decimal places in the tick step. + */ +export const calculateMaxDigitsInRange = ( + range: [number, number], + tickStep: number, +): number => { + // Calculate maximum digits in the integer part of range values + const maxDigitsInRange = Math.max( + String(Math.abs(Math.floor(range[MIN]))).length, + String(Math.abs(Math.ceil(range[MAX]))).length, + ); + + // A decimal tick step can result in additional digits, so we need to account for that, + // including the leading 0 and decimal point. + const tickStepSigFigs = countSignificantDecimals(tickStep) + 2; + + // Return the maximum of the two + return Math.max(maxDigitsInRange, tickStepSigFigs); +}; + +export const getLabelPosition = ( + graphInfo: GraphDimensions, + labelLocation: GraphConfig["labelLocation"], + tickStep: GraphConfig["tickStep"], +): 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 2 font sizes if the y-axis min is positive + : [0, fontSize * 1.5]; // Move the label down by 1.5 font sizes if the y-axis min is negative + + // Determine if the x-axis min is negative and relatively close to zero, based on the scale of the x-axis range. + const isRelativelyCloseToZero = + graphInfo.range[X][MIN] < 0 && + Math.abs(graphInfo.range[X][MIN]) < + (graphInfo.range[X][MAX] - graphInfo.range[X][MIN]) * 0.07; + + // Determine if the tick labels extend beyond the left edge of the graph, either + // because the x-axis is wholly positive or because the x-axis min is negative and close to zero. + const needsExtraSpacing = + graphInfo.range[X][MIN] >= 0 || isRelativelyCloseToZero; + + // When tick labels extend beyond the left edge of the graph, we need to account for their + // width to prevent the main axis label from overlapping with them. + let paddingRequiredForTickLabels = 0; + if (needsExtraSpacing) { + // Calculate the maximum digits needed for tick labels + const maxDigits = calculateMaxDigitsInRange( + graphInfo.range[Y], + tickStep[Y], + ); + + // Estimate the width of the tick labels so that we can ensure + // the axis labels do not overlap with the tick labels. + paddingRequiredForTickLabels = + maxDigits * (fontSize * 0.75) + + (graphInfo.range[Y][MIN] < 0 && graphInfo.range[X][MIN] <= 0 + ? fontSize * 0.5 + : 0); // Add space for negative sign if needed + } + + const yAxisLabelOffset: [number, number] = [ + -fontSize * 1.25 - paddingRequiredForTickLabels, + -fontSize, + ]; + + // 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, + ]; + + // If we're VERY close to zero, we want to account for the strange grid offset to avoid + // the y-axis label from getting too far from the tick labels. + if (isRelativelyCloseToZero) { + yAxisLabelLocation[X] = + yAxisLabelLocation[X] - graphInfo.range[X][MIN]; + } + + // 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]; +}; +// 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 + "π"; + } +} 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 e1cf63c580a..dbf7c59d963 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 @@ -207,6 +207,20 @@ export const MafsWithXAxisOffTop: Story = { }; export const MafsWithLabelsAlongEdge: Story = { + args: { + question: interactiveGraphQuestionBuilder() + .withXRange(-10, 10) + .withYRange(-10, 10) + .withAxisLabels( + "Video Game Hours per Week", + "Reaction Time (milliseconds)", + ) + .withLabelLocation("alongEdge") + .build(), + }, +}; + +export const MafsWithLabelsAlongEdgeAtLeft: Story = { args: { question: interactiveGraphQuestionBuilder() .withXRange(0, 10) @@ -234,6 +248,51 @@ export const MafsWithLabelsAlongEdgeJustOverLeft: Story = { }, }; +export const MafsWithLabelsAlongEdgeAtRight: Story = { + args: { + question: interactiveGraphQuestionBuilder() + .withXRange(0, 0.01) + .withYRange(0, 0.01) + .withTickStep(0.001, 0.001) + .withGridStep(0.001, 0.001) + .withAxisLabels( + "Video Game Hours per Week", + "Reaction Time (milliseconds)", + ) + .withLabelLocation("alongEdge") + .build(), + }, +}; + +export const MafsWithLabelsAlongEdgeWithCloseToZeroXMin: Story = { + args: { + question: interactiveGraphQuestionBuilder() + .withXRange(-0.03, 0.84) + .withYRange(-2.8, 63) + .withTickStep(0.2, 10) + .withGridStep(0.05, 5) + .withSnapStep(0.025, 2) + .withAxisLabels("Time (seconds)", "Distance (meters)") + .withLabelLocation("alongEdge") + .build(), + }, +}; + +export const MafsWithLabelsAlongEdgeWithCloseToZeroXMinMultipliedBy1000: Story = + { + args: { + question: interactiveGraphQuestionBuilder() + .withXRange(-30, 840) + .withYRange(-2.8, 63) + .withTickStep(200, 10) + .withGridStep(50, 5) + .withSnapStep(25, 2) + .withAxisLabels("Time (seconds)", "Distance (meters)") + .withLabelLocation("alongEdge") + .build(), + }, + }; + export const MafsWithLabelsAlongEdgeZoomed: Story = { args: { question: interactiveGraphQuestionBuilder() diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx index b6541262285..0b4d70966ae 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx @@ -10,10 +10,10 @@ import { } from "../../../../../testing/test-dependencies"; import * as Dependencies from "../../dependencies"; -import {calculateNestedSVGCoords, MafsGraph} from "./mafs-graph"; +import {MafsGraph} from "./mafs-graph"; import {actions, REMOVE_POINT} from "./reducer/interactive-graph-action"; import {interactiveGraphReducer} from "./reducer/interactive-graph-reducer"; -import {getBaseMafsGraphPropsForTests} from "./utils"; +import {calculateNestedSVGCoords, getBaseMafsGraphPropsForTests} from "./utils"; import type {MafsGraphProps} from "./mafs-graph"; import type {InteractiveGraphState} from "./types"; diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx index a31de96611a..3274ccc85bf 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx @@ -38,11 +38,15 @@ import {renderRayGraph} from "./graphs/ray"; import {renderSegmentGraph} from "./graphs/segment"; import {renderSinusoidGraph} from "./graphs/sinusoid"; import {getArrayWithoutDuplicates} from "./graphs/utils"; -import {MIN, X, Y} from "./math"; +import {X, Y} from "./math"; import {Protractor} from "./protractor"; import {actions} from "./reducer/interactive-graph-action"; import {GraphConfigContext} from "./reducer/use-graph-config"; -import {isUnlimitedGraphState, REMOVE_BUTTON_ID} from "./utils"; +import { + calculateNestedSVGCoords, + isUnlimitedGraphState, + REMOVE_BUTTON_ID, +} from "./utils"; import type {InteractiveGraphAction} from "./reducer/interactive-graph-action"; import type { @@ -671,61 +675,6 @@ function handleKeyboardEvent( } } -// Calculate the difference between the min and max values of a range -const getRangeDiff = (range: vec.Vector2) => { - const [min, max] = range; - return Math.abs(max - min); -}; - -// We need to adjust the nested SVG viewbox x and Y values based on the range of the graph in order -// to ensure that the graph is sized and positioned correctly within the Mafs SVG and the clipping mask. -// Exported for testing. -export const calculateNestedSVGCoords = ( - range: vec.Vector2[], - width: number, - height: number, -): {viewboxX: number; viewboxY: number} => { - // X RANGE - let viewboxX = 0; // When xMin is 0, we want to use 0 as the viewboxX value - const totalXRange = getRangeDiff(range[X]); - const gridCellWidth = width / totalXRange; - const minX = range[X][MIN]; - - // If xMin is entirely positive, we need to adjust the - // viewboxX to be the grid cell width multiplied by xMin - if (minX > 0) { - viewboxX = gridCellWidth * Math.abs(minX); - } - // If xMin is negative, we need to adjust the viewboxX to be - // the negative value of the grid cell width multiplied by xMin - if (minX < 0) { - viewboxX = -gridCellWidth * Math.abs(minX); - } - - // Y RANGE - let viewboxY = -height; // When yMin is 0, we want to use the negative value of the graph height - const totalYRange = getRangeDiff(range[Y]); - const gridCellHeight = height / totalYRange; - const minY = range[Y][MIN]; - - // If the y range is entirely positive, we want a negative sum of the - // height and the gridcell height multiplied by the absolute value of yMin - if (minY > 0) { - viewboxY = -height - gridCellHeight * Math.abs(minY); - } - - // If the yMin is negative, we want to multiply the gridcell height - // by the absolute value of yMin, and subtract the full height of the graph - if (minY < 0) { - viewboxY = gridCellHeight * Math.abs(minY) - height; - } - - return { - viewboxX, - viewboxY, - }; -}; - const renderGraphElements = (props: { state: InteractiveGraphState; dispatch: (action: InteractiveGraphAction) => unknown; diff --git a/packages/perseus/src/widgets/interactive-graphs/utils.ts b/packages/perseus/src/widgets/interactive-graphs/utils.ts index d4ac0bb0976..fd03105d79b 100644 --- a/packages/perseus/src/widgets/interactive-graphs/utils.ts +++ b/packages/perseus/src/widgets/interactive-graphs/utils.ts @@ -1,7 +1,7 @@ import {pureMarkdownRules} from "@khanacademy/pure-markdown"; import SimpleMarkdown from "@khanacademy/simple-markdown"; -import {clampToBox, inset, MIN, size} from "./math"; +import {clampToBox, inset, MIN, size, X, Y} from "./math"; import type {MafsGraphProps} from "./mafs-graph"; import type {InteractiveGraphState, UnlimitedGraphState} from "./types"; @@ -162,3 +162,57 @@ export function getBaseMafsGraphPropsForTests(): MafsGraphProps { }, }; } +// Calculate the difference between the min and max values of a range +const getRangeDiff = (range: vec.Vector2) => { + const [min, max] = range; + return Math.abs(max - min); +}; + +// We need to adjust the nested SVG viewbox x and Y values based on the range of the graph in order +// to ensure that the graph is sized and positioned correctly within the Mafs SVG and the clipping mask. +// Exported for testing. +export const calculateNestedSVGCoords = ( + range: vec.Vector2[], + width: number, + height: number, +): {viewboxX: number; viewboxY: number} => { + // X RANGE + let viewboxX = 0; // When xMin is 0, we want to use 0 as the viewboxX value + const totalXRange = getRangeDiff(range[X]); + const gridCellWidth = width / totalXRange; + const minX = range[X][MIN]; + + // If xMin is entirely positive, we need to adjust the + // viewboxX to be the grid cell width multiplied by xMin + if (minX > 0) { + viewboxX = gridCellWidth * Math.abs(minX); + } + // If xMin is negative, we need to adjust the viewboxX to be + // the negative value of the grid cell width multiplied by xMin + if (minX < 0) { + viewboxX = -gridCellWidth * Math.abs(minX); + } + + // Y RANGE + let viewboxY = -height; // When yMin is 0, we want to use the negative value of the graph height + const totalYRange = getRangeDiff(range[Y]); + const gridCellHeight = height / totalYRange; + const minY = range[Y][MIN]; + + // If the y range is entirely positive, we want a negative sum of the + // height and the gridcell height multiplied by the absolute value of yMin + if (minY > 0) { + viewboxY = -height - gridCellHeight * Math.abs(minY); + } + + // If the yMin is negative, we want to multiply the gridcell height + // by the absolute value of yMin, and subtract the full height of the graph + if (minY < 0) { + viewboxY = gridCellHeight * Math.abs(minY) - height; + } + + return { + viewboxX, + viewboxY, + }; +};