[Interactive Graph] Polygon side snapping is keyboard accessible. #2270

catandthemachines · 2025-02-25T23:55:25Z

Summary:

Updating Polygon interactive graph implementation to have keyboard accessible side snapping. This is done by hooking up the side snapping behavior into a constraint function to make to keyboard arrow keys.

Issue: LEMS-2243

Test plan:

Go to: /?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-polygon
Set Snap to: to side measures
Use your keyboard (tab and arrow keys) to move the various polygon points.
Notice they now move!

…ing.

…lementation.

…and mouse behavior.

… we keep it whole numbers.

github-actions · 2025-02-25T23:57:14Z

Size Change: +1.41 kB (+0.16%)

Total Size: 874 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	369 kB	+1.41 kB (+0.38%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	39.7 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	11.1 kB
`packages/math-input/dist/es/index.js`	78.2 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	29.9 kB
`packages/perseus-editor/dist/es/index.js`	276 kB
`packages/perseus-linter/dist/es/index.js`	22.8 kB
`packages/perseus-score/dist/es/index.js`	20.6 kB
`packages/perseus/dist/es/strings.js`	6.74 kB
`packages/pure-markdown/dist/es/index.js`	4.14 kB
`packages/simple-markdown/dist/es/index.js`	13.1 kB

_{compressed-size-action}

github-actions · 2025-02-25T23:57:33Z

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR2270

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

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

…olygons to be keyboard accessible.

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx

packages/perseus/src/widgets/interactive-graphs/utils.ts

nishasy · 2025-02-27T00:28:14Z

Question: I see this is allowing approximate sides too. Is that intended?

catandthemachines · 2025-02-27T00:35:57Z

Question: I see this is allowing approximate sides too. Is that intended?

Yes, this is how the functionality works currently. It will show that when it's not exactly a whole number, but it's very close.

benchristel

This looks good overall! Thanks for this huge accessibility improvement!

Requesting changes because coords is being mutated in the reducer; we should make a copy instead. I left a few other suggestions inline too.

packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts

packages/perseus/src/widgets/interactive-graphs/utils.ts

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx

packages/perseus/src/widgets/interactive-graphs/utils.ts

packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts

packages/perseus/src/widgets/interactive-graphs/__snapshots__/interactive-graph.test.tsx.snap

packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts

packages/perseus/src/widgets/interactive-graphs/utils.ts

…tation.

benchristel

Thanks for the updates! LGTM!

I left a couple small questions and suggestions inline, but nothing blocking.

benchristel · 2025-03-04T17:53:41Z

packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts

@@ -302,7 +302,7 @@ export const polygonWithFourSidesSnappingQuestion: PerseusRenderer =
        .withGridStep(0.5, 0.5)
        .withSnapStep(0.25, 0.25)
        .withTickStep(0.5, 0.5)
-        .withMarkings("none")
+        .withMarkings("graph")


Why did these values change? In general, I think we should be pretty cautious about changing existing values in these testdata files, since they are shared between tests and any changes could cause tests to pass for the wrong reason.

Oh okay, I can revert them. I updated them because I found debugging very challenging for these testdata examples and I'm curious why we don't want it to have any markings?

Ah, I believe I added "none" markings when I migrated over old data to use the builder. This data was probably originally taken from an existing question, dumped in the testdata, and never changed. And then I did a 1:1 update to the builder.

As far as use of this testdata goes, I don't think these are being used to test markings anywhere. I support removing the withMarkings call altogether for any of them that say "none"

Yay! I'll add them back in a follow-up PR.

packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts

nishasy

LGTM! Just left one small suggestion, but this looks great.

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx

packages/perseus/src/widgets/interactive-graphs/math/box.test.ts

@handeyeco

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/math-input@23.0.0 ### Major Changes - [#2226](#2226) [`909148cdc`](909148c) Thanks [@handeyeco](https://github.com/handeyeco)! - Answerless Expression: Expression can render and is interactive with answerless data ### Patch Changes - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 - @khanacademy/keypad-context@1.1.1 ## @khanacademy/perseus@56.0.0 ### Major Changes - [#2233](#2233) [`a0aee41b6`](a0aee41) Thanks [@handeyeco](https://github.com/handeyeco)! - RadioWidget v2 in support of answerless Radio ### Patch Changes - [#2279](#2279) [`e02cc4109`](e02cc41) Thanks [@nishasy](https://github.com/nishasy)! - [SR] Tweak strings for Segment and Angle graphs - [#2270](#2270) [`941343ee3`](941343e) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Fixing side snapping within polygons to be keyboard accessible. - [#2226](#2226) [`909148cdc`](909148c) Thanks [@handeyeco](https://github.com/handeyeco)! - Answerless Expression: Expression can render and is interactive with answerless data - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 - @khanacademy/math-input@23.0.0 - @khanacademy/keypad-context@1.1.1 - @khanacademy/kmath@0.4.1 - @khanacademy/perseus-linter@1.3.1 - @khanacademy/perseus-score@2.3.1 ## @khanacademy/perseus-core@5.0.0 ### Major Changes - [#2233](#2233) [`a0aee41b6`](a0aee41) Thanks [@handeyeco](https://github.com/handeyeco)! - RadioWidget v2 in support of answerless Radio - [#2226](#2226) [`909148cdc`](909148c) Thanks [@handeyeco](https://github.com/handeyeco)! - Answerless Expression: Expression can render and is interactive with answerless data ## @khanacademy/perseus-editor@17.9.0 ### Minor Changes - [#2226](#2226) [`909148cdc`](909148c) Thanks [@handeyeco](https://github.com/handeyeco)! - Answerless Expression: Expression can render and is interactive with answerless data ### Patch Changes - [#2233](#2233) [`a0aee41b6`](a0aee41) Thanks [@handeyeco](https://github.com/handeyeco)! - RadioWidget v2 in support of answerless Radio - Updated dependencies \[[`e02cc4109`](e02cc41), [`941343ee3`](941343e), [`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus@56.0.0 - @khanacademy/perseus-core@5.0.0 - @khanacademy/math-input@23.0.0 - @khanacademy/keypad-context@1.1.1 - @khanacademy/kmath@0.4.1 - @khanacademy/perseus-linter@1.3.1 - @khanacademy/perseus-score@2.3.1 ## @khanacademy/keypad-context@1.1.1 ### Patch Changes - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 ## @khanacademy/kmath@0.4.1 ### Patch Changes - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 ## @khanacademy/perseus-linter@1.3.1 ### Patch Changes - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 ## @khanacademy/perseus-score@2.3.1 ### Patch Changes - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 - @khanacademy/kmath@0.4.1 ## @khanacademy/perseus-dev-ui@5.3.1 ### Patch Changes - Updated dependencies \[[`a0aee41b6`](a0aee41), [`909148cdc`](909148c)]: - @khanacademy/perseus-core@5.0.0 - @khanacademy/math-input@23.0.0 - @khanacademy/kmath@0.4.1 - @khanacademy/perseus-linter@1.3.1 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

catandthemachines added 10 commits February 19, 2025 17:43

[catjohnson/lems-2243] initial idea for angle and side snapping track…

326d7bc

…ing.

[catjohnson/lems-2243] Initial prototyping of the snapping fix.

4fd11c4

[catjohnson/lems-2243] fixing linting.

5980c15

[catjohnson/lems-2243] more linting.

1e83c41

[catjohnson/lems-2243] Moving keyboard functionality to polygon.

cb22e21

[catjohnson/lems-2243] adding back the side snapping in the mouse imp…

b09d0b1

…lementation.

[catjohnson/lems-2243] Removing unnecessary file

0eab122

[catjohnson/lems-2243] refactoring out repeat logic used by keyboard …

036ef07

…and mouse behavior.

[catjohnson/lems-2243] Showing what the side snapping looks like when…

5907617

… we keep it whole numbers.

[catjohnson/lems-2243] removing snapstep adjustment.

00b445d

catandthemachines self-assigned this Feb 25, 2025

catandthemachines added 7 commits February 25, 2025 16:01

[catjohnson/lems-2243] revert changes.

f34f591

[catjohnson/lems-2243] fixing angle snapping.

86c0bb2

[catjohnson/lems-2243] cleaning up conditions.

2e9658a

[catjohnson/lems-2243] Updating comment

345beba

[catjohnson/lems-2243] Merge branch 'main' into catjohnson/lems-2243

1abdc9d

[catjohnson/lems-2243] docs(changeset): Fixing side snapping within p…

0dbdc7f

…olygons to be keyboard accessible.

[catjohnson/lems-2243] updating snapshots.

dc422cf

catandthemachines requested review from nishasy, SonicScrewdriver and benchristel February 26, 2025 00:57

nishasy reviewed Feb 27, 2025

View reviewed changes

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx Show resolved Hide resolved

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx Show resolved Hide resolved

packages/perseus/src/widgets/interactive-graphs/utils.ts Outdated Show resolved Hide resolved

benchristel requested changes Feb 27, 2025

View reviewed changes

[catjohnson/lems-2243] Merge branch 'main' into catjohnson/lems-2243

5bdf5dd

catandthemachines commented Feb 27, 2025

View reviewed changes

[catjohnson/lems-2243] Adding snapping type, to clean up some typing.

81c354c

catandthemachines added 5 commits March 3, 2025 14:45

[catjohnson/lems-2243] Updating usage of SnapTo type.

2ac5f98

[catjohnson/lems-2243] Adding

4137605

[catjohnson/lems-2243] Cleaning up and fixing the coordsCopy implemen…

2faf18e

…tation.

[catjohnson/lems-2243] Adding tests.

c7adcba

[catjohnson/lems-2243] Adding comments

312887f

catandthemachines requested review from nishasy and benchristel March 4, 2025 01:00

benchristel approved these changes Mar 4, 2025

View reviewed changes

catandthemachines added 2 commits March 4, 2025 10:53

[catjohnson/lems-2243] Fixing nits.

6597bdc

[catjohnson/lems-2243] Update snap.

657c027

nishasy approved these changes Mar 4, 2025

View reviewed changes

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx Show resolved Hide resolved

packages/perseus/src/widgets/interactive-graphs/math/box.test.ts Outdated Show resolved Hide resolved

nishasy reviewed Mar 4, 2025

View reviewed changes

packages/perseus/src/widgets/interactive-graphs/math/box.test.ts Outdated Show resolved Hide resolved

nishasy reviewed Mar 4, 2025

View reviewed changes

packages/perseus/src/widgets/interactive-graphs/math/box.test.ts Outdated Show resolved Hide resolved

[catjohnson/lems-2243] Fixing box test.

1794604

catandthemachines merged commit 941343e into main Mar 4, 2025
8 checks passed

catandthemachines deleted the catjohnson/lems-2243 branch March 4, 2025 22:47

khan-actions-bot mentioned this pull request Mar 4, 2025

Version Packages #2280

Merged

catandthemachines mentioned this pull request Mar 4, 2025

[Interactive Graph] Improve Polygon angle snapping is for keyboard users. #2281

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Interactive Graph] Polygon side snapping is keyboard accessible. #2270

[Interactive Graph] Polygon side snapping is keyboard accessible. #2270

catandthemachines commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading

nishasy commented Feb 27, 2025

catandthemachines commented Feb 27, 2025

benchristel left a comment

benchristel left a comment

benchristel Mar 4, 2025

catandthemachines Mar 4, 2025

nishasy Mar 4, 2025

catandthemachines Mar 4, 2025

nishasy left a comment

[Interactive Graph] Polygon side snapping is keyboard accessible. #2270

[Interactive Graph] Polygon side snapping is keyboard accessible. #2270

Conversation

catandthemachines commented Feb 25, 2025 • edited Loading

Summary:

Test plan:

github-actions bot commented Feb 25, 2025 • edited Loading

github-actions bot commented Feb 25, 2025 • edited Loading

npm Snapshot: Published

nishasy commented Feb 27, 2025

catandthemachines commented Feb 27, 2025

benchristel left a comment

Choose a reason for hiding this comment

benchristel left a comment

Choose a reason for hiding this comment

benchristel Mar 4, 2025

Choose a reason for hiding this comment

catandthemachines Mar 4, 2025

Choose a reason for hiding this comment

nishasy Mar 4, 2025

Choose a reason for hiding this comment

catandthemachines Mar 4, 2025

Choose a reason for hiding this comment

nishasy left a comment

Choose a reason for hiding this comment

catandthemachines commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading

github-actions bot commented Feb 25, 2025 •

edited

Loading