Skip to content
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.

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

[Interactive Graph] View a locked polygon #1353

Merged
merged 8 commits into from
Jun 17, 2024
Merged

[Interactive Graph] View a locked polygon #1353

merged 8 commits into from
Jun 17, 2024

Conversation

nishasy
Copy link
Contributor

@nishasy nishasy commented Jun 14, 2024

Summary:

View a locked polygon!

  • Add the types necessary to view a locked polygon
  • Add the "locked polygon" component that displays on an interactive graph
  • Add a storybook story to confirm that locked polygons render on the graph

Small note: I decided to switch the order of ellipse and vector, because I
thought it would make more sense for vector to be next to line and for
ellipse to be next to polygon.

Note that markings (congruent/similar/parallel/right angle) are part of
a separate tasking.

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

Test plan:

yarn jest

Storybook

Screenshot 2024-06-14 at 4 29 24 PM

… only take degrees for the angle input

We had it so that locked ellipse settings allow for either degrees or radians
for the angel input. However, I realized that we're not saving the data for
what unit is being used. This means that if an angle were saved as "30 degrees"
and then the page were reloaded, it would then show up as "0.523599 radians".

For the sake of simplicty and consistency, I'm changing this so that it only
takes degrees, and I'm changing it to a number input since it no longer needs
to evaluate expressions with pi (that required it being a text input).

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

Test plan:
`yarn jest`

Storybook
- http://localhost:6006/?path=/story/perseuseditor-editorpage--mafs-with-locked-figures-m-2-flag
- Confirm that the ellipse settings angle input shows degrees only
- Confirm that it is rotated correctly
…ll locked figures

Super minor. Just adding a locked vector to the `segmentWithLockedFigures`
testdata so that the stories with all locked figures show all the current
locked figures.

Issue: none

Test plan:
http://localhost:6006/?path=/story/perseuseditor-editorpage--mafs-with-locked-figures-m-2-flag

confirm all four locked figure types' settings are there
…d figures

At the moment, we only allow authors to use 5 colors for interactive graph
locked figures. However, there was a request to add blue and gold to the colorset.

- Adding blue and orange (formerly gold) to the colorset

Issue: none

Test plan:
- http://localhost:6006/?path=/story/perseuseditor-editorpage--mafs-with-locked-figures-m-2-flag
- Confirm that blue and gold are present in the color dropdowns
- there should be 7 total colors
View a locked polygon!

- Add the types necessary to view a locked polygon
- Add the "locked polygon" component that displays on an interactive graph
- Add a storybook story to confirm that locked polygons render on the graph

Note that markings (congruent/similar/parallel/right angle) are part of
a separate tasking.

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

Test plan:
`yarn jest`

Storybook
- http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--locked-polygon
@nishasy nishasy self-assigned this Jun 14, 2024
@nishasy nishasy requested review from mark-fitzgerald, benchristel and a team June 14, 2024 23:29
Copy link
Contributor

github-actions bot commented Jun 14, 2024

Size Change: +181 B (+0.02%)

Total Size: 845 kB

Filename Size Change
packages/perseus-editor/dist/es/index.js 271 kB +85 B (+0.03%)
packages/perseus/dist/es/index.js 407 kB +96 B (+0.02%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.2 kB
packages/kmath/dist/es/index.js 4.26 kB
packages/math-input/dist/es/index.js 80.1 kB
packages/math-input/dist/es/strings.js 1.73 kB
packages/perseus-core/dist/es/index.js 906 B
packages/perseus-error/dist/es/index.js 866 B
packages/perseus-linter/dist/es/index.js 21.8 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

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 89.83051% with 12 lines in your changes missing coverage. Please review.

Project coverage is 70.74%. Comparing base (9a6517c) to head (bc78caa).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1353      +/-   ##
==========================================
+ Coverage   69.67%   70.74%   +1.07%     
==========================================
  Files         488      493       +5     
  Lines      103250   103409     +159     
  Branches     7392    10493    +3101     
==========================================
+ Hits        71939    73161    +1222     
+ Misses      31130    30248     -882     
+ Partials      181        0     -181     

Impacted file tree graph

Files Coverage Δ
...s/perseus-editor/src/components/ellipse-swatch.tsx 100.00% <100.00%> (ø)
...-editor/src/components/locked-ellipse-settings.tsx 100.00% <100.00%> (ø)
packages/perseus/src/index.ts 100.00% <100.00%> (ø)
...widgets/__testdata__/interactive-graph.testdata.ts 100.00% <ø> (ø)
.../widgets/interactive-graphs/graph-locked-layer.tsx 89.70% <100.00%> (+1.37%) ⬆️
...ctive-graphs/interactive-graph-question-builder.ts 100.00% <100.00%> (ø)
.../src/widgets/interactive-graphs/locked-ellipse.tsx 100.00% <100.00%> (ø)
.../src/widgets/interactive-graphs/locked-polygon.tsx 100.00% <100.00%> (ø)
...s-editor/src/components/locked-figures-section.tsx 96.79% <28.57%> (-3.21%) ⬇️
packages/perseus-editor/src/components/util.ts 60.62% <65.00%> (+1.70%) ⬆️

... and 140 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 9a6517c...bc78caa. Read the comment docs.

@nishasy nishasy marked this pull request as ready for review June 14, 2024 23:40
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/metal-apricots-brake.md, packages/perseus/src/index.ts, packages/perseus/src/perseus-types.ts, packages/perseus-editor/src/components/ellipse-swatch.tsx, packages/perseus-editor/src/components/locked-ellipse-settings.tsx, packages/perseus-editor/src/components/locked-figures-section.tsx, packages/perseus-editor/src/components/util.ts, 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/interactive-graph-question-builder.test.ts, packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts, packages/perseus/src/widgets/interactive-graphs/locked-ellipse.tsx, packages/perseus/src/widgets/interactive-graphs/locked-polygon.tsx, packages/perseus-editor/src/components/__tests__/util.test.ts

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

Copy link
Member

@benchristel benchristel left a comment

Choose a reason for hiding this comment

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

Cool! LGTM!

Base automatically changed from lf-add-2-colors to main June 17, 2024 19:16
Copy link
Contributor

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1353

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

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

@nishasy nishasy merged commit e528c5b into main Jun 17, 2024
13 checks passed
@nishasy nishasy deleted the view-locked-polygon branch June 17, 2024 21:40
nishasy added a commit that referenced this pull request Jun 17, 2024
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@23.5.0

### Minor Changes

- [#1348](#1348)
[`73ba4f7c9`](73ba4f7)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Update the locked ellipse settings so they only take degrees as
input.


- [#1353](#1353)
[`e528c5b2b`](e528c5b)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
View a locked polygon


- [#1351](#1351)
[`9a6517ca2`](9a6517c)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Add blue and gold to locked figures colorset

### Patch Changes

- [#1350](#1350)
[`1e877c6d4`](1e877c6)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Add locked vector to storybook story for all locked figures


- [#1345](#1345)
[`92990f15f`](92990f1)
Thanks [@benchristel](https://github.com/benchristel)! - Fix a bug in
the exercise editor where the preview did not update after a change to
the graph type or number of line segments.

## @khanacademy/perseus-editor@6.11.0

### Minor Changes

- [#1348](#1348)
[`73ba4f7c9`](73ba4f7)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Update the locked ellipse settings so they only take degrees as
input.


- [#1353](#1353)
[`e528c5b2b`](e528c5b)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
View a locked polygon


- [#1351](#1351)
[`9a6517ca2`](9a6517c)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Add blue and gold to locked figures colorset


- [#1354](#1354)
[`e73373f48`](e73373f)
Thanks [@Myranae](https://github.com/Myranae)! - Fix interactive graph
editor in storybook to display and persist options

### Patch Changes

- [#1350](#1350)
[`1e877c6d4`](1e877c6)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Add locked vector to storybook story for all locked figures

- Updated dependencies
\[[`1e877c6d4`](1e877c6),
[`92990f15f`](92990f1),
[`73ba4f7c9`](73ba4f7),
[`e528c5b2b`](e528c5b),
[`9a6517ca2`](9a6517c)]:
    -   @khanacademy/perseus@23.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants