View a locked point with mafs #1067

nishasy · 2024-03-12T23:03:09Z

Summary:

The ability to view a locked points was added as part of
LEMS-1702, but now that we've
switched to mafs, we need to implement viewing a point again with mafs.

Add a layer for locked figures to the mafs graph
Createa a new component for said locked feature
Add an interactive graph story to confirm that locked points can be viewed

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

Test plan:

Open http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--segment-with-mafs-and-locked-points
Confirm that red points are there at (-7, -7) and (2, -5)
Drag the points of the line segment over the locked points and confirm that
the blue points sit on top of the red points. (Check the interactive layer
is on top of the locked layer.)

Story

The ability to view a locked points was added as part of [LEMS-1702](https://khanacademy.atlassian.net/browse/LEMS-1702), but now that we've switched to mafs, we need to implement viewing a point again with mafs. - Add a layer for locked figures to the mafs graph - Createa a new component for said locked feature - Add an interactive graph story to confirm that locked points can be viewed Issue: https://khanacademy.atlassian.net/browse/LEMS-1702 Test plan: - Open http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--segment-with-mafs-and-locked-points - Confirm that red points are there at (-7, -7) and (2, -5) - Drag the points of the line segment over the locked points and confirm that the blue points sit on top of the red points. (Check the interactive layer is on top of the locked layer.)

github-actions · 2024-03-12T23:05:49Z

Size Change: +223 B (0%)

Total Size: 810 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	385 kB	+223 B (0%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.1 kB
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	80.4 kB
`packages/perseus-core/dist/es/index.js`	908 B
`packages/perseus-editor/dist/es/index.js`	263 kB
`packages/perseus-error/dist/es/index.js`	878 B
`packages/perseus-linter/dist/es/index.js`	21.8 kB
`packages/pure-markdown/dist/es/index.js`	3.69 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

codecov · 2024-03-12T23:10:58Z

Codecov Report

Attention: Patch coverage is 83.01887% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 68.52%. Comparing base (881da72) to head (1ffe5b5).

❗ Current head 1ffe5b5 differs from pull request most recent head 19b34f8. Consider uploading reports for the commit 19b34f8 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1067      +/-   ##
==========================================
+ Coverage   67.25%   68.52%   +1.27%     
==========================================
  Files         435      437       +2     
  Lines       96468    96515      +47     
  Branches     6685    10259    +3574     
==========================================
+ Hits        64879    66138    +1259     
+ Misses      31589    30377    -1212

Files	Coverage Δ
...widgets/__testdata__/interactive-graph.testdata.ts	`100.00% <100.00%> (ø)`
...seus/src/widgets/interactive-graphs/mafs-graph.tsx	`93.04% <100.00%> (+0.59%)`	⬆️
.../widgets/interactive-graphs/graph-locked-layer.tsx	`78.57% <78.57%> (ø)`

... and 102 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 881da72...19b34f8. Read the comment docs.

benchristel

The code looks good! The graph in the screenshot doesn't look quite right though—I wonder if you need to import the Mafs CSS? Something like this:

import "mafs/core.css";
import "./mafs-styles.css";

I also left a couple suggestions inline.

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

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx

packages/perseus/src/widgets/__tests__/interactive-graph.test.ts

…eCaseError.

khan-actions-bot · 2024-03-13T01:33:42Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/shaggy-parrots-provide.md, packages/perseus/src/widgets/interactive-graph.tsx, packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx, packages/perseus/src/widgets/__testdata__/interactive-graph.testdata.ts, packages/perseus/src/widgets/__tests__/interactive-graph.test.ts, packages/perseus/src/widgets/interactive-graphs/graph-locked-layer.tsx, packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx, packages/perseus/src/widgets/interactive-graphs/mafs-styles.css, packages/perseus-editor/src/widgets/__stories__/interactive-graph-editor.stories.tsx

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

jeremywiebe

This is really cool...especially how little code you had to write!

packages/perseus/src/perseus-types.ts

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx

jeremywiebe · 2024-03-13T03:37:55Z

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx

@@ -0,0 +1,41 @@
+import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core";


nedredmond

+1 to @jeremywiebe's comment about keeping props readonly. Otherwise, looks great!

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx

…ph-locked-layer.

github-actions · 2024-03-13T16:50:25Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1067

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

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

…led story. Stop the bug where that story would break on input field change.

… type

We changed it (accidentally) to offBlack16 in #1067. This commit fixes it. I removed the `--mafs-line-color` custom property because it's no longer used. Issue: none Test plan: View a Mafs segment graph in the dev UI (`yarn dev`). Verify that the segment is a darker gray than the gridlines.

## Summary: We changed it (accidentally) to offBlack16 in #1067. This commit fixes it. Issue: none Test plan: View the Mafs segment graph in Storybook (http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--segment-with-mafs-and-locked-points). Verify that the segment is a darker gray than the gridlines. <img width="472" alt="Screen Shot 2024-03-13 at 6 11 27 PM" src="https://github.com/Khan/perseus/assets/693920/855b2366-8b97-46cd-9811-4d23d5d73755"> Author: benchristel Reviewers: jeremywiebe, nedredmond, nishasy, mark-fitzgerald Required Reviewers: Approved By: jeremywiebe Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Extract i18n strings (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1073

@benchristel

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-dev-ui@1.3.0 ### Minor Changes - [#1071](#1071) [`eb637b35`](eb637b3) Thanks [@benchristel](https://github.com/benchristel)! - Add a "check answer" button to interactive graphs displayed in the dev flipbook UI ## @khanacademy/perseus@20.2.0 ### Minor Changes - [#1067](#1067) [`6196375a`](6196375) Thanks [@nishasy](https://github.com/nishasy)! - Can pass a point into `lockedFigures` into MafsGraph and a point will be displayed. - [#1074](#1074) [`a263e940`](a263e94) Thanks [@nishasy](https://github.com/nishasy)! - Add "add a locked figure" UI to interactive graph editor + adding points (mafs graphs only) ### Patch Changes - [#1068](#1068) [`881da724`](881da72) Thanks [@benchristel](https://github.com/benchristel)! - Adjust styling of the movable points and line segments on the Mafs segment graph - [#1071](#1071) [`eb637b35`](eb637b3) Thanks [@benchristel](https://github.com/benchristel)! - Add a "check answer" button to interactive graphs displayed in the dev flipbook UI - [#1073](#1073) [`ea0db7d9`](ea0db7d) Thanks [@benchristel](https://github.com/benchristel)! - Fix color of Mafs line segments ## @khanacademy/perseus-editor@5.1.0 ### Minor Changes - [#1074](#1074) [`a263e940`](a263e94) Thanks [@nishasy](https://github.com/nishasy)! - Add "add a locked figure" UI to interactive graph editor + adding points (mafs graphs only) ### Patch Changes - Updated dependencies \[[`881da724`](881da72), [`eb637b35`](eb637b3), [`6196375a`](6196375), [`a263e940`](a263e94), [`ea0db7d9`](ea0db7d)]: - @khanacademy/perseus@20.2.0 Author: khan-actions-bot Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Extract i18n strings (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x) Pull Request URL: #1069

nishasy self-assigned this Mar 12, 2024

nishasy requested review from nedredmond, benchristel and a team March 12, 2024 23:03

Add changeset

a8d4e30

benchristel reviewed Mar 12, 2024

View reviewed changes

packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx Outdated Show resolved Hide resolved

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx Outdated Show resolved Hide resolved

add tests

29da3de

nishasy commented Mar 13, 2024

View reviewed changes

packages/perseus/src/widgets/__tests__/interactive-graph.test.ts Show resolved Hide resolved

nishasy added 3 commits March 12, 2024 18:23

Use regular array for lockedFigures. Use wonder-stuff-core Unreachabl…

758df60

…eCaseError.

reset snapshot test

1455c8d

Make tests nicer

96ccaf1

nishasy marked this pull request as ready for review March 13, 2024 01:33

jeremywiebe approved these changes Mar 13, 2024

View reviewed changes

nedredmond approved these changes Mar 13, 2024

View reviewed changes

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx Outdated Show resolved Hide resolved

packages/perseus/src/widgets/interactive-graphs/mafs-locked-layer.tsx Outdated Show resolved Hide resolved

Use readonly array for lockedFigures. Rename mafs-locked-layer to gra…

1ffe5b5

…ph-locked-layer.

nishasy added 6 commits March 13, 2024 13:49

Make grid lines lighter. Add mafs to interactive graph editor control…

6270d1d

…led story. Stop the bug where that story would break on input field change.

Make a separate story for mafs

7cec868

Make the interactive graph story with mafs start on the segment graph…

c04dd7d

… type

Merge branch 'main' into view-point-mafs

0fafcf4

use a wonder block color for the grid

1ba762f

Fix test after merging main

19b34f8

nishasy merged commit 6196375 into main Mar 13, 2024
10 checks passed

nishasy deleted the view-point-mafs branch March 13, 2024 21:14

khan-actions-bot mentioned this pull request Mar 13, 2024

Version Packages #1069

Merged

benchristel mentioned this pull request Mar 13, 2024

Make Mafs line segment color offBlack64 #1073

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

View a locked point with mafs #1067

View a locked point with mafs #1067

nishasy commented Mar 12, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024 •

edited

Loading

codecov bot commented Mar 12, 2024 •

edited

Loading

benchristel left a comment

khan-actions-bot commented Mar 13, 2024 •

edited

Loading

jeremywiebe left a comment

jeremywiebe Mar 13, 2024

nedredmond left a comment

github-actions bot commented Mar 13, 2024 •

edited

Loading

		@@ -0,0 +1,41 @@
		import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core";

View a locked point with mafs #1067

View a locked point with mafs #1067

Conversation

nishasy commented Mar 12, 2024 • edited Loading

Summary:

Test plan:

Story

github-actions bot commented Mar 12, 2024 • edited Loading

codecov bot commented Mar 12, 2024 • edited Loading

Codecov Report

benchristel left a comment

Choose a reason for hiding this comment

khan-actions-bot commented Mar 13, 2024 • edited Loading

Gerald

jeremywiebe left a comment

Choose a reason for hiding this comment

jeremywiebe Mar 13, 2024

Choose a reason for hiding this comment

nedredmond left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 13, 2024 • edited Loading

npm Snapshot: Published

nishasy commented Mar 12, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024 •

edited

Loading

codecov bot commented Mar 12, 2024 •

edited

Loading

khan-actions-bot commented Mar 13, 2024 •

edited

Loading

github-actions bot commented Mar 13, 2024 •

edited

Loading