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

Add keyboard controls to angle graphs #1472

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

benchristel
Copy link
Member

@benchristel benchristel commented Aug 1, 2024

This PR makes it so angle graph questions can be solved with only a keyboard -
no mouse required!

There are several tricky constraints to this problem:

  • Every distinct state of the graph needs to be reachable using only the
    keyboard.
  • In order for the movement to feel natural, pressing an arrow key should
    always move the point in the corresponding direction: up, down, left, or right.
  • However, the points need to snap, not to a grid, but to angle measures
    (in degrees).
  • The side points of the angle should not be allowed to get too close to the
    vertex - if they overlap the vertex there's no way to drag them away again.
  • The points shouldn't be able to move outside the graph bounds
  • It should never be possible for an edge point to get "stuck" or blocked in its
    movement by the edge of the graph, or by the angle vertex.

I believe this PR solves for all these constraints. To make it work, I had to do
a few slightly weird things:

  • I constrained the movement of the vertex so no part of the angle-measure arc
    can go outside the graph bounds. This ensures that all the control points of
    the angle can always be placed onscreen, and prevents the side points from
    getting stuck on the edges of the graph.
  • I reworked the interface of useDraggable, renaming constrain to
    constrainKeyboardMovement. Movement constraints for dragging are now
    implemented entirely in the reducer. (They were almost all in the reducer
    before; circles were the only graph type that still depended on the constrain
    function for dragging.)
  • I also introduced a new type of useDraggable constraint, where the destination
    points for the up, left, right, and down arrows are listed explicitly. I think
    this ultimately makes the code clearer. It's also more efficient (since
    useDraggable does not have to do a bunch of repeated trig calculations to
    search for valid destination points).

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

Test plan:

View /?path=/story/perseus-widgets-interactive-graph--angle-with-mafs in
Storybook. Try to break the graph.

@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Aug 1, 2024

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/neat-days-remain.md, packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx, packages/perseus/src/widgets/interactive-graphs/protractor.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/angle.test.ts, packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/quadratic.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/sinusoid.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.test.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.ts, packages/perseus/src/widgets/interactive-graphs/math/angles.ts, packages/perseus/src/widgets/interactive-graphs/math/geometry.test.ts, packages/perseus/src/widgets/interactive-graphs/math/geometry.ts, packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts, packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts, packages/perseus/src/widgets/interactive-graphs/graphs/components/angle-indicators.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx

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 August 1, 2024 19:04
@benchristel benchristel changed the title Prototype keyboard controls for angle graphs Prototype keyboard controls for angle graphs (not ready for code review) Aug 1, 2024
Copy link
Contributor

github-actions bot commented Aug 1, 2024

Size Change: +1.19 kB (+0.14%)

Total Size: 856 kB

Filename Size Change
packages/perseus/dist/es/index.js 414 kB +1.19 kB (+0.29%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.3 kB
packages/kmath/dist/es/index.js 4.27 kB
packages/math-input/dist/es/index.js 80.3 kB
packages/math-input/dist/es/strings.js 1.73 kB
packages/perseus-core/dist/es/index.js 1.48 kB
packages/perseus-editor/dist/es/index.js 275 kB
packages/perseus-linter/dist/es/index.js 21.6 kB
packages/perseus/dist/es/strings.js 3.29 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
Contributor

github-actions bot commented Aug 1, 2024

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1472

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

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

@benchristel benchristel force-pushed the benc/angle-keyboard-controls-2 branch from 9a07ab8 to 6fcf30b Compare August 2, 2024 18:36
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 86.26761% with 39 lines in your changes missing coverage. Please review.

Project coverage is 70.39%. Comparing base (b8a342c) to head (ec33ba9).

Files Patch % Lines
...active-graphs/reducer/interactive-graph-reducer.ts 60.29% 27 Missing ⚠️
...widgets/interactive-graphs/graphs/use-draggable.ts 84.78% 7 Missing ⚠️
...us/src/widgets/interactive-graphs/graphs/angle.tsx 97.08% 3 Missing ⚠️
...us/src/widgets/interactive-graphs/math/geometry.ts 96.15% 1 Missing ⚠️
...seus/src/widgets/interactive-graphs/protractor.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1472      +/-   ##
==========================================
+ Coverage   69.87%   70.39%   +0.51%     
==========================================
  Files         513      516       +3     
  Lines      105884   106100     +216     
  Branches     7647    10835    +3188     
==========================================
+ Hits        73987    74688     +701     
+ Misses      31711    31412     -299     
+ Partials      186        0     -186     

Impacted file tree graph

Files Coverage Δ
...s/src/widgets/interactive-graphs/graphs/circle.tsx 98.46% <100.00%> (ø)
...tive-graphs/graphs/components/angle-indicators.tsx 88.16% <ø> (-1.24%) ⬇️
...eractive-graphs/graphs/components/movable-line.tsx 100.00% <100.00%> (ø)
...ractive-graphs/graphs/components/movable-point.tsx 100.00% <100.00%> (ø)
...us/src/widgets/interactive-graphs/graphs/point.tsx 100.00% <100.00%> (ø)
.../src/widgets/interactive-graphs/graphs/polygon.tsx 91.42% <100.00%> (+0.95%) ⬆️
...rc/widgets/interactive-graphs/graphs/quadratic.tsx 98.82% <100.00%> (+2.35%) ⬆️
...src/widgets/interactive-graphs/graphs/sinusoid.tsx 99.00% <100.00%> (ø)
...seus/src/widgets/interactive-graphs/math/angles.ts 89.47% <100.00%> (+0.28%) ⬆️
...us/src/widgets/interactive-graphs/math/geometry.ts 95.65% <96.15%> (+0.41%) ⬆️
... and 4 more

... and 136 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 b8a342c...ec33ba9. Read the comment docs.

snapDegrees,
} = graphState;
const {coords, showAngles, range, allowReflexAngles, snapDegrees} =
graphState;
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed angleOffsetDeg because it wasn't used.

@benchristel benchristel changed the title Prototype keyboard controls for angle graphs (not ready for code review) Add keyboard controls to angle graphs Aug 6, 2024
Copy link
Contributor

@SonicScrewdriver SonicScrewdriver left a comment

Choose a reason for hiding this comment

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

Very well commented and incredibly appreciated! Thank you for setting all of this up Ben, I think it's much cleaner!

});

it("constrains the destination point using a constrainKeyboardMovement object", async () => {
// Arrange: a 200x200px graph with a 20-unit range in each dimension.
Copy link
Contributor

Choose a reason for hiding this comment

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

I really appreciate these comment explanations about what the graph is setting up!

/>
))}
{/* vertex */}
<MovablePoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Also appreciate these callouts to help explain which points they are, now that we're not mapping them.

@benchristel benchristel force-pushed the benc/angle-keyboard-controls-2 branch from a9e09dd to ec33ba9 Compare August 8, 2024 23:38
@benchristel benchristel merged commit 4c2ace5 into main Aug 9, 2024
11 checks passed
@benchristel benchristel deleted the benc/angle-keyboard-controls-2 branch August 9, 2024 00:00
Myranae pushed a commit that referenced this pull request Aug 9, 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/[email protected]

### Major Changes

-   [#1479](#1479) [`ef96cdd54`](ef96cdd) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove PropCheckBox component from Perseus; use WB instead

### Minor Changes

-   [#1455](#1455) [`8b0268215`](8b02682) Thanks [@Myranae](https://github.com/Myranae)! - Update Polygon graphs to have weighted lines only on keyboard focus


-   [#1492](#1492) [`2ebbc1978`](2ebbc19) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Build the foundation for adding start coords UI for angle graphs


-   [#1428](#1428) [`eb9f3f9c0`](eb9f3f9) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Drop katex dependency - no longer used


-   [#1472](#1472) [`4c2ace57a`](4c2ace5) Thanks [@benchristel](https://github.com/benchristel)! - Add keyboard controls to Mafs angle graphs


-   [#1487](#1487) [`53031f8df`](53031f8) Thanks [@Myranae](https://github.com/Myranae)! - Make graphs non-interactive via keyboard when their question has been answered correctly


-   [#1486](#1486) [`0b625f560`](0b625f5) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for point graphs


-   [#1488](#1488) [`0bec013e8`](0bec013) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for polygon graphs (snap to grid only)

### Patch Changes

-   [#1474](#1474) [`a8aaac339`](a8aaac3) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Minor type fixes in the `ImageLoader` and `SvgImage` components


-   [#1493](#1493) [`de13fd337`](de13fd3) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - Add/Edit/Delete Locked Function in Interactive Graph Editor


-   [#1476](#1476) [`18f38fca4`](18f38fc) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Miscellaneous type improvements


-   [#1489](#1489) [`de2883b3f`](de2883b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Correct `hints` type on ItemRenderer


-   [#1478](#1478) [`7fd586ef4`](7fd586e) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Add "white" as a fill option for locked ellipses and polygons


-   [#1482](#1482) [`f920a4cc7`](f920a4c) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor: Storybook] Add a story for Point graph type start coords


-   [#1484](#1484) [`808956098`](8089560) Thanks [@handeyeco](https://github.com/handeyeco)! - Minor cleanup around InfoTip


-   [#1475](#1475) [`6a218bcc1`](6a218bc) Thanks [@anakaren-rojas](https://github.com/anakaren-rojas)! - add non-visual text as a description for expression widget

-   Updated dependencies \[[`6c6ff52f4`](6c6ff52), [`342a72211`](342a722), [`5e66539e6`](5e66539)]:
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Minor Changes

-   [#1493](#1493) [`de13fd337`](de13fd3) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - Add/Edit/Delete Locked Function in Interactive Graph Editor


-   [#1492](#1492) [`2ebbc1978`](2ebbc19) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Build the foundation for adding start coords UI for angle graphs


-   [#1486](#1486) [`0b625f560`](0b625f5) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for point graphs


-   [#1488](#1488) [`0bec013e8`](0bec013) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Add start coords UI for polygon graphs (snap to grid only)

### Patch Changes

-   [#1491](#1491) [`395b44f29`](395b44f) Thanks [@nishasy](https://github.com/nishasy)! - [Hint Mode: Start Coords] Correct flag logic for polygon graph start coords UI


-   [#1476](#1476) [`18f38fca4`](18f38fc) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Miscellaneous type improvements


-   [#1479](#1479) [`ef96cdd54`](ef96cdd) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove PropCheckBox component from Perseus; use WB instead


-   [#1489](#1489) [`de2883b3f`](de2883b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Correct `hints` type on ItemRenderer


-   [#1478](#1478) [`7fd586ef4`](7fd586e) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Add "white" as a fill option for locked ellipses and polygons


-   [#1482](#1482) [`f920a4cc7`](f920a4c) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor: Storybook] Add a story for Point graph type start coords


-   [#1471](#1471) [`b4e0b60dc`](b4e0b60) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Fix the dropdowns overlapping labels in locked figures settings


-   [#1481](#1481) [`a35be719f`](a35be71) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Stop page scrolling on number text field focused scroll


-   [#1484](#1484) [`808956098`](8089560) Thanks [@handeyeco](https://github.com/handeyeco)! - Minor cleanup around InfoTip


-   [#1477](#1477) [`653b520c6`](653b520) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Migrate ItemExtrasEditor to WB checkboxes

-   Updated dependencies \[[`a8aaac339`](a8aaac3), [`de13fd337`](de13fd3), [`8b0268215`](8b02682), [`2ebbc1978`](2ebbc19), [`6c6ff52f4`](6c6ff52), [`18f38fca4`](18f38fc), [`ef96cdd54`](ef96cdd), [`342a72211`](342a722), [`de2883b3f`](de2883b), [`7fd586ef4`](7fd586e), [`f920a4cc7`](f920a4c), [`eb9f3f9c0`](eb9f3f9), [`4c2ace57a`](4c2ace5), [`53031f8df`](53031f8), [`808956098`](8089560), [`5e66539e6`](5e66539), [`0b625f560`](0b625f5), [`6a218bcc1`](6a218bc), [`0bec013e8`](0bec013)]:
    -   @khanacademy/[email protected]
    -   @khanacademy/[email protected]

## @khanacademy/[email protected]

### Patch Changes

-   [#1495](#1495) [`6c6ff52f4`](6c6ff52) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove old buttons that we weren't using anymore


-   [#1475](#1475) [`342a72211`](342a722) Thanks [@anakaren-rojas](https://github.com/anakaren-rojas)! - update wonder blocks popover versions


-   [#1496](#1496) [`5e66539e6`](5e66539) Thanks [@handeyeco](https://github.com/handeyeco)! - Remove unused buttons from MathInput; add Lato

## @khanacademy/[email protected]

### Patch Changes

-   Updated dependencies \[[`6c6ff52f4`](6c6ff52), [`342a72211`](342a722), [`5e66539e6`](5e66539)]:
    -   @khanacademy/[email protected]

Author: khan-actions-bot

Reviewers: Myranae

Required Reviewers:

Approved By: Myranae

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ⏭️  Publish npm snapshot, ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1473
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