Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kmath-radian-to-degree] Add a convertRadiansToDegrees function to kmath #2190

Merged
merged 2 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/forty-foxes-brush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/kmath": patch
"@khanacademy/perseus-editor": patch
---

Add convertRadiansToDegrees function to kmath
44 changes: 43 additions & 1 deletion packages/kmath/src/angles.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import {calculateAngleInDegrees} from "./angles";
import {
calculateAngleInDegrees,
convertDegreesToRadians,
convertRadiansToDegrees,
} from "./angles";

describe("calculateAngleInDegrees", () => {
it.each`
Expand All @@ -15,3 +19,41 @@ describe("calculateAngleInDegrees", () => {
expect(calculateAngleInDegrees([x, y])).toBe(expected);
});
});

describe("convertDegreesToRadians", () => {
test.each`
degrees | radians
${0} | ${0}
${45} | ${Math.PI / 4}
${90} | ${Math.PI / 2}
${180} | ${Math.PI}
${270} | ${Math.PI * 1.5}
${360} | ${Math.PI * 2}
${-45} | ${-Math.PI / 4}
${-90} | ${-Math.PI / 2}
`(
"should convert $degrees degrees to $radians radians",
({degrees, radians}) => {
expect(convertDegreesToRadians(degrees)).toBe(radians);
},
);
});

describe("convertRadiansToDegrees", () => {
test.each`
radians | degrees
${0} | ${0}
${Math.PI / 4} | ${45}
${Math.PI / 2} | ${90}
${Math.PI} | ${180}
${Math.PI * 1.5} | ${270}
${Math.PI * 2} | ${360}
${-Math.PI / 4} | ${-45}
${-Math.PI / 2} | ${-90}
`(
"should convert $radians radians to $degrees degrees",
({radians, degrees}) => {
expect(convertRadiansToDegrees(radians)).toBe(degrees);
},
);
});
6 changes: 6 additions & 0 deletions packages/kmath/src/angles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ export function convertDegreesToRadians(degrees: number): number {
return (degrees / 180) * Math.PI;
}

export function convertRadiansToDegrees(radians: number): number {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action needed, but I was musing on naming in this file. We have calculate, convert, get, and bare action naming.

I think I'd be inclined to remove the prefixes as they don't really add much value (is there a difference between "calculating" or "converting" or "getting" a value given another value? It's all a transformation/calculation.

That would leave us with an angles API like this:

  • degreesToRadians(degrees: number): number
  • radiansToDegrees(radians: number): number
  • angleToDegrees([x, y]: Coord): number
  • angleFromVertex...
  • clockwiseAngle...

Not asking you to change anything, but I feel having a consistent naming convention for these types of things is useful. :) Dig in if you feel motivated and agree though!

const degree = (radians / Math.PI) * 180;
// Account for floating point errors.
return Number(degree.toPrecision(15));
}

// Returns a value between -180 and 180, inclusive. The angle is measured
// between the positive x-axis and the given vector.
export function calculateAngleInDegrees([x, y]: Coord): number {
Expand Down
39 changes: 0 additions & 39 deletions packages/perseus-editor/src/components/__tests__/util.test.ts

This file was deleted.

8 changes: 5 additions & 3 deletions packages/perseus-editor/src/components/angle-input.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import {angles} from "@khanacademy/kmath";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import {spacing} from "@khanacademy/wonder-blocks-tokens";
import {LabelMedium} from "@khanacademy/wonder-blocks-typography";
import {StyleSheet} from "aphrodite";
import * as React from "react";

import ScrolllessNumberTextField from "./scrollless-number-text-field";
import {degreeToRadian, radianToDegree} from "./util";

const {convertDegreesToRadians, convertRadiansToDegrees} = angles;

type Props = {
angle: number;
Expand All @@ -16,7 +18,7 @@ const AngleInput = (props: Props) => {
const {angle, onChange} = props;

const [angleInput, setAngleInput] = React.useState(
radianToDegree(angle).toString(),
convertRadiansToDegrees(angle).toString(),
);

function handleAngleChange(newValue) {
Expand All @@ -30,7 +32,7 @@ const AngleInput = (props: Props) => {
}

// Update the graph.
onChange(degreeToRadian(newValue));
onChange(convertDegreesToRadians(newValue));
}

return (
Expand Down
7 changes: 0 additions & 7 deletions packages/perseus-editor/src/components/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,3 @@ export function focusWithChromeStickyFocusBugWorkaround(element: Element) {
// @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'Element'.
element.focus({preventScroll: true});
}

export function degreeToRadian(degrees: number) {
return (degrees / 180) * Math.PI;
}
export function radianToDegree(radians: number) {
return (radians / Math.PI) * 180;
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {angles} from "@khanacademy/kmath";
import {components} from "@khanacademy/perseus";
import {lockedFigureFillStyles} from "@khanacademy/perseus-core";
import Button from "@khanacademy/wonder-blocks-button";
Expand All @@ -13,7 +14,6 @@ import * as React from "react";
import AngleInput from "../../../components/angle-input";
import CoordinatePairInput from "../../../components/coordinate-pair-input";
import PerseusEditorAccordion from "../../../components/perseus-editor-accordion";
import {radianToDegree} from "../../../components/util";

import ColorSelect from "./color-select";
import EllipseSwatch from "./ellipse-swatch";
Expand All @@ -37,6 +37,7 @@ import type {
LockedLabelType,
} from "@khanacademy/perseus-core";

const {convertRadiansToDegrees} = angles;
const {InfoTip} = components;

export type Props = LockedFigureSettingsCommonProps &
Expand Down Expand Up @@ -74,7 +75,7 @@ const LockedEllipseSettings = (props: Props) => {
const spokenCenterX = await generateSpokenMathDetails(`$${center[0]}$`);
const spokenCenterY = await generateSpokenMathDetails(`$${center[1]}$`);
const spokenRotation = await generateSpokenMathDetails(
`$${radianToDegree(angle)}$`,
`$${convertRadiansToDegrees(angle)}$`,
);

const isCircle = radius[0] === radius[1];
Expand Down