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] Remove the start coords UI if the graph is static #1610

Merged
merged 4 commits into from
Sep 10, 2024

Conversation

nishasy
Copy link
Contributor

@nishasy nishasy commented Sep 10, 2024

Summary:

When the graph is static, it uses the "correct" graph's coords to set
the coords on the now noninteractive graph. This makes the start coords
UI useless, as those coords will not be used.

  • Remove the start coords UI if the graph is static

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

Test plan:

yarn jest packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx

Storybook

Screen.Recording.2024-09-10.at.3.10.52.PM.mov

Now that locked functions have been safely out for a while now, we can
remove the `interactive-graph-locked-features-m2b` flag from Perseus.

Next, we can remove it from webapp Growthbook.

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

Test plan:
`yarn jest`
`yarn typecheck`

Storybook
- http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-figures-current
- Confirm that the locked functions are still available in the graph and editor
Now that start coords UI for all graph types have been
safely out for a while now, we can remove the `start-coords-ui-*`
flags from Perseus.

Next, we can remove them from webapp and Growthbook.

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

Test plan:
`yarn jest`
`yarn typecheck`

- The the perseus repo in your IDE, seach for all instances of `start-coords-ui` and confirm there are none left.
- Confirm there are also no instances of `TODO(LEMS-2228)` left.

Storybook
- Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-segment (as well as all the other graphs' stories)
- Confirm that the start coords UI show up
- Confirm that the start coords UI does not show up for unlimited points, unlimited polygons, or polygons with snapTo angles or sides
… static

When the graph is static, it uses the "correct" graph's coords to set
the coords on the now noninteractive graph. This makes the start coords
UI useless, as those coords will not be used.

- Remove the start coords UI if the graph is static

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

Test plan:
`yarn jest packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx`

Storybook
- http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-segment (and other graph types)
- Toggle between static and not static graphs
- Confirm that the start coords UI goes away when the graph is static, and the
  "correct" graph coords are used
- Confirm that the start coords UI is present when the graph is not static,
  and the start coords are used
@nishasy nishasy self-assigned this Sep 10, 2024
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.

🎉

Copy link
Contributor

github-actions bot commented Sep 10, 2024

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1610

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

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

@nishasy nishasy marked this pull request as ready for review September 10, 2024 22:14
Copy link
Contributor

github-actions bot commented Sep 10, 2024

Size Change: +96 B (+0.01%)

Total Size: 860 kB

Filename Size Change
packages/perseus-editor/dist/es/index.js 277 kB +96 B (+0.03%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.3 kB
packages/keypad-context/dist/es/index.js 760 B
packages/kmath/dist/es/index.js 4.27 kB
packages/math-input/dist/es/index.js 78 kB
packages/math-input/dist/es/strings.js 1.79 kB
packages/perseus-core/dist/es/index.js 1.48 kB
packages/perseus-linter/dist/es/index.js 22.2 kB
packages/perseus/dist/es/index.js 417 kB
packages/perseus/dist/es/strings.js 3.36 kB
packages/pure-markdown/dist/es/index.js 3.66 kB
packages/simple-markdown/dist/es/index.js 12.4 kB

compressed-size-action

@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/six-melons-camp.md, packages/perseus-editor/src/components/util.ts, packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx, packages/perseus-editor/src/widgets/interactive-graph-editor/interactive-graph-editor.tsx

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

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.46%. Comparing base (9810472) to head (aef4558).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1610      +/-   ##
==========================================
+ Coverage   70.22%   70.46%   +0.24%     
==========================================
  Files         548      573      +25     
  Lines      107262   111747    +4485     
  Branches     7799    11222    +3423     
==========================================
+ Hits        75326    78748    +3422     
- Misses      31748    32999    +1251     
+ Partials      188        0     -188     

Impacted file tree graph

see 1121 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 9810472...aef4558. Read the comment docs.

@nishasy nishasy changed the base branch from start-coords-flag-cleanup to main September 10, 2024 23:42
An error occurred while trying to automatically change base from start-coords-flag-cleanup to main September 10, 2024 23:42
@nishasy
Copy link
Contributor Author

nishasy commented Sep 10, 2024

The parent pull-request (#1609) has been merged into main, but this branch (no-start-coords-static) 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 e9b317c into main Sep 10, 2024
12 of 13 checks passed
@nishasy nishasy deleted the no-start-coords-static branch September 10, 2024 23:55
SonicScrewdriver added a commit that referenced this pull request Sep 11, 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@33.1.0

### Minor Changes

- [#1452](#1452)
[`3980a36fa`](3980a36)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Implementation of SVG-based Axis Tick Labels for Interactive Graph

### Patch Changes

- [#1609](#1609)
[`981047211`](9810472)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Remove the start-coords-ui flags


- [#1610](#1610)
[`e9b317ca0`](e9b317c)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Remove the start coords UI if the graph is static


- [#1608](#1608)
[`737fe30b4`](737fe30)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Remove the interactive-graph-locked-feature-m2b flag

## @khanacademy/perseus-editor@14.1.1

### Patch Changes

- [#1609](#1609)
[`981047211`](9810472)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Remove the start-coords-ui flags


- [#1607](#1607)
[`1b340b197`](1b340b1)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Use Wonder Blocks TextArea in the graph description settings UI


- [#1610](#1610)
[`e9b317ca0`](e9b317c)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph
Editor] Remove the start coords UI if the graph is static


- [#1608](#1608)
[`737fe30b4`](737fe30)
Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph]
Remove the interactive-graph-locked-feature-m2b flag

- Updated dependencies
\[[`981047211`](9810472),
[`e9b317ca0`](e9b317c),
[`737fe30b4`](737fe30),
[`3980a36fa`](3980a36)]:
    -   @khanacademy/perseus@33.1.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
Development

Successfully merging this pull request may close these issues.

3 participants