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

[Locked Figure Labels] View locked function labels #1658

Merged
merged 11 commits into from
Sep 24, 2024
Merged

Conversation

nishasy
Copy link
Contributor

@nishasy nishasy commented Sep 23, 2024

Summary:

  • Add feature flag for locked function labels
  • Add labels to LockedFunctionType
  • update builder
  • update stories and tests
    • add a story for all locked figure labels

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

Test plan:

image

- Add feature flag for locked ellipse labels
- Add labels to LockedEllipseType
- Update builder
- Update stories and tests

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

Test plan:
- Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-ellipse-labels-flag
- Confirm that the visible label is on the locked ellipse only
- Go to the point, line, and vector  stories and confirm that the
  locked ellipse does not have a visible label (make sure the
  flags are separating the behaviors correctly)
Add the ability to add/edit/delete labels for locked ellipses
within the interactive graph editor.

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

Test plan:
- Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-ellipse-labels-flag
- Go to locked figures
- Open the locked ellipse settings
- Confirm that the label settings are there
- Play around with the label settings and confirm that it moves
  and updates with the ellipse
Copy link
Contributor

github-actions bot commented Sep 23, 2024

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1658

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

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

Copy link
Contributor

github-actions bot commented Sep 23, 2024

Size Change: +29 B (0%)

Total Size: 863 kB

Filename Size Change
packages/perseus/dist/es/index.js 418 kB +29 B (+0.01%)
ℹ️ 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-editor/dist/es/index.js 279 kB
packages/perseus-linter/dist/es/index.js 22.2 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

@nishasy nishasy marked this pull request as ready for review September 23, 2024 22:13
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/spotty-days-burn.md, 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/src/widgets/interactive-graphs/graph-locked-labels-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/interactive-graph.test.tsx, packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts, packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts, packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts

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 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.32%. Comparing base (790e189) to head (e13f7aa).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1658      +/-   ##
==========================================
- Coverage   70.68%   70.32%   -0.36%     
==========================================
  Files         562      588      +26     
  Lines      108177   112727    +4550     
  Branches     7968    12004    +4036     
==========================================
+ Hits        76463    79279    +2816     
- Misses      31517    33448    +1931     
+ Partials      197        0     -197     

Impacted file tree graph

see 1150 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 790e189...e13f7aa. Read the comment docs.

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.

LGTM!

@nishasy nishasy changed the base branch from locked-ellipse-labels-editor to main September 24, 2024 17:30
@nishasy
Copy link
Contributor Author

nishasy commented Sep 24, 2024

The parent pull-request (#1655) has been merged into main, but this branch (locked-function-labels) 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 20b3a24 into main Sep 24, 2024
11 checks passed
@nishasy nishasy deleted the locked-function-labels branch September 24, 2024 17:47
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