[Locked Figure Labels] Add/edit/delete locked polygon labels #1690

nishasy · 2024-09-27T21:30:15Z

Summary:

Add the ability to add/edit/delete labels for locked polygons
within the interactive graph editor.

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

Test plan:

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

…tor)

…itor)

github-actions · 2024-09-27T21:32:49Z

Size Change: +155 B (+0.02%)

Total Size: 864 kB

Filename	Size	Change
`packages/perseus-editor/dist/es/index.js`	280 kB	+155 B (+0.06%)

ℹ️ 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`	418 kB
`packages/perseus/dist/es/strings.js`	3.4 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

github-actions · 2024-09-27T21:32:56Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1690

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

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

khan-actions-bot · 2024-09-27T21:33:51Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/dirty-ducks-rescue.md, packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx, packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.test.tsx, packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx

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

catandthemachines

Looks good, just a gentle nit on some if statements. 😉

catandthemachines · 2024-09-27T21:54:45Z

...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx

+        updatedLabel: LockedLabelType,
+        labelIndex: number,
+    ) {
+        if (!labels) {


nit: I'm personally against empty returns when simply wrapping the logic in an if statement would work just as effectively. Highly recommend the following pattern instead.
if (label) { .... }

Interesting! This might be an "agree to disagree" situation because I'm pretty solidly in opposition. I prefer returning early, because I find that makes the rest of the function easier to read. It generally reduces indentation, and I feel it makes it clear that we don't have to worry about the null case anymore past that point. I think the "return early" pattern is pretty widely used/taught in my (admittedly limited) experience.

It's so funny because while the "return early" pattern was widely taught in my experience. The "empty return" was highly discouraged. Let's agree to disagree here 😉

catandthemachines · 2024-09-27T21:55:02Z

...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx

+    }
+
+    function handleLabelRemove(labelIndex: number) {
+        if (!labels) {


nit: same here.

@nishasy

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@35.1.0 ### Minor Changes - [#1687](#1687) [`c0cbb484a`](c0cbb48) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked function aria labels (graph + editor) - [#1690](#1690) [`a65da60c2`](a65da60) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked polygon labels - [#1683](#1683) [`2d7cadaf0`](2d7cada) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked line aria label behavior on graph - [#1691](#1691) [`fc16bc77f`](fc16bc7) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked polygon aria labels (graph + editor) - [#1677](#1677) [`3c73f4aa4`](3c73f4a) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked point aria label behavior on graph - [#1684](#1684) [`17ebfc0de`](17ebfc0) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked line aria label editor UI - [#1685](#1685) [`b3de0d80e`](b3de0d8) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked vector aria labels (graph + editor) - [#1689](#1689) [`eda436f00`](eda436f) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked polygon labels - [#1682](#1682) [`039e0a360`](039e0a3) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked point aria label editor UI - [#1686](#1686) [`8230d96ee`](8230d96) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked ellipse aria labels (graph + editor) ### Patch Changes - [#1702](#1702) [`30725ec88`](30725ec) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out validation logic for NumberLine - [#1671](#1671) [`f326139ee`](f326139) Thanks [@handeyeco](https://github.com/handeyeco)! - Move and test Grapher's validator - [#1704](#1704) [`82e0ebd71`](82e0ebd) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out validation logic from table - [#1664](#1664) [`c41e4b2f3`](c41e4b2) Thanks [@benchristel](https://github.com/benchristel)! - Omit unused data from interactive graph onChange callback - [#1678](#1678) [`49efaaff5`](49efaaf) Thanks [@handeyeco](https://github.com/handeyeco)! - Port some tests to new custom matcher - [#1676](#1676) [`f5af24371`](f5af243) Thanks [@benchristel](https://github.com/benchristel)! - Internal: remove dead code from InteractiveGraph.validate() - [#1698](#1698) [`339a6db38`](339a6db) Thanks [@handeyeco](https://github.com/handeyeco)! - Refine Expression's Rubric type - [#1700](#1700) [`493715e3d`](493715e) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out InteractiveGraph validator - [#1705](#1705) [`e432666fd`](e432666) Thanks [@Myranae](https://github.com/Myranae)! - Move validation logic out of the iframe widget and add tests - [#1693](#1693) [`466d010c6`](466d010) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add color select to locked figure labels settings - [#1707](#1707) [`d3767f720`](d3767f7) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out matcherValidator from Matcher - [#1670](#1670) [`463755970`](4637559) Thanks [@handeyeco](https://github.com/handeyeco)! - Split validation logic out of Matrix - [#1688](#1688) [`10ce86925`](10ce869) Thanks [@Myranae](https://github.com/Myranae)! - Move validation logic out of the cs-program widget and add tests - [#1703](#1703) [`e818b0f15`](e818b0f) Thanks [@handeyeco](https://github.com/handeyeco)! - Split out Plotter validator ## @khanacademy/perseus-editor@14.6.0 ### Minor Changes - [#1687](#1687) [`c0cbb484a`](c0cbb48) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked function aria labels (graph + editor) - [#1690](#1690) [`a65da60c2`](a65da60) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add/edit/delete locked polygon labels - [#1683](#1683) [`2d7cadaf0`](2d7cada) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked line aria label behavior on graph - [#1691](#1691) [`fc16bc77f`](fc16bc7) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked polygon aria labels (graph + editor) - [#1677](#1677) [`3c73f4aa4`](3c73f4a) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked point aria label behavior on graph - [#1684](#1684) [`17ebfc0de`](17ebfc0) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked line aria label editor UI - [#1685](#1685) [`b3de0d80e`](b3de0d8) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked vector aria labels (graph + editor) - [#1689](#1689) [`eda436f00`](eda436f) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] View locked polygon labels - [#1682](#1682) [`039e0a360`](039e0a3) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Locked point aria label editor UI - [#1686](#1686) [`8230d96ee`](8230d96) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Aria] Implement locked ellipse aria labels (graph + editor) ### Patch Changes - [#1706](#1706) [`624be8143`](624be81) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figures Aria] Use TextArea instead of TextField in the locked figures aria settings - [#1693](#1693) [`466d010c6`](466d010) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Add color select to locked figure labels settings - Updated dependencies \[[`30725ec88`](30725ec), [`c0cbb484a`](c0cbb48), [`f326139ee`](f326139), [`a65da60c2`](a65da60), [`82e0ebd71`](82e0ebd), [`c41e4b2f3`](c41e4b2), [`2d7cadaf0`](2d7cada), [`49efaaff5`](49efaaf), [`fc16bc77f`](fc16bc7), [`f5af24371`](f5af243), [`339a6db38`](339a6db), [`3c73f4aa4`](3c73f4a), [`17ebfc0de`](17ebfc0), [`493715e3d`](493715e), [`e432666fd`](e432666), [`466d010c6`](466d010), [`b3de0d80e`](b3de0d8), [`eda436f00`](eda436f), [`d3767f720`](d3767f7), [`463755970`](4637559), [`10ce86925`](10ce869), [`039e0a360`](039e0a3), [`8230d96ee`](8230d96), [`e818b0f15`](e818b0f)]: - @khanacademy/perseus@35.1.0 Author: khan-actions-bot Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ⏭️ Publish npm snapshot, ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1680

nishasy added 14 commits September 25, 2024 17:32

[Locked Figure Aria] Implement locked point aria label behavior on graph

4632687

[Locked Figures Aria] Add/edit/delete locked point aria label

3dd3251

Keep text field there at all times

53acecf

Break out the aria section into its own component. Add tooltip.

e15d264

changeset

10a8d55

Add test for undefined

e099b44

[Locked Figure Aria] Implement locked line aria label behavior on graph

fd3875e

[Locked Figure Aria] Locked line aria label editor UI

6340759

[Locked Figure Aria] Implement locked vector aria labels (graph + edi…

1003256

…tor)

[Locked Figure Aria] Implement locked ellipse aria labels (graph + ed…

3c48adc

…itor)

streamline commas

111505f

[Locked Figure Aria] Locked function aria labels (graph + editor)

f5ac0a2

[Locked Figure Labels] View locked polygon labels

e153641

[Locked Figure Labels] Add/edit/delete locked polygon labels

dad7ee5

nishasy self-assigned this Sep 27, 2024

nishasy requested review from benchristel, catandthemachines, anakaren-rojas and a team September 27, 2024 21:30

nishasy marked this pull request as ready for review September 27, 2024 21:33

catandthemachines approved these changes Sep 27, 2024

View reviewed changes

Base automatically changed from locked-polygon-labels to main September 30, 2024 22:37

Merge branch 'main' into locked-polygon-labels-editor

fde7357

nishasy merged commit a65da60 into main Sep 30, 2024
9 checks passed

nishasy deleted the locked-polygon-labels-editor branch September 30, 2024 22:45

khan-actions-bot mentioned this pull request Sep 30, 2024

Version Packages #1680

Merged

khan-actions-bot mentioned this pull request Oct 1, 2024

Version Packages #1709

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Locked Figure Labels] Add/edit/delete locked polygon labels #1690

[Locked Figure Labels] Add/edit/delete locked polygon labels #1690

nishasy commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

khan-actions-bot commented Sep 27, 2024

catandthemachines left a comment

catandthemachines Sep 27, 2024

nishasy Sep 27, 2024

catandthemachines Sep 27, 2024

catandthemachines Sep 27, 2024

[Locked Figure Labels] Add/edit/delete locked polygon labels #1690

[Locked Figure Labels] Add/edit/delete locked polygon labels #1690

Conversation

nishasy commented Sep 27, 2024 • edited Loading

Summary:

Test plan:

github-actions bot commented Sep 27, 2024 • edited Loading

github-actions bot commented Sep 27, 2024 • edited Loading

npm Snapshot: Published

khan-actions-bot commented Sep 27, 2024

Gerald

catandthemachines left a comment

Choose a reason for hiding this comment

catandthemachines Sep 27, 2024

Choose a reason for hiding this comment

nishasy Sep 27, 2024

Choose a reason for hiding this comment

catandthemachines Sep 27, 2024

Choose a reason for hiding this comment

catandthemachines Sep 27, 2024

Choose a reason for hiding this comment

nishasy commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading