An attempt at some code cleanup, renaming things

[mirisuzanne]

Description

Uncomment if you want to provide a custom description.

• Mostly renaming and simplifying methods, for consistency
• The component itself is only aware of two 'built-in' views, named presentation (used for start and resume events) and speaker (used for the join-as-speaker event)
• Only listen for keydown on body, and add/remove listener on connect/disconnect
• A lot of 'keydown' event-handler cleanup to try and achieve proper priority in different situations
  1. cmd/ctrl-k always works, open/closes control panel
  2. if the control panel is open, esc will blur and close the panel
  3. if a blank slide is open, any non-cmd/ctrl key will close it (this allows you to open the control panel while a blank slide is shown)
  4. cmd/ctrl-enter will always resume presentation (or start from the top, with shift)
  5. alt-enter will always join presentation as speaker
  6. other events are only triggered when key-control is enabled

I was hoping to remove all known views, and allow those to be totally defined by the page author and the stylesheets. But we have useful shortcuts to start and join-as-speaker, so it seemed like those two views need to be provided. We could allow them to be changed? Or just make sure we've chosen good names. I went back and forth a bit between presentation and slideshow for the public-facing one.

Steps to test/reproduce

Please explain how to best reproduce the issue and/or test the changes locally (including the pages/URLs/views/states to review).

• try lots of key combinations in different arrangements
• try all the buttons?

[netlify[bot]]

✅ Deploy Preview for slide-deck ready!

Name	Link
🔨 Latest commit	7cfbbcb6a5e27dfb6f49fd92956fa4a39dadec32
🔍 Latest deploy log	https://app.netlify.com/sites/slide-deck/deploys/65fc950f39feaf0008590af4
😎 Deploy Preview	https://deploy-preview-43--slide-deck.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[mirisuzanne]

I made the changes based on our conversation this morning.

[mirisuzanne]

The shortcut for opening the control panel opens the search bar in Chrome and Firefox.

Oh, I accidentally lost the event.preventDefault();. Fixed in bfdc38c

Should there be a visible confirmation that keyboard navigation is turned on?

I'm not sure what sort of indication we would provide, besides the pressed state of the button (which is subtle but there). Do you have ideas?

I'm not actually sure if we need a key-control toggle? The other option is to always have keyboard control turned on. I expect that's the way most people will use this. I was just worried about conflicts like the ones you mentioned above: key-control hijacks a number of keyboard events. Not just cmd-k, but arrow keys, period, comma, etc…

When switching views, it seems to be a two step process to use keys to navigate slides:

1. Press a button to change the view
2. Press escape
3. Navigate with keys.

Is that expected?

This isn't new in this branch, but comes out of exactly the same concern. If we're hijacking common keys while another interactive element has focus that seems especially risky. In the context of an embedded demo, you press an arrow key, and the slide changes?

Maybe the current behavior is too cautious, though. I could imagine a solution that only allows form input focus to override key control, and stops propagation of keyboard events inside iframes (embeded demos)? That way you could still use keyboard controls despite a top-level link or button having focus?

[mirisuzanne]

I also think it would be helpful to provide a view-switching keyboard shortcut. Maybe we should open a new issue about that.

[mirisuzanne]

@dvdherron Oh I see, I also broke the key-control pressed state with one of the name changes. That should also be fixed now.

[stacyk]

If I am in "grid" view (by clicked the button on the first slide) and slide 4 is my active slide, when I open the command palette and click resume, what should happen?

[stacyk]

I feel like there's no feedback that a button was pressed in the control panel (except for the View: slideshow or speaker buttons).

I also can't seem to get keyboard navigation working (as in, I click the keyboard navigation button in the control panel, then close the panel, then hit arrow down and the slide doesn't move but the whole document scrolls like 30 pixels). Is that just me?

[mirisuzanne]

@stacyk

If I am in "grid" view (by clicked the button on the first slide) and slide 4 is my active slide, when I open the command palette and click resume, what should happen?

Oh hey, I broke two of those command-panel buttons. 🤦🏻‍♀️ Thanks for catching it. They should be fixed. Resume should take you into presentation view, turning on key-control and follow-active.

I feel like there's no feedback that a button was pressed in the control panel (except for the View: slideshow or speaker buttons).

Yeah, several of these buttons aren't 'toggles' - they don't control a single thing on-or-off. So it's hard to know what they should show. Another issue here is that key-control and follow-active (the things that change in addition to the view) are kinda invisible and abstract concepts? Do we need to think a bit more about the UX around those?

But it doesn't help that two of these buttons were just totally broken and doing nothing. Does it feel better now that they actually do something?

I also can't seem to get keyboard navigation working (as in, I click the keyboard navigation button in the control panel, then close the panel, then hit arrow down and the slide doesn't move but the whole document scrolls like 30 pixels). Is that just me?

Key-control is on by default. When you click the keyboard navigation button, you might be turning it off?

[dvdherron]

The shortcut for opening the control panel opens the search bar in Chrome and Firefox.

Oh, I accidentally lost the event.preventDefault();. Fixed in bfdc38c

👍🏽

I'm not actually sure if we need a key-control toggle? The other option is to always have keyboard control turned on. I expect that's the way most people will use this. I was just worried about conflicts like the ones you mentioned above: key-control hijacks a number of keyboard events. Not just cmd-k, but arrow keys, period, comma, etc…

Hmm, with those conflicts in mind it could be nice to keep the key-control option available.

When switching views, it seems to be a two step process to use keys to navigate slides:

1. Press a button to change the view
2. Press escape
3. Navigate with keys.

Is that expected?

This isn't new in this branch, but comes out of exactly the same concern. If we're hijacking common keys while another interactive element has focus that seems especially risky. In the context of an embedded demo, you press an arrow key, and the slide changes?

Maybe the current behavior is too cautious, though. I could imagine a solution that only allows form input focus to override key control, and stops propagation of keyboard events inside iframes (embeded demos)? That way you could still use keyboard controls despite a top-level link or button having focus?

I think this sounds like a good approach.

[mirisuzanne]

ok, I pushed a less cautious approach to keyboard controls. Happy for anyone to review - but especially @jamesnw or @jgerigmeyer.

[mirisuzanne]

@jamesnw Yes, that describes the idea correctly. And yeah, I had the same thought. Maybe I'll open a new issue for it, though. We can call this a good start?