Handle Ctrl and Cmd in keyboard events #18
Conversation
✅ Deploy Preview for slide-deck ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
slide-deck.js
Outdated
| @@ -455,9 +455,12 @@ class slideDeck extends HTMLElement { | |||
| this.goTo(this.slideFromStore()); | |||
| } | |||
|
|
|||
| // Detect Ctrl / Cmd modifiers in a platform-agnostic way | |||
| cmdOrCtrl = (event) => event.ctrlKey || event.metaKey; | |||
There was a problem hiding this comment.
We could try to be clever and do platform sniffing to respond to only one of these, but from a quick investigation and issues like this, it seems like just handling both should be enough.
slide-deck.js
Outdated
| @@ -455,9 +455,12 @@ class slideDeck extends HTMLElement { | |||
| this.goTo(this.slideFromStore()); | |||
| } | |||
|
|
|||
| // Detect Ctrl / Cmd modifiers in a platform-agnostic way | |||
| cmdOrCtrl = (event) => event.ctrlKey || event.metaKey; | |||
There was a problem hiding this comment.
I think this should be a private method (the other PR is making most of the others private, so this would match that, eventually).
@dvdherron What browser are you using, and what shows in the console when you try it? Any chance you're running into #1 |
Hmm. That's interesting. I get the same response in Chrome and Firefox. I tried this with the And once I get the slides to go fullscreen with the shortcut I can only see the first slide. Is that expected? |
Strange. I'm not sure how to debug that. I'm also not sure if this is the right shortcut for fullscreen.
Nope. But that seems like a separate issue we can add to the fullscreen issue pile. |
|
I'd be real tempted to merge this and remove fullscreen support until we can get it working correctly. Fullscreen is not a feature that I expect to use in the workshop. |
Related Issue(s)
Fixes #17
Steps to test/reproduce
Use either Cmd or Ctrl for keyboard shortcuts