Skip to content

[AbsoluteValue] PR5 - editor support #3352

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.

Sign up for GitHub

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

Merged
SonicScrewdriver merged 4 commits into from
Mar 23, 2026
Merged

[AbsoluteValue] PR5 - editor support #3352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11660f2
[LEMS-3347/absolute-value-pr5] Implement PR #5
handeyeco Mar 13, 2026
d53d5ef
[LEMS-3347/absolute-value-pr5] restore export
handeyeco Mar 13, 2026
166e08c
[LEMS-3347/absolute-value-pr5] docs(changeset): Add editor support fo…
handeyeco Mar 13, 2026
99494a9
[LEMS-3347/absolute-value-pr5] add feature flag
handeyeco Mar 18, 2026
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/lemon-cooks-judge.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

Add editor support for AbsoluteValue
8 changes: 8 additions & 0 deletions ...es/perseus-editor/src/widgets/interactive-graph-editor/components/graph-type-selector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ type GraphTypeSelectorProps = {
};

const GraphTypeSelector = (props: GraphTypeSelectorProps) => {
const showAbsoluteValue = isFeatureOn(
{apiOptions: props.apiOptions},
"interactive-graph-absolute-value",
);

const showTangent = isFeatureOn(
{apiOptions: props.apiOptions},
"interactive-graph-tangent",
Expand All @@ -24,6 +29,9 @@ const GraphTypeSelector = (props: GraphTypeSelectorProps) => {
placeholder="Select an answer type"
style={styles.singleSelectShort}
>
{showAbsoluteValue && (
<OptionItem value="absolute-value" label="Absolute value" />
)}
<OptionItem value="none" label="None" />
<OptionItem value="linear" label="Linear function" />
<OptionItem value="quadratic" label="Quadratic function" />
Expand Down
13 changes: 13 additions & 0 deletions ...erseus-editor/src/widgets/interactive-graph-editor/start-coords/start-coords-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {vector as kvector} from "@khanacademy/kmath";
import {
getAbsoluteValueCoords,
getAngleCoords,
getCircleCoords,
getLineCoords,
Expand Down Expand Up @@ -44,6 +45,18 @@ const StartCoordsSettingsInner = (props: Props) => {
const {type, range, step, allowReflexAngles, onChange} = props;

switch (type) {
case "absolute-value":
const absoluteValueCoords = getAbsoluteValueCoords(
props,
range,
step,
);
return (
<StartCoordsPoint
startCoords={absoluteValueCoords}
onChange={onChange}
/>
);
Comment on lines +48 to +59
Copy link
Copy Markdown

@claude claude Bot Mar 13, 2026

Choose a reason for hiding this comment

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

🟡 The absolute-value case uses StartCoordsPoint (which expects Coord[]) instead of StartCoordsLine (which expects CollinearTuple / [Coord, Coord]). Since getAbsoluteValueCoords returns [Coord, Coord], using StartCoordsLine would be more type-safe and consistent with how linear/ray handle the same structure.

Extended reasoning...

What the bug is

The new absolute-value case in StartCoordsSettingsInner (line 46-57) renders StartCoordsPoint to handle start coordinate editing. However, getAbsoluteValueCoords returns [Coord, Coord] (a fixed-length tuple), which is the same shape as CollinearTuple used by linear and ray graph types. StartCoordsPoint expects Coord[] (variable-length array) and is designed for point/polygon graphs.

How the type widening happens

StartCoordsPoint declares its props as { startCoords: Coord[]; onChange: (startCoords: Coord[]) => void }. Its onChange handler uses const newStartCoords = [...startCoords] (spread into a new array), then calls onChange(newStartCoords). This spread widens the TypeScript type from [Coord, Coord] to Coord[]. In contrast, StartCoordsLine declares { startCoords: CollinearTuple; onChange: (startCoords: CollinearTuple) => void } and its onChange constructs the tuple explicitly: onChange([value, startCoords[1]]), preserving the [Coord, Coord] type.

Step-by-step proof

  1. User opens the interactive graph editor and selects "Absolute value" as the graph type.
  2. StartCoordsSettingsInner matches case "absolute-value" (line 46).
  3. getAbsoluteValueCoords is called, returning a [Coord, Coord] value.
  4. This is passed to <StartCoordsPoint startCoords={absoluteValueCoords} onChange={onChange} />.
  5. User edits Point 1 — StartCoordsPoint.onChange fires with [...startCoords] (line 31 of start-coords-point.tsx), producing Coord[].
  6. The onChange prop propagates this Coord[] value upward, where the absolute-value graph expects [Coord, Coord].

For comparison, linear and ray (which have the identical [Coord, Coord] type) correctly use StartCoordsLine (lines 62-68), and sinusoid (also [Coord, Coord]) uses StartCoordsSinusoid.

Impact

This is not a runtime bug — at JavaScript runtime, a 2-element array is indistinguishable whether typed as [Coord, Coord] or Coord[], and the UI renders identically (both show "Point 1" / "Point 2" inputs). The onChange handler at line 170 is untyped, so the widening passes silently. However, it is a type-safety and consistency concern: downstream TypeScript code expecting [Coord, Coord] could receive Coord[], and this breaks the established pattern in the codebase.

How to fix

Replace StartCoordsPoint with StartCoordsLine in the absolute-value case, matching the pattern used by linear/ray:

case "absolute-value":
    const absoluteValueCoords = getAbsoluteValueCoords(props, range, step);
    return (
        <StartCoordsLine
            startCoords={absoluteValueCoords}
            onChange={onChange}
        />
    );

// Graphs with startCoords of type CollinearTuple
case "linear":
case "ray":
Expand Down
1 change: 1 addition & 0 deletions packages/perseus-editor/src/widgets/interactive-graph-editor/start-coords/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {PerseusGraphType} from "@khanacademy/perseus-core";

type GraphTypesThatHaveStartCoords =
| {type: "absolute-value"}
| {type: "angle"}
| {type: "circle"}
| {type: "linear"}
Expand Down
7 changes: 7 additions & 0 deletions packages/perseus-editor/src/widgets/interactive-graph-editor/start-coords/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {angles, vector as kvector} from "@khanacademy/kmath";
import {
getAbsoluteValueCoords,
getAngleCoords,
getCircleCoords,
getLineCoords,
Expand Down Expand Up @@ -31,6 +32,12 @@ export function getDefaultGraphStartCoords(
step: [x: number, y: number],
): StartCoords {
switch (graph.type) {
case "absolute-value":
return getAbsoluteValueCoords(
{...graph, startCoords: undefined},
range,
step,
);
case "linear":
case "ray":
return getLineCoords(
Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export {
} from "./widget-type-utils";
export {convertWidgetNameToEnum} from "./util/widget-enum-utils";
export {
getAbsoluteValueCoords,
getCircleCoords,
getLineCoords,
getLinearSystemCoords,
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ export function getSinusoidCoords(
return coords;
}

function getAbsoluteValueCoords(
export function getAbsoluteValueCoords(
graph: PerseusGraphTypeAbsoluteValue,
range: [x: Interval, y: Interval],
step: [x: number, y: number],
Expand Down
Loading