Change keyboard shortcuts to platform safe

[Hypnosphi]

Issue: #403

See background: #403 (comment)

It turned out that ⌘ ⇧ Q is "quit all apps" on macOS, and ⌘ ⇧ E just doesn't work, so I used X for stories panel and O (the closest one to previous P) for search

[codecov]

Codecov Report

Merging #1858 into release/3.3 will decrease coverage by 1.25%.
The diff coverage is 0%.

@@               Coverage Diff               @@
##           release/3.3    #1858      +/-   ##
===============================================
- Coverage        23.47%   22.22%   -1.26%     
===============================================
  Files              311      326      +15     
  Lines             6538     6539       +1     
  Branches           827      823       -4     
===============================================
- Hits              1535     1453      -82     
- Misses            4399     4462      +63     
- Partials           604      624      +20

Impacted Files	Coverage Δ
lib/ui/src/modules/ui/components/shortcuts_help.js	40.9% <ø> (ø)	⬆️
lib/ui/src/libs/key_events.js	22.22% <0%> (-1.04%)	⬇️
addons/knobs/src/components/types/Text.js	50% <0%> (-16.67%)	⬇️
addons/knobs/src/components/types/Array.js	84.61% <0%> (-8.72%)	⬇️
addons/knobs/src/react/WrapStory.js	12% <0%> (-7.55%)	⬇️
addons/info/src/components/PropTable.js	22.22% <0%> (-2.78%)	⬇️
addons/knobs/src/KnobStore.js	6.81% <0%> (-2.28%)	⬇️
lib/ui/src/modules/ui/components/layout/index.js	76.27% <0%> (-1.7%)	⬇️
addons/knobs/src/vue/index.js	22.72% <0%> (-1.42%)	⬇️
addons/info/src/components/Props.js	36.36% <0%> (-0.85%)	⬇️
... and 185 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d92d0c...167e9d7. Read the comment docs.

[ndelangen]

I kind of feel this is not the best move to make.
If we're going to change the the shortcut, we should come up with a good system. This just moves some shortcuts to other letters that make less sense, and just happen to not collide with native ones (probably because the letters do not make much sense)

https://medium.com/@sashika/j-k-or-how-to-choose-keyboard-shortcuts-for-web-applications-a7c3b7b408ee

[Hypnosphi]

less sense

What do P in search and J in addon position stand for?

[Hypnosphi]

Letters only shortcuts

Actually I like this idea! We already block shortcuts when an input or textarea is focused, so it shouldn't interfere with text input. I'd suggest following shortcuts then:

• Q(uery) for search
• A(ddons) for addons panel
• S(tories) for stories panel
• D(own) and R(ight) for addons panel position. Both letters should act the same way, i.e. one can press R to move the panel to the right, and the second R press will move it back down, same thing with D
• F(ullscreen) for fullscreen
• ? for showing the shortcuts help

Note that all the letters are quite close on keyboard

[danielduan]

I think we have to be careful here to not interfere with the story's keydown/keyup events if a story component has it.

ctrl + shift + x gives us some protection if a story itself contains a component that has a keydown handler.

Those keyboard shortcuts make sense to me.

[Hypnosphi]

not interfere with the story's keydown/keyup events

I think we can introduce some way to mark element so that it blocks shortcuts when focused, like it's done for all the input fields by default. Maybe some data attribute like data-block-storybook-shortcuts.

[danielduan]

I'm gonna okay this one because I think it makes sense and fixes some old issues.

[codecov-io]

Codecov Report

Merging #1858 into release/3.3 will decrease coverage by <.01%.
The diff coverage is 0%.

@@               Coverage Diff               @@
##           release/3.3    #1858      +/-   ##
===============================================
- Coverage        22.74%   22.74%   -0.01%     
===============================================
  Files              326      326              
  Lines             6748     6750       +2     
  Branches           845      841       -4     
===============================================
  Hits              1535     1535              
- Misses            4579     4601      +22     
+ Partials           634      614      -20

Impacted Files	Coverage Δ
lib/ui/src/modules/ui/components/shortcuts_help.js	40.9% <ø> (ø)	⬆️
lib/ui/src/libs/key_events.js	22.22% <0%> (-1.04%)	⬇️
app/react/src/server/config/babel.js	0% <0%> (-100%)	⬇️
addons/info/src/components/PropTable.js	25% <0%> (ø)	⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js	22.03% <0%> (ø)	⬆️
addons/knobs/src/components/types/Color.js	8.21% <0%> (ø)	⬆️
lib/components/src/highlight_button.js	0% <0%> (ø)	⬆️
addons/info/src/components/types/ArrayOf.js	14.28% <0%> (ø)	⬆️
app/react-native/src/bin/storybook-build.js	0% <0%> (ø)	⬆️
addons/knobs/src/components/types/Boolean.js	11.62% <0%> (ø)	⬆️
... and 35 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c665e2...6647840. Read the comment docs.

[timgivois]

why did we use cmd + shift + c.... it is inspect element shortcut in chrome, it's REALLY annoying....

[Hypnosphi]

Oh I see, my fault. I wish it was documented here
https://support.google.com/chrome/answer/157179?hl=en

[benediktvaldez]

It's also 'Start element selection' in Safari (essentially the same as selecting an element for inspection).

[Hypnosphi]

Ok looks like we need to switch to letter only shortcuts
#1858 (comment)

If anyone wants to open a PR, go for it

[apertureless]

Well, why not make it configurable over addon-options?
It is really tough to find a cross os + cross browser all in one solution.