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

Dev ignore submatches #186

Merged
merged 3 commits into from
Jun 2, 2019
Merged

Dev ignore submatches #186

merged 3 commits into from
Jun 2, 2019

Conversation

greena13
Copy link
Owner

@greena13 greena13 commented Jun 2, 2019

Context

The current action matching behaviour of react-hotkeys has caused confusion and seemed to contradict user's expectations a couple of times (#161, #181, #175): It first examines <HotKeys/> components closest to the element currently in focus, and then moves towards the top of the app, examining the full hierarchy of focused <HotKeys /> components, before moving on to <GlobalHotkeys/>.

The trouble is that submatches are allowed in this matching process: If an application has a context-dependent action, bound to a short key combination (e.g. ?) and a longer global action bound to a longer key combination (e.g. shift+?) the longer key combination is hidden behind the shorter one and never triggered whenever a child of the <HotKeys/> component that defines the shorter combination is in focus.

This pull request

  • Adds a new allowCombinationSubmatches configuration option to enable submatching and disables it by default (i.e. submatching is turned off by default)
  • Moves around some Readme sections and adds a more in depth description of how the key matching algorithm works, together with its customisation options and the tradeoffs.
  • Updates the test suite to cover the new expected behaviour for both configuration option settings.

src/lib/strategies/AbstractKeyEventStrategy.js Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
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.

1 participant