[Interactive Graph] POC Add logarithm graph in Interactive Graph widget#3322
Draft
[Interactive Graph] POC Add logarithm graph in Interactive Graph widget#3322
Conversation
github-actions
bot
added
the
schema-change
Contributor
🗄️ Schema Change: Changes Detected
|
Contributor
|
Size Change: +4.65 kB (+0.96%) Total Size: 491 kB
ℹ️ View Unchanged
|
Contributor
🛠️ Item Splitting: Changes Detected
|
Contributor
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (1d208b0) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3322If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3322If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3322 |
ivyolamit
force-pushed
the
LEMS-3950/poc-logarithm-interactive-graph
branch
2 times, most recently
from
96dafe6 to
915a9d7
Compare
ivyolamit
commented
Mar 11, 2026
| const FOCUS_RING_PAD = 2; | ||
| const FOCUS_RING_STROKE = 2; | ||
|
|
||
| function AsymptoteDragHandle(props: { |
Contributor
Author
There was a problem hiding this comment.
add this in packages/perseus/src/widgets/interactive-graphs/graphs/components/
ivyolamit
commented
Mar 11, 2026
Contributor
Author
There was a problem hiding this comment.
This file is very long, this should only contain the LogarithmGraph move other logic in its own component or util file. Maybe have packages/perseus/src/widgets/interactive-graphs/graphs/components/logarithm/ folder to contain all files and logic related to logarithm.
ivyolamit
commented
Mar 12, 2026
| <OptionItem value="linear" label="Linear function" /> | ||
| <OptionItem value="quadratic" label="Quadratic function" /> | ||
| <OptionItem value="sinusoid" label="Sinusoid function" /> | ||
| <OptionItem value="logarithm" label="Logarithm function" /> |
Contributor
Author
There was a problem hiding this comment.
Hide this behind a feature flag
ivyolamit
commented
Mar 12, 2026
| }; | ||
| } | ||
|
|
||
| if (state.type === "logarithm" && initialGraph.type === "logarithm") { |
Contributor
Author
There was a problem hiding this comment.
Base on new-graph-type.md guide
Make sure this is added before the final throw at the bottom of the function.
ivyolamit
commented
Mar 12, 2026
|
|
||
| static getLogarithmEquationString(props: Props): string { | ||
| const coords = | ||
| // @ts-expect-error - TS2339 - Property 'coords' does not exist on type 'PerseusGraphType'. |
Contributor
Author
There was a problem hiding this comment.
fix this in actual implementation
ivyolamit
commented
Mar 12, 2026
| // @ts-expect-error - TS2339 - Property 'coords' does not exist on type 'PerseusGraphType'. | ||
| props.userInput.coords || | ||
| InteractiveGraph.defaultLogarithmCoords(props); | ||
| // @ts-expect-error - TS2339 - Property 'asymptote' does not exist on type 'PerseusGraphType'. |
Contributor
Author
There was a problem hiding this comment.
fix this in actual implementation
ivyolamit
commented
Mar 12, 2026
| [0.55, 0.5], | ||
| [0.75, 0.75], | ||
| ]; | ||
| // @ts-expect-error - TS2345 - Argument of type 'number[][]' is not assignable to parameter of type 'readonly Coord[]'. |
Contributor
Author
There was a problem hiding this comment.
fix this in actual implementation
ivyolamit
commented
Mar 12, 2026
| }; | ||
| } | ||
|
|
||
| // Compute logarithm coefficients from two points and an asymptote. |
Contributor
Author
There was a problem hiding this comment.
This file is already getting long, move this in a different file for code maintainability.
ivyolamit
commented
Mar 12, 2026
Contributor
Author
There was a problem hiding this comment.
Missing logic based on guide in start-coords-settings.tsx
🔗 https://github.com/Khan/perseus/blob/main/packages/perseus/src/widgets/interactive-graphs/__docs__/notes/new-graph-type.md#11-add-start-coords-support-in-the-editor
ivyolamit
commented
Mar 12, 2026
| type: props.userInput.type, | ||
| startCoords: props.userInput.startCoords, | ||
| }; | ||
| case "logarithm": |
Contributor
Author
There was a problem hiding this comment.
Hide this behind a feature flag
…logarithm answer type in Interactive Graph
…dd logarithm graph in Interactive Graph widget
1. Keyboard constraint now enforces same-side-of-asymptote — Added check (coord[X] > asymptoteX) !== otherPointSide to isValidPosition(). Points can no longer arrow-key past the asymptote.
2. Keyboard constraint no longer risks infinite loop — Replaced the two separate if-then-skip checks with a unified isValidPosition() validator inside a bounded loop (max 3 attempts). If no valid position is found, the point stays in place instead of returning an invalid coordinate.
3. Stale comment fixed — "Draggable dashed vertical asymptote line" → "Draggable vertical asymptote line"
Accessibility Fixed
1. Asymptote aria-label is now localized — Added srLogarithmAsymptote string to strings.ts (both translatable definition and mock). The label now reads: "Vertical asymptote at x equals {x}. Use left and right arrow keys to move." — formatted with srFormatNumber for locale support.
2. Asymptote position changes are now announced — Added aria-live="polite" to the asymptote <g> element. When the aria-label updates on move, screen readers will announce the new position.
3. role="button" kept as-is — After checking the codebase, all draggable elements (movable lines, points, circles) consistently use role="button". Changing to role="slider" would break the established pattern.
…the keyboard navigation, the drag handle cannot move cross the curve
…ng the curve to the opposite side of the asymptote
ivyolamit
force-pushed
the
LEMS-3950/poc-logarithm-interactive-graph
branch
from
2a771f6 to
1d208b0
Compare
ivyolamit
commented
Mar 13, 2026
| // would land between or on the curve points, snap past all points in the | ||
| // movement direction. This mirrors the reducer's snap-through logic but | ||
| // uses explicit direction (keyboard always moves one step at a time). | ||
| const constrainAsymptoteKeyboard = ( |
Contributor
Author
There was a problem hiding this comment.
move this in a util file
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
POC Add logarithm answer type in Interactive Graph
f(x) = a * ln(b * x + c)with two movable curve points and a fully draggable vertical asymptoteKey features
useDraggable+SVGLinepattern asMovableLine, with a pill-shaped drag handle for visual affordancePlot.OfXwith a restricteddomainprop (no segment-splitting workaround needed, unlike tangent)approximateDeepEqualcomparison of[a, b, c]coefficients (no canonical normalization needed)Files changed
perseus-core/data-schema.ts, parser, generatorgraphs/logarithm.tsx(new — 430 lines)doMovePoint+doMoveCenter), init, statescore-interactive-graph.tswithgetLogarithmCoeffs()mafs-graph.tsx,interactive-graph.tsx, state-to-graph conversionstrings.ts(5 new SR strings), AI utilsIssue: LEMS-3950
Co-Authored by Claude (Opus)
Test plan: