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

Key Binding API #1885

Merged
merged 7 commits into from
Nov 29, 2017
Merged

Key Binding API #1885

merged 7 commits into from
Nov 29, 2017

Conversation

denehyg
Copy link
Contributor

@denehyg denehyg commented Apr 30, 2017

Adds a Key Binding API that allows plugins to programmatically add and remove key bindings to Reveal's key handling.

  • plugins can make use of Reveal's pre-processing logic for key handling so they don't have to replicate (for example, ignoring key presses when paused)
  • (optionally) includes key in the help overlay. Unlike Reveal.registerKeyboardShortcut() when key bindings are removed they are also removed from the help overlay
  • reduces the need for plugins to directly add keydown event listeners that can't be overridden by user defined bindings using keyboard config option
  • incorporates API usage into notes plugin (should fix Keyboard binding "s" does not override Speaker Notes #1832 and Keyboard Bindings Trigger when using electron <webview> tag #1630)
  • passes keydown event to config.keyboardCondition() to allow plugins to filter keyboard based on keycodes. Use case: the reveal.js-menu plugin blocks Reveal keyboard interaction (using keyboardCondition) when the menu is open but using the key binding API to toggle the menu fails to close the menu unless the 'm' key is allowed to pass through to Reveal.

@denehyg
Copy link
Contributor Author

denehyg commented Nov 25, 2017

This will also provide access for plugins to respond to Reveal.triggerKey(), which is currently not possible as triggerKey() directly invokes onDocumentKeyDown() without generating key press. For example, Reveal.triggerKey(83) should open the notes window, but is ignored.

@hakimel hakimel merged commit b86b667 into hakimel:dev Nov 29, 2017
@hakimel
Copy link
Owner

hakimel commented Nov 29, 2017

This is a great addition. Thanks for a well structured PR with docs, comments and all 🙌

@denehyg
Copy link
Contributor Author

denehyg commented Nov 29, 2017

You’re welcome, glad it was useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants