Migrate out of react-hotkeys to @mantine/hooks#2968
Merged
RichDom2185 merged 12 commits intomasterfrom May 3, 2024
Merged
Conversation
chownces
force-pushed
the
hotkeys-migration
branch
from
161c3d6 to
b70fa5f
Compare
Pull Request Test Coverage Report for Build 8934209129Details
💛 - Coveralls |
chownces
force-pushed
the
hotkeys-migration
branch
from
b658b16 to
fac3eda
Compare
chownces
force-pushed
the
hotkeys-migration
branch
from
fac3eda to
b63fd75
Compare
Member
Note that we plan to shift DataViz to a module: #2809 |
RichDom2185
reviewed
May 2, 2024
Member
RichDom2185
left a comment
RichDom2185
left a comment
There was a problem hiding this comment.
Thanks for this! Did a quick run through and some comments below:
RichDom2185
approved these changes
May 2, 2024
Comment on lines
+197
to
+209
| const hotkeyBindings: HotkeyItem[] = this.state.visualization | ||
| ? [ | ||
| ['a', this.stepFirst], | ||
| ['f', this.stepNext], | ||
| ['b', this.stepPrevious], | ||
| ['e', this.stepLast(this.props.stepsTotal)] | ||
| ] | ||
| : [ | ||
| ['a', () => {}], | ||
| ['f', () => {}], | ||
| ['b', () => {}], | ||
| ['e', () => {}] | ||
| ]; |
Member
There was a problem hiding this comment.
While not related to this PR, it it just be or are the keyboard shortcuts not intuitive? I would expect the following:
(first, prev, next, last) to be mapped to (h, j, k, l) or (a, s, d, f) -- basically just a sequence of keys that are next to each other on the keyboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Consolidating our usage of 3rd party hooks under the mantine hooks library.
h u l khotkey sequence has been replaced withalt+shift+hsince mantine'suseHotkeydoes not support hotkey sequencesCloses #2935
Type of change
How to test
Checklist