Skip to content

Commit

Permalink
Move math functions to a math/ folder (#1390)
Browse files Browse the repository at this point in the history
Note to reviewers: it's going to be easiest to review this PR one commit
at a time!

This is the first part of a multi-part refactor. I am submitting this
intermediate PR to get feedback and hopefully avoid running into too many
merge conflicts.

In future PRs, I plan to:

- move more functions into `math/`
- reuse the existing functions in more places
- re-export functions from `kmath` in `math/index.ts`, possibly
  wrapping them with more suitable type signatures
- fix TODOs added in this PR.

Adding a `math/` folder gives us a clear place to put new mathy functions,
and will help us locate and reuse the functions that already exist.

Issue: LEMS-2135

## Test plan:

`yarn test`

Author: benchristel

Reviewers: mark-fitzgerald, benchristel, jeremywiebe, nicolecomputer, nishasy, SonicScrewdriver

Required Reviewers:

Approved By: mark-fitzgerald

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1390
  • Loading branch information
benchristel authored Jul 3, 2024
1 parent 5de4833 commit 7e6ccf3
Show file tree
Hide file tree
Showing 32 changed files with 318 additions and 137 deletions.
6 changes: 6 additions & 0 deletions .changeset/loud-goats-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

Internal: Move graphing-agnostic, mathy functions in the interactive graph code to a math/ folder.
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import React from "react";
import {getDependencies} from "../../dependencies";

import {pointToPixel} from "./graphs/use-transform";
import {MAX, X, Y} from "./math";
import useGraphConfig from "./reducer/use-graph-config";

import type {GraphDimensions} from "./types";

export default function AxisLabels() {
const {range, labels, width, height} = useGraphConfig();

const yAxisLabelLocation: vec.Vector2 = [0, range[1][1]];
const xAxisLabelLocation: vec.Vector2 = [range[0][1], 0];
const yAxisLabelLocation: vec.Vector2 = [0, range[Y][MAX]];
const xAxisLabelLocation: vec.Vector2 = [range[X][MAX], 0];

const [xAxisLabelText, yAxisLabelText] = labels;
const graphInfo: GraphDimensions = {
Expand Down
26 changes: 14 additions & 12 deletions packages/perseus/src/widgets/interactive-graphs/axis-ticks.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from "react";

import {useTransformVectorsToPixels} from "./graphs/use-transform";
import {MAX, MIN, X, Y} from "./math";
import useGraphConfig from "./reducer/use-graph-config";

import type {GraphDimensions} from "./types";
Expand All @@ -18,13 +19,13 @@ const YGridTick = ({y, graphInfo}: {y: number; graphInfo: GraphDimensions}) => {

// If the graph is zoomed in, we want to make sure the ticks are still visible
// even if they are outside the graph's range.
if (graphInfo.range[0][0] > 0) {
if (graphInfo.range[X][MIN] > 0) {
// If the graph is on the positive side of the x-axis, lock the ticks to the left side of the graph
xPointOnAxis = graphInfo.range[0][0];
xPointOnAxis = graphInfo.range[X][MIN];
}
if (graphInfo.range[0][1] < 0) {
if (graphInfo.range[X][MAX] < 0) {
// If the graph is on the negative side of the x-axis, lock the ticks to the right side of the graph
xPointOnAxis = graphInfo.range[0][1];
xPointOnAxis = graphInfo.range[X][MAX];
}

const pointOnAxis: vec.Vector2 = [xPointOnAxis, y];
Expand Down Expand Up @@ -57,13 +58,13 @@ const XGridTick = ({x, graphInfo}: {x: number; graphInfo: GraphDimensions}) => {
let yPointOnAxis = 0;
// If the graph is zoomed in, we want to make sure the ticks are still visible
// even if they are outside the graph's range.
if (graphInfo.range[1][0] > 0) {
if (graphInfo.range[Y][MIN] > 0) {
// If the graph is on the positive side of the y-axis, lock the ticks to the top of the graph
yPointOnAxis = graphInfo.range[1][0];
yPointOnAxis = graphInfo.range[Y][MIN];
}
if (graphInfo.range[1][1] < 0) {
if (graphInfo.range[Y][MAX] < 0) {
// If the graph is on the negative side of the x-axis, lock the ticks to the bottom of the graph
yPointOnAxis = graphInfo.range[1][1];
yPointOnAxis = graphInfo.range[Y][MAX];
}

const pointOnAxis: vec.Vector2 = [x, yPointOnAxis];
Expand Down Expand Up @@ -119,11 +120,12 @@ export const AxisTicks = () => {
height,
};

const [xMin, xMax] = range[0];
const [yMin, yMax] = range[1];
// TODO(benchristel): destructure these in one line
const [xMin, xMax] = range[X];
const [yMin, yMax] = range[Y];

const yTickStep = tickStep[1];
const xTickStep = tickStep[0];
const yTickStep = tickStep[Y];
const xTickStep = tickStep[X];

const yGridTicks = generateTickLocations(yTickStep, yMin, yMax);
const xGridTicks = generateTickLocations(xTickStep, xMin, xMax);
Expand Down
26 changes: 13 additions & 13 deletions packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import {vec} from "mafs";
import * as React from "react";
import {useRef} from "react";

import {snap, X, Y} from "../math";
import {moveCenter, moveRadiusPoint} from "../reducer/interactive-graph-action";
import {getRadius} from "../reducer/interactive-graph-state";
import useGraphConfig from "../reducer/use-graph-config";
import {snap} from "../utils";

import {StyledMovablePoint} from "./components/movable-point";
import {useDraggable} from "./use-draggable";
Expand Down Expand Up @@ -68,17 +68,17 @@ function MovableCircle(props: {
>
<ellipse
className="focus-ring"
cx={centerPx[0]}
cy={centerPx[1]}
rx={radiiPx[0] + 3}
ry={radiiPx[1] + 3}
cx={centerPx[X]}
cy={centerPx[Y]}
rx={radiiPx[X] + 3}
ry={radiiPx[Y] + 3}
/>
<ellipse
className="circle"
cx={centerPx[0]}
cy={centerPx[1]}
rx={radiiPx[0]}
ry={radiiPx[1]}
cx={centerPx[X]}
cy={centerPx[Y]}
rx={radiiPx[X]}
ry={radiiPx[Y]}
/>
<DragHandle center={center} />
</g>
Expand All @@ -97,10 +97,10 @@ function DragHandle(props: {center: [x: number, y: number]}) {
<>
<rect
className="movable-circle-handle"
x={topLeft[0]}
y={topLeft[1]}
width={dragHandleDimensions[0]}
height={dragHandleDimensions[1]}
x={topLeft[X]}
y={topLeft[Y]}
width={dragHandleDimensions[X]}
height={dragHandleDimensions[Y]}
rx={cornerRadius}
ry={cornerRadius}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {type vec} from "mafs";
import * as React from "react";

import {pathBuilder} from "../../../../util/svg";
import {X, Y} from "../../math";
import {useTransformVectorsToPixels} from "../use-transform";

type Props = {
Expand All @@ -27,7 +28,7 @@ export function Arrowhead(props: Props) {
return (
<g
className="interactive-graph-arrowhead"
transform={`translate(${point[0]} ${point[1]}) rotate(${props.angle})`}
transform={`translate(${point[X]} ${point[Y]}) rotate(${props.angle})`}
>
<g transform="translate(-1.5)">
<path
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from "react";

import {X, Y} from "../../math";
import useGraphConfig from "../../reducer/use-graph-config";
import {pointToPixel} from "../use-transform";

Expand Down Expand Up @@ -41,11 +42,12 @@ export const AxisTickLabels = () => {
const graphConfig = useGraphConfig();
const {tickStep, range} = graphConfig;

const [xMin, xMax] = range[0];
const [yMin, yMax] = range[1];
// TODO(benchristel): use destructuring here
const [xMin, xMax] = range[X];
const [yMin, yMax] = range[Y];

const yTickStep = tickStep[1];
const xTickStep = tickStep[0];
const yTickStep = tickStep[Y];
const xTickStep = tickStep[X];

const yGridTicks = generateTickLocations(yTickStep, yMin, yMax);
const xGridTicks = generateTickLocations(xTickStep, xMin, xMax);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {MovableLine, trimRange} from "./movable-line";
import type {Interval, vec} from "mafs";

describe("trimRange", () => {
it("does not trim smaller than [[0, 0], [0, 0]]", () => {
it("does not trim ranges below a size of 0", () => {
const graphDimensionsInPixels: vec.Vector2 = [1, 1];
const range: [Interval, Interval] = [
[0, 1],
Expand All @@ -19,8 +19,8 @@ describe("trimRange", () => {
const trimmed = trimRange(range, graphDimensionsInPixels);

expect(trimmed).toEqual([
[0, 0],
[0, 0],
[0.5, 0.5],
[0.5, 0.5],
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import {vec} from "mafs";
import {useRef, useState} from "react";
import * as React from "react";

import {inset, snap, size} from "../../math";
import useGraphConfig from "../../reducer/use-graph-config";
import {snap, TARGET_SIZE} from "../../utils";
import {TARGET_SIZE} from "../../utils";
import {useDraggable} from "../use-draggable";
import {useTransformVectorsToPixels} from "../use-transform";
import {getIntersectionOfRayWithBox} from "../utils";
Expand Down Expand Up @@ -227,16 +228,5 @@ export function trimRange(
const graphUnitsPerPixelY = size(yRange) / pixelsTall;
const graphUnitsToTrimX = pixelsToTrim * graphUnitsPerPixelX;
const graphUnitsToTrimY = pixelsToTrim * graphUnitsPerPixelY;
return [trim(xRange, graphUnitsToTrimX), trim(yRange, graphUnitsToTrimY)];
}

function trim(interval: Interval, amount: number): Interval {
if (size(interval) < amount * 2) {
return [0, 0];
}
return [interval[0] + amount, interval[1] - amount];
}

function size(interval: Interval): number {
return interval[1] - interval[0];
return inset([graphUnitsToTrimX, graphUnitsToTrimY], range);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Tooltip from "@khanacademy/wonder-blocks-tooltip";
import * as React from "react";
import {forwardRef} from "react";

import {X, Y} from "../../math";
import useGraphConfig from "../../reducer/use-graph-config";
import {useTransformVectorsToPixels} from "../use-transform";

Expand Down Expand Up @@ -56,8 +57,9 @@ export const MovablePointView = forwardRef(

const [[x, y]] = useTransformVectorsToPixels(point);

const [xMin, xMax] = range[0];
const [yMin, yMax] = range[1];
// TODO(benchristel): destructure range in one line
const [xMin, xMax] = range[X];
const [yMin, yMax] = range[Y];

const [[verticalStartX]] = useTransformVectorsToPixels([xMin, 0]);
const [[verticalEndX]] = useTransformVectorsToPixels([xMax, 0]);
Expand Down Expand Up @@ -119,7 +121,7 @@ export const MovablePointView = forwardRef(
<Tooltip
autoUpdate={true}
backgroundColor={wbColorName}
content={`(${point[0]}, ${point[1]})`}
content={`(${point[X]}, ${point[Y]})`}
contentStyle={{color: "white"}}
>
{svgForPoint}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import {color as WBColor} from "@khanacademy/wonder-blocks-tokens";
import * as React from "react";
import {useRef} from "react";

import {snap} from "../../math";
import useGraphConfig from "../../reducer/use-graph-config";
import {snap} from "../../utils";
import {useDraggable} from "../use-draggable";

import {MovablePointView} from "./movable-point-view";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as React from "react";

import {X, Y} from "../../math";

import type {vec} from "mafs";
import type {SVGProps} from "react";

Expand All @@ -15,10 +17,10 @@ export function SVGLine(props: Props) {
const {start, end, style, className, testId} = props;
return (
<line
x1={start[0]}
y1={start[1]}
x2={end[0]}
y2={end[1]}
x1={start[X]}
y1={start[Y]}
x2={end[X]}
y2={end[Y]}
style={style}
className={className}
data-testid={testId}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {Polygon, vec} from "mafs";
import * as React from "react";

import {snap} from "../math";
import {moveAll, movePoint} from "../reducer/interactive-graph-action";
import {TARGET_SIZE, snap} from "../utils";
import {TARGET_SIZE} from "../utils";

import {Angle} from "./components/angle";
import {StyledMovablePoint} from "./components/movable-point";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {color} from "@khanacademy/wonder-blocks-tokens";
import {Plot} from "mafs";
import * as React from "react";

import {X, Y} from "../math";
import {movePoint} from "../reducer/interactive-graph-action";

import {StyledMovablePoint} from "./components/movable-point";
Expand Down Expand Up @@ -86,15 +87,15 @@ export const getSinusoidCoefficients = (
const p2 = coords[1];

// If the x-coordinates are the same, we are unable to calculate the coefficients
if (p2[0] === p1[0]) {
if (p2[X] === p1[X]) {
return;
}

// Resulting coefficients are canonical for this sine curve
const amplitude = p2[1] - p1[1];
const angularFrequency = Math.PI / (2 * (p2[0] - p1[0]));
const phase = p1[0] * angularFrequency;
const verticalOffset = p1[1];
const amplitude = p2[Y] - p1[Y];
const angularFrequency = Math.PI / (2 * (p2[X] - p1[X]));
const phase = p1[X] * angularFrequency;
const verticalOffset = p1[Y];

return {amplitude, angularFrequency, phase, verticalOffset};
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {useTransformContext, vec} from "mafs";
import * as React from "react";
import invariant from "tiny-invariant";

import {X, Y} from "../math";
import useGraphConfig from "../reducer/use-graph-config";

import type {RefObject} from "react";
Expand Down Expand Up @@ -68,10 +69,10 @@ export function useDraggable(args: Params): DragState {
} = state;

const direction = [
yDownDirection[0],
-yDownDirection[1],
yDownDirection[X],
-yDownDirection[Y],
] as vec.Vector2;
const span = Math.abs(direction[0]) ? xSpan : ySpan;
const span = Math.abs(direction[X]) ? xSpan : ySpan;

let divisions = 50;
if (altKey || metaKey) {
Expand Down
5 changes: 3 additions & 2 deletions packages/perseus/src/widgets/interactive-graphs/grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as React from "react";

import AxisArrows from "./axis-arrows";
import {AxisTicks} from "./axis-ticks";
import {X, Y} from "./math";

import type {GraphRange} from "../../perseus-types";
import type {SizeClass} from "../../util/sizing-utils";
Expand Down Expand Up @@ -63,8 +64,8 @@ export const Grid = (props: GridProps) => {
return props.markings === "none" ? null : (
<>
<Coordinates.Cartesian
xAxis={axisOptions(props, 0)}
yAxis={axisOptions(props, 1)}
xAxis={axisOptions(props, X)}
yAxis={axisOptions(props, Y)}
/>
{
// Only render the axis ticks and arrows if the markings are set to a full "graph"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import AssetContext from "../../asset-context";
import {SvgImage} from "../../components";
import {interactiveSizes} from "../../styles/constants";

import {X} from "./math";

import type {PerseusImageBackground} from "../../perseus-types";

interface Props {
Expand All @@ -19,7 +21,7 @@ interface Props {
export const LegacyGrid = ({box, backgroundImage}: Props) => {
const {url, width, height} = backgroundImage ?? {};
if (url && typeof url === "string") {
const scale = box[0] / interactiveSizes.defaultBoxSize;
const scale = box[X] / interactiveSizes.defaultBoxSize;
return (
<View
style={{
Expand Down
Loading

0 comments on commit 7e6ccf3

Please sign in to comment.