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 Editor] Add blue and gold back as colors for locked figures #1351

Merged
merged 7 commits into from
Jun 17, 2024

Conversation

nishasy
Copy link
Contributor

@nishasy nishasy commented Jun 13, 2024

Summary:

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

Note that these colors were part of the original colorset. They were removed in the following commit:
2d3c3b4#diff-5737e5c75b244a64356894bb42c711539f1589cfbd9dcbbd4b22a3db8773d524

Issue: none

Test plan:

Screenshot 2024-06-13 at 3 42 54 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
@nishasy nishasy self-assigned this Jun 13, 2024
Copy link
Contributor

github-actions bot commented Jun 13, 2024

Size Change: +38 B (0%)

Total Size: 845 kB

Filename Size Change
packages/perseus/dist/es/index.js 407 kB +38 B (+0.01%)
ℹ️ 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-editor/dist/es/index.js 271 kB
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 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.14%. Comparing base (73ba4f7) to head (0576fb3).
Report is 1 commits behind head on main.

Current head 0576fb3 differs from pull request most recent head 019a47a

Please upload reports for the commit 019a47a to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1351      +/-   ##
==========================================
+ Coverage   69.63%   71.14%   +1.50%     
==========================================
  Files         488      492       +4     
  Lines      103250   103307      +57     
  Branches     7437    10509    +3072     
==========================================
+ Hits        71902    73499    +1597     
+ Misses      31166    29808    -1358     
+ Partials      182        0     -182     

Impacted file tree graph

Files Coverage Δ
...ages/perseus-editor/src/components/angle-input.tsx 100.00% <100.00%> (ø)
...eus-editor/src/components/locked-line-settings.tsx 100.00% <100.00%> (ø)
...widgets/__testdata__/interactive-graph.testdata.ts 100.00% <ø> (ø)

... and 144 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 1e877c6...019a47a. Read the comment docs.

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

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/sweet-jokes-poke.md, packages/perseus/src/perseus-types.ts

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

@khan-actions-bot khan-actions-bot requested a review from a team June 14, 2024 16:34
Copy link
Contributor

github-actions bot commented Jun 14, 2024

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1351

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

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

Copy link
Contributor

@Myranae Myranae left a comment

Choose a reason for hiding this comment

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

Was able to see the two new colors! And there are 7 in the list. Looks good!

"red",
] as const;

export type LockedFigureColor = (typeof lockedFigureColorNames)[number];

export const lockedFigureColors: Record<LockedFigureColor, string> = {
blue: "#3D7586",
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Are these the same as the WB color tokens? Just curious :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope! They're MathJax colors.

Base automatically changed from add-vector-to-testdata to main June 17, 2024 19:10
@nishasy
Copy link
Contributor Author

nishasy commented Jun 17, 2024

The parent pull-request (#1350) has been merged into main, but this branch (lf-add-2-colors) now has conflicts with the new base branch. These conflicts must be resolved before checks can complete on this pull-request.

@nishasy nishasy merged commit 9a6517c into main Jun 17, 2024
11 checks passed
@nishasy nishasy deleted the lf-add-2-colors branch June 17, 2024 19:16
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
4 participants