[Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) #1382

nishasy · 2024-06-27T23:26:16Z

Summary:

Implement a UI in the interactive graph editor to allow conten authors
to set custom starting coordinates for interactive graphs. This is a
prerequisite for hint mode.

Note: There was some funky stuff happening when I tried using coords.
I added a startCoords field to the graph types, and that seemed to fix
all my issues. This change is included in this PR.

Included in this PR

The creation of the StartCoordSettings component
The flag that this feature is behind
The UI itself, only for linear and ray graphs
Added startCoords field

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

Test plan:

Storybook existing starting coords

http://localhost:6006/?path=/story/perseuseditor-editorpage--interactive-graph-linear-with-starting-coords
Confirm that the UI reflects the custom coords
Change the coords
Confirm that the graph changes
Change the graph type to Ray
Confirm that the coords change to the ray initial state coords
Confirm that the "correct" preview is still interact-able and does not interfere with the start coords preview
Press "Use default start coords" and confirm it uses the start coords

Storybook new graph

http://localhost:6006/?path=/story/perseuseditor-editorpage--demo
Add a new interactive graph
Select the linear graph type
Confirm that it sets the default coords in the UI
Confirm that the "correct" preview is still interact-able and does not interfere with the start coords preview

Copy/pasting

Press the "+ Add a hint" button at the bottom of the page
Copy/paste the interactive graph ("[[interactive-graph 1]]")
into the hint text box
Confirm that the graph is copied correctly with the same
start coords
Confirm that the "correct" preview is still interact-able and does not interfere with the start coords preview
Press "Use default start coords" and confirm it uses the start coords

github-actions · 2024-06-27T23:29:04Z

Size Change: +936 B (+0.11%)

Total Size: 854 kB

Filename	Size	Change
`packages/perseus-editor/dist/es/index.js`	273 kB	+798 B (+0.29%)
`packages/perseus/dist/es/index.js`	414 kB	+138 B (+0.03%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.2 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.21 kB
`packages/pure-markdown/dist/es/index.js`	3.67 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

khan-actions-bot · 2024-06-28T22:22:41Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/chilly-planets-walk.md, packages/perseus/src/index.ts, packages/perseus/src/perseus-types.ts, packages/perseus/src/types.ts, packages/perseus-editor/src/__stories__/flags-for-api-options.ts, packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx, packages/perseus-editor/src/components/coordinate-pair-input.tsx, packages/perseus-editor/src/components/start-coord-settings.tsx, packages/perseus-editor/src/widgets/interactive-graph-editor.tsx, packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts, packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts, packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx, packages/perseus-editor/src/components/__tests__/start-coord-settings.test.tsx, 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-06-29T00:10:39Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (a741a28) and published it to npm. You
can install it using the tag PR1382.

Example:

yarn add @khanacademy/perseus@PR1382

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1382

codecov · 2024-06-29T00:14:04Z

Codecov Report

Attention: Patch coverage is 83.10502% with 37 lines in your changes missing coverage. Please review.

Project coverage is 71.09%. Comparing base (a8a1a0f) to head (a741a28).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1382      +/-   ##
==========================================
+ Coverage   69.78%   71.09%   +1.31%     
==========================================
  Files         503      508       +5     
  Lines      105061   105355     +294     
  Branches     7563    10771    +3208     
==========================================
+ Hits        73315    74903    +1588     
+ Misses      31630    30452    -1178     
+ Partials      116        0     -116

Files	Coverage Δ
...us-editor/src/components/coordinate-pair-input.tsx	`100.00% <100.00%> (ø)`
packages/perseus/src/index.ts	`100.00% <100.00%> (ø)`
...ctive-graphs/interactive-graph-question-builder.ts	`98.55% <100.00%> (ø)`
...widgets/interactive-graphs/stateful-mafs-graph.tsx	`98.42% <100.00%> (-1.58%)`	⬇️
...eus-editor/src/components/start-coord-settings.tsx	`97.52% <97.52%> (ø)`
...teractive-graphs/reducer/initialize-graph-state.ts	`94.30% <69.23%> (-2.21%)`	⬇️
...us-editor/src/widgets/interactive-graph-editor.tsx	`89.29% <47.61%> (-2.32%)`	⬇️

... and 124 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 a8a1a0f...a741a28. Read the comment docs.

…ests.

nishasy · 2024-06-29T00:56:12Z

packages/perseus-editor/src/widgets/interactive-graph-editor.tsx

@@ -254,12 +255,18 @@ class InteractiveGraphEditor extends React.Component<Props> {
                    let correct = this.props.correct;
                    // @ts-expect-error - TS2532 - Object is possibly 'undefined'.
                    if (correct.type === newProps.graph.type) {
-                        correct = _.extend({}, correct, newProps.graph);


lodash removal

nishasy · 2024-06-29T00:56:28Z

packages/perseus-editor/src/widgets/interactive-graph-editor.tsx

+                    this.props.onChange({
+                        correct: correct,
+                        graph: this.props.graph,
+                    });


Added graph here because it's required for the update to happen correctly.

…startCoords as a separate prop

catandthemachines

Overall test coverage and implementation looks pretty clean and straight forward to me, Approved! Though highly recommend a review from a fellow interactive graph peep ;)

catandthemachines · 2024-07-02T00:58:59Z

packages/perseus-editor/src/widgets/interactive-graph-editor.tsx

+                graph: {
+                    type: correct.type,
+                    coords:
+                        this.props.graph?.type === "circle"


I'm very curious, why do these property need to be exposed now for the starting coordinates work? (non-blocking question 😉)

This is the serialization function, which is used to copy/paste the graph across editors. Before, coords was never being used, because it was never part of the initial state. It was always undefined, and then it got set as the user would start interacting with it.

In this PR, I'm trying to set initial specific coordinates on the graph, and it needs to be copy/pastable in that state, which is why I now needed to add coords here when it wasn't there before.

I'll add a comment here for clarity!

catandthemachines · 2024-07-02T01:08:57Z

Just confirming, it only meant to work once in Storybook? It uses the default coordinates, but if I move it again it doesn't work to more to default. Might need to check it out.

https://650db21c3f5d1b2f13c02952-jijqmcqxid.chromatic.com/?path=/story/perseuseditor-editorpage--interactive-graph-linear-with-starting-coords

benchristel · 2024-07-01T23:11:53Z

packages/perseus-editor/src/widgets/interactive-graph-editor.tsx

+        };
+        this.props.onChange({graph: graph});
+    };
+
    serialize(): PerseusInteractiveGraphWidgetOptions {


Where is serialize called? GoLand's "find usages" feature isn't turning up anything. I was also able to delete serialize and got no type errors.

Asking because there's a change to this function (below).

I think it's used here inside _maybeCopyWidgets.

It's also called by the save mechanism in webapp by calling serialize() on a ref to the EditorPage component. The call stack is essentially:

EditorPage

ItemEditor

Editor (which is named questionEditor in the ItemEditor code.

The Editor then calls serialize on each widget (through a ref handle).

…'t need startCoords as a separate prop" This reverts commit 16095ae. Turns out we can't use the same coords after all

…, we don't need startCoords as a separate prop"" This reverts commit 71081af (which reverts commit 16095ae).

jeremywiebe

Agree with @catandthemachines. LGTM!

packages/perseus-editor/src/components/__tests__/start-coord-settings.test.tsx

…ettings.test.tsx Co-authored-by: Jeremy Wiebe <[email protected]>

nishasy · 2024-07-08T23:18:02Z

Talked about @catandthemachines's comment offline - that behavior is expected. The preview on the right is not a wysiwyg for the starting coords. When you update the starting coords, it updates the starting state and thus displays it in the preview. But that preview is still only showing the learner experience.

…rlier PR, we don't need startCoords as a separate prop""" This reverts commit 64eb1ba. Adding back `startCoords`

@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/[email protected] ### Minor Changes - [#1382](#1382) [`f392dcfba`](f392dcf) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Implement UI to edit start coordinates for linear and ray graphs ## @khanacademy/[email protected] ### Minor Changes - [#1382](#1382) [`f392dcfba`](f392dcf) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Implement UI to edit start coordinates for linear and ray graphs ### Patch Changes - Updated dependencies \[[`f392dcfba`](f392dcf)]: - @khanacademy/[email protected] Author: khan-actions-bot Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1414

jeremywiebe · 2024-07-24T01:32:45Z

packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.tsx

+        startCoords,
        latestPropsRef,
+        startCoords,


@nishasy Was this intentional to list startCoords twice here? :)

[Interactive Graph Editor] Add UI for updating starting coordinates

5e1a4ea

nishasy self-assigned this Jun 27, 2024

nishasy added 2 commits June 28, 2024 15:12

Merge branch 'main' into start-coords-linear

8512b2b

Stop preview from resetting on correct change. remove lodash

d93740b

nishasy changed the title ~~[Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray)~~ WIP!!!! [Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) Jun 28, 2024

nishasy marked this pull request as ready for review June 28, 2024 22:22

khan-actions-bot requested a review from a team June 28, 2024 22:22

nishasy marked this pull request as draft June 28, 2024 22:55

Remove console logs

8e85884

nishasy marked this pull request as ready for review June 28, 2024 23:46

nishasy added 2 commits June 28, 2024 17:03

Use startCoords instead of coords

0239a51

Add reset button

49b6f3f

nishasy added 2 commits June 28, 2024 17:16

Make copying work

eada628

Move the button so that it's hidden when the section is closed. Add t…

238f675

…ests.

nishasy changed the title ~~WIP!!!! [Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray)~~ [Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) Jun 29, 2024

nishasy requested review from benchristel, SonicScrewdriver and Myranae June 29, 2024 00:53

remove random space

e715c00

nishasy commented Jun 29, 2024

View reviewed changes

nishasy added 2 commits July 1, 2024 14:46

Merge branch 'main' into start-coords-linear

5aa91e5

Now that the remount issue was fixed in an earlier PR, we don't need …

16095ae

…startCoords as a separate prop

catandthemachines approved these changes Jul 2, 2024

View reviewed changes

nishasy added 2 commits July 2, 2024 10:01

Add clarifying comment

57f17d5

Merge branch 'main' into start-coords-linear

3415fef

benchristel reviewed Jul 2, 2024

View reviewed changes

Revert "Now that the remount issue was fixed in an earlier PR, we don…

71081af

…'t need startCoords as a separate prop" This reverts commit 16095ae. Turns out we can't use the same coords after all

khan-actions-bot requested a review from a team July 2, 2024 18:29

nishasy requested a review from benchristel July 2, 2024 20:45

nishasy added 2 commits July 8, 2024 14:31

Merge branch 'main' into start-coords-linear

df622b3

Revert "Revert "Now that the remount issue was fixed in an earlier PR…

64eb1ba

…, we don't need startCoords as a separate prop"" This reverts commit 71081af (which reverts commit 16095ae).

jeremywiebe approved these changes Jul 8, 2024

View reviewed changes

packages/perseus-editor/src/components/__tests__/start-coord-settings.test.tsx Outdated Show resolved Hide resolved

Update packages/perseus-editor/src/components/__tests__/start-coord-s…

dbf2961

…ettings.test.tsx Co-authored-by: Jeremy Wiebe <[email protected]>

benchristel approved these changes Jul 9, 2024

View reviewed changes

nishasy added 2 commits July 15, 2024 11:01

Revert "Revert "Revert "Now that the remount issue was fixed in an ea…

ff136ad

…rlier PR, we don't need startCoords as a separate prop""" This reverts commit 64eb1ba. Adding back `startCoords`

Merge branch 'main' into start-coords-linear

a741a28

nishasy merged commit f392dcf into main Jul 15, 2024
11 checks passed

nishasy deleted the start-coords-linear branch July 15, 2024 18:19

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

Version Packages #1414

Merged

jeremywiebe reviewed Jul 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) #1382

[Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) #1382

nishasy commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

khan-actions-bot commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jun 29, 2024 •

edited

Loading

codecov bot commented Jun 29, 2024 •

edited

Loading

nishasy Jun 29, 2024

nishasy Jun 29, 2024

catandthemachines left a comment

catandthemachines Jul 2, 2024

nishasy Jul 2, 2024

catandthemachines commented Jul 2, 2024

benchristel Jul 1, 2024 •

edited

Loading

nishasy Jul 2, 2024

jeremywiebe Jul 8, 2024

jeremywiebe left a comment

nishasy commented Jul 8, 2024

jeremywiebe Jul 24, 2024

[Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) #1382

[Interactive Graph Editor] Add UI for updating starting coordinates (linear and ray) #1382

Conversation

nishasy commented Jun 27, 2024 • edited Loading

Summary:

Test plan:

github-actions bot commented Jun 27, 2024 • edited Loading

khan-actions-bot commented Jun 28, 2024 • edited Loading

Gerald

github-actions bot commented Jun 29, 2024 • edited Loading

npm Snapshot: Published

codecov bot commented Jun 29, 2024 • edited Loading

Codecov Report

nishasy Jun 29, 2024

Choose a reason for hiding this comment

nishasy Jun 29, 2024

Choose a reason for hiding this comment

catandthemachines left a comment

Choose a reason for hiding this comment

catandthemachines Jul 2, 2024

Choose a reason for hiding this comment

nishasy Jul 2, 2024

Choose a reason for hiding this comment

catandthemachines commented Jul 2, 2024

benchristel Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

nishasy Jul 2, 2024

Choose a reason for hiding this comment

jeremywiebe Jul 8, 2024

Choose a reason for hiding this comment

jeremywiebe left a comment

Choose a reason for hiding this comment

nishasy commented Jul 8, 2024

jeremywiebe Jul 24, 2024

Choose a reason for hiding this comment

nishasy commented Jun 27, 2024 •

edited

Loading

github-actions bot commented Jun 27, 2024 •

edited

Loading

khan-actions-bot commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jun 29, 2024 •

edited

Loading

codecov bot commented Jun 29, 2024 •

edited

Loading

benchristel Jul 1, 2024 •

edited

Loading