[Hint Mode: Start Coords] Add start coords UI for quadratic graphs #1469

nishasy · 2024-07-31T21:55:51Z

Summary:

Add the UI to specify start coords for Quadratic graph type.

Add the quadratic graph type to start-coord-settings.tsx
Create a start-coords-quadratic.tsx file with the main UI
Add a utility for getting the quadratic equation from coordinates (less complex
than the static method on InteractiveGraph that does the same)
Add quadratic graph to the phase 2 flag

Issue: https://khanacademy.atlassian.net/browse/LEMS-2210

Test plan:

yarn jest

Storybook

http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-quadratic-with-starting-coords

khan-actions-bot · 2024-07-31T21:57:13Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/grumpy-rivers-draw.md, packages/perseus/src/index.ts, packages/perseus-editor/src/components/start-coords-quadratic.tsx, packages/perseus-editor/src/components/start-coords-settings.tsx, packages/perseus-editor/src/components/util.ts, packages/perseus-editor/src/widgets/interactive-graph-editor.tsx, packages/perseus-editor/src/components/__tests__/start-coords-settings.test.tsx, packages/perseus-editor/src/components/__tests__/util.test.ts, packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2024-07-31T21:58:49Z

Size Change: +407 B (+0.05%)

Total Size: 852 kB

Filename	Size	Change
`packages/perseus-editor/dist/es/index.js`	272 kB	+398 B (+0.15%)
`packages/perseus/dist/es/index.js`	412 kB	+9 B (0%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.3 kB
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	80.8 kB
`packages/math-input/dist/es/strings.js`	1.73 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-linter/dist/es/index.js`	21.6 kB
`packages/perseus/dist/es/strings.js`	3.23 kB
`packages/pure-markdown/dist/es/index.js`	3.67 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

codecov · 2024-07-31T22:04:01Z

Codecov Report

Attention: Patch coverage is 96.57534% with 5 lines in your changes missing coverage. Please review.

Project coverage is 70.56%. Comparing base (af68a9e) to head (7adf247).
Report is 1 commits behind head on main.

Files	Patch %	Lines
packages/perseus-editor/src/components/util.ts	86.84%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1469      +/-   ##
==========================================
+ Coverage   69.93%   70.56%   +0.62%     
==========================================
  Files         508      512       +4     
  Lines      105287   105468     +181     
  Branches     5364    10771    +5407     
==========================================
+ Hits        73636    74422     +786     
+ Misses      31534    31046     -488     
+ Partials      117        0     -117

Files	Coverage Δ
...s-editor/src/components/start-coords-quadratic.tsx	`100.00% <100.00%> (ø)`
...us-editor/src/components/start-coords-settings.tsx	`98.54% <100.00%> (+0.10%)`	⬆️
...us-editor/src/widgets/interactive-graph-editor.tsx	`91.52% <100.00%> (ø)`
packages/perseus/src/index.ts	`100.00% <100.00%> (ø)`
...teractive-graphs/reducer/initialize-graph-state.ts	`83.25% <100.00%> (-10.41%)`	⬇️
packages/perseus-editor/src/components/util.ts	`77.64% <86.84%> (+9.90%)`	⬆️

... and 128 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af68a9e...7adf247. Read the comment docs.

mark-fitzgerald · 2024-07-31T22:27:09Z

packages/perseus-editor/src/components/start-coords-quadratic.tsx

+    );
+};
+
+const styles = StyleSheet.create({


NIT: Grrr. Same styling as all the other "start-coords-*" implementations.
No need to change anything in this PR.

Ah, I did admittedly copy/paste the entire sinusoid file and build off of it. I should probably break out Tile into a separate component. Maybe in a different PR.

mark-fitzgerald

LGTM

benchristel

LGTM!

benchristel · 2024-07-31T22:17:10Z

packages/perseus-editor/src/components/__tests__/util.test.ts

+        // Act
+        const equation = getQuadraticEquation([point1, point2, point3]);
+
+        expect(equation).toBe("Division by zero error");


This is an interesting case! Thanks for testing it.

I am curious: how does the graph preview render in this case? What happens to the graph when the start coords aren't valid?

It just shows all the points where they would be, but the quadratic graph doesn't go through the points. The equation box says "Division by zero error"

I'm not too worried about it because I think this is within the "trust the content authors" territory

benchristel · 2024-07-31T22:37:35Z

packages/perseus-editor/src/components/util.ts

+        (p2[0] * p3[0] * (p2[0] - p3[0]) * p1[1] +
+            p3[0] * p1[0] * (p3[0] - p1[0]) * p2[1] +
+            p1[0] * p2[0] * (p1[0] - p2[0]) * p3[1]) /
+        denom;


This is copied from InteractiveGraph.getQuadraticCoefficients?

I wish we had a central place where this logic could live, and both perseus and perseus-editor could import it from there. I think kmath is probably the best candidate that place. Would it be too hard to move getQuadraticCoefficients to kmath?

If you don't want to move it in this PR, I can move it once this is landed.

Yes, that would be awesome! Sorry, I didn't see your comment before landing.

@nishasy

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/perseus@27.2.0 ### Minor Changes - [#1468](#1468) [`af68a9e08`](af68a9e) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for sinusoid graphs - [#1469](#1469) [`6e1ec850c`](6e1ec85) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for quadratic graphs ### Patch Changes - [#1470](#1470) [`942b0a9a5`](942b0a9) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Locked Figures] Remove m2 flag from the code - [#1465](#1465) [`94ad04fee`](94ad04f) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add separate flags for graph types - [#1432](#1432) [`ed6737025`](ed67370) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Bug fix to ensure that new angle graphs are scored correctly. ## @khanacademy/perseus-editor@11.2.0 ### Minor Changes - [#1468](#1468) [`af68a9e08`](af68a9e) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for sinusoid graphs - [#1469](#1469) [`6e1ec850c`](6e1ec85) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for quadratic graphs ### Patch Changes - [#1470](#1470) [`942b0a9a5`](942b0a9) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Locked Figures] Remove m2 flag from the code - [#1465](#1465) [`94ad04fee`](94ad04f) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add separate flags for graph types - Updated dependencies \[[`af68a9e08`](af68a9e), [`942b0a9a5`](942b0a9), [`6e1ec850c`](6e1ec85), [`94ad04fee`](94ad04f), [`ed6737025`](ed67370)]: - @khanacademy/perseus@27.2.0

[Hint Mode: Start Coords] Add start coords UI for quadratic graphs

7adf247

nishasy self-assigned this Jul 31, 2024

nishasy marked this pull request as ready for review July 31, 2024 21:56

nishasy requested review from benchristel, mark-fitzgerald and a team July 31, 2024 21:57

mark-fitzgerald reviewed Jul 31, 2024

View reviewed changes

mark-fitzgerald approved these changes Jul 31, 2024

View reviewed changes

benchristel approved these changes Jul 31, 2024

View reviewed changes

nishasy merged commit 6e1ec85 into main Jul 31, 2024
12 checks passed

nishasy deleted the start-coords-quadratic branch July 31, 2024 22:38

khan-actions-bot mentioned this pull request Jul 31, 2024

Version Packages #1467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Hint Mode: Start Coords] Add start coords UI for quadratic graphs #1469

[Hint Mode: Start Coords] Add start coords UI for quadratic graphs #1469

nishasy commented Jul 31, 2024

khan-actions-bot commented Jul 31, 2024

github-actions bot commented Jul 31, 2024

codecov bot commented Jul 31, 2024

mark-fitzgerald Jul 31, 2024

nishasy Jul 31, 2024

mark-fitzgerald left a comment

benchristel left a comment

benchristel Jul 31, 2024

nishasy Jul 31, 2024

nishasy Jul 31, 2024

benchristel Jul 31, 2024

nishasy Jul 31, 2024

[Hint Mode: Start Coords] Add start coords UI for quadratic graphs #1469

[Hint Mode: Start Coords] Add start coords UI for quadratic graphs #1469

Conversation

nishasy commented Jul 31, 2024

Summary:

Test plan:

khan-actions-bot commented Jul 31, 2024

Gerald

github-actions bot commented Jul 31, 2024

codecov bot commented Jul 31, 2024

Codecov Report

mark-fitzgerald Jul 31, 2024

Choose a reason for hiding this comment

nishasy Jul 31, 2024

Choose a reason for hiding this comment

mark-fitzgerald left a comment

Choose a reason for hiding this comment

benchristel left a comment

Choose a reason for hiding this comment

benchristel Jul 31, 2024

Choose a reason for hiding this comment

nishasy Jul 31, 2024

Choose a reason for hiding this comment

nishasy Jul 31, 2024

Choose a reason for hiding this comment

benchristel Jul 31, 2024

Choose a reason for hiding this comment

nishasy Jul 31, 2024

Choose a reason for hiding this comment