API: Add addon keyboard shortcuts & create shortcuts for addon-viewport

[Dschungelabenteuer]

Issue: #14401

What I did

This is my very first time going deeper into Storybook's source code and architecture. I'm therefore not confident about all this but I really wanted to give it a try to learn and improve. My french fellow @kaelig* wanted a way to cycle through viewports using keyboard shortcuts, which is a great idea to save some time and efforts while visually testing components.

Defining this kind of addon-specific shortcuts should be the addon's responsibility: we obviously don't want to make them available if the relevant addon is not actually used and specified into Storybook's main.js configuration file. Unfortunately, I could not find any way to create addon-specific shortcuts. At the moment, it looks like Storybook does not offer addon developers such a feature, which could be really interesting way beyond addon-viewport.

This is why I've tried to go a bit further than the original issue's feature request by extending the shortcut API so that addon developers can provide their own shortcuts.

Extending the shortcut API

Storybook provides a shortcut API which exposes methods to get, set and reset shortcuts. This API is pretty narrow/restricted since it relies on some hard-coded and core-related values (e.g. go fullscreen, show/hide addons, etc.)

setAddonShortcut(addon, shortcut)

First, I've added a setAddonShortcut method which can be used to set addon-specific shortcuts. When called, it creates (or overwrites) a shortcut key whose name is a simple concatenation of the actual addon id and an actionName to avoid potential duplicates across different addons. It also feeds a global addonsShortcuts object which will be consumed later on to actually execute the relevant action and get the shortcut's description and default value.

This method requires two arguments :

1. addon: a string which should correspond to the relevant addon's id (often set as a ADDON_ID constant).
2. shortcut: an object of type AddonShortcut which describes the actual shortcut.

This shortcut argument is explicitely typed (see the AddonShortcut interface):

• label: Label of the shortcut which will be displayed in Storybook's keyboard shortcuts setting page/the menu.
• defaultShortcut: the KeyCollection value which defines the expected keyboard shortcut key to apply the relevant action.
• actionName: the action name which should be unique within a single addon.
• showInMenu: a boolean which determines whether the shortcut should be displayed inside Storybook's menu. (optional and defaults to false).
• action: the actual action that should be triggered when pressing the shortcut key.

Display and customize the addon shortcuts

At this point, the addon shortcuts should be working correctly if correctly set from any addon. However, Storybook:

1. Lets the user customize all the shortcuts from the Keyboard shortcuts page.
2. Displays some of the shortcuts into the menu (accessible from the meatballs button (···))

Customize addon shortcuts

The Keyboard shortcuts page browses shortcutKeys to build a list of all customizable shortcuts. This shortcutKeys now includes our addon-specific shortcuts, which is pretty useful for the users to customize both core and addon-specific shortcuts . However, until now, every single shortcut had its label hard-coded through a shortcutLabels const passed as a prop to the ShortcutsScreen component.

I therefore needed a way to fetch the addon-specific shortcuts' labels/titles: this is why I've added both getAddonsShortcuts and getAddonsShortcutLabels methods to the shortcut API. They are used to pass addon-specific shortcuts' labels through an addonsShortcutLabels prop to the ShortcutScreen component. This component will now try to display a shortcut's hard-coded label and if it does not exist, will try to display an addon-specific shortcut label.

Customization and persistence are automatically handled by the existing shortcut API.

Reset shortcuts to their default values.

Similarly, core shortcuts' default values are hard-coded. I had to add getAddonsShortcutDefaults to get default values for addon shortcuts, and a getDefaultShortcuts which returns a merged object of both core and addon-specific shortcuts' default values to make restoreAllDefaultShortcuts and restoreDefaultShortcut work properly in both cases.

Display shortcuts into Storybook's menu

I also made it possible to display addon-specific shortcuts into the Storybook's menu. When adding an addon-specific shortcut, you just have to set the showInMenu property to true. It will also make use of the getAddonsShortcuts and getAddonsShortcutLabels methods from the updated shortcut API to display the right label.

Adding addon-viewport shortcuts

I've used the updated shortcut API to add three different shortcuts to the viewport addon:

• shift + v to set to the previous viewport.
• v to set to the next viewport.
• ctrl + v to reset the viewport (not referenced in the original issue).

These shortcuts are registered one at a time from an async function (registerShortcuts). To be honest, I did not really know the best way to call this function: I just assumed it had to be called within the Functional Component since calling useStorybookApi anywhere else would raise an error. I eventually called it inside a useEffect hook without an empty dep array to avoid useless calls. Unfortunately, it still gets called twice instead of just once and I'm not sure why.

Pitfalls and notes

• I still have no clear idea on how to handle shortcut conflicts.
• If this eventually gets accepted and merged: we should maybe update the Create an addon tutorial
• I have updated the addon-viewport to list the different shortcuts (previous viewport, next viewport, clear viewport).
• I have updated the shortcut api's unit tests but I've never been that much comfortable with unit testing so this might be irrelevant). I used three different mocks : two different shortcuts from the same fake addon, and another one from another fake addon, just in case we want to make sure everything works fine with multiple addons and multiple shortcuts on a single addon.

How to test

• Is this testable with Jest or Chromatic screenshots? no
• Does this need a new example in the kitchen sink apps? no
• Does this need an update to the documentation? yes

(*) I'm still waiting for that design tokens roundtable transcript kaelig, ne m'oublie pas please ! 😛

Edit 24/04/2021 : I remove the WIP status: I need some feedback which is not likely to happen from a draft state. Please do not hesitate pointing out anythig I could have done wrong and help me improve, <3

[kaelig]

Amazing, thank you for taking this on!

Quick feedback: it might bother Windows and Linux users that the addon takes over CTRL+V 😅

[shilman]

This is so awesome @Dschungelabenteuer, tremendous work! 💯

Before merging, I'd like to get a review from @ndelangen on the code, which LGTM, and from @jonniebigodes @winkerVSbecks on the addons API documentation.

[Dschungelabenteuer]

Thank you for your feedbacks, really appreciated! @kaelig I have to admit I told myself this would not be much of a problem since shortcuts do not apply when focus is set on inputs, Any suggestions?

[kaelig]

When a custom list of viewports is set, will it be registered? (It's a dynamic list, and can completely change between two stories)

[Dschungelabenteuer]

Oh good catch, I had not thought of that at all! Thank you for pointing that out, I'll be fixing it as soon as possible! Edit: custom viewports are now correctly registered 🎉

[kaelig]

I have a few questions, this is looking great. I haven't tested it yet.

[winkerVSbecks]

LGTM

[jonniebigodes]

@shilman on the docs side of things it's good to me as well! @Dschungelabenteuer wow, this is so cool, thank you very much for the time and effort you put into this! Can't wait to see this one merged and have the functionality generally available for all Storybook users.

Stay safe

[Dschungelabenteuer]

Should I update the lib\ui\src\components\sidebar\Menu.stories.tsx file so that UI Tests do not fail?

[shilman]

@Dschungelabenteuer yes please! 🙏 we can't merge until the tests pass. Let me know if you need any help with that.

[ndelangen]

First: awesome work! really cool

@Dschungelabenteuer @kaelig If we're talking about shortcuts.
I wonder if we should (possible long term change) start configuring shortcuts via main.js instead of runtime. (the initial shortcuts).

Then when users want to change, they can change it for everyone (in main.js), OR they change it in the UI, will will create an overload in their localStorage.

This is not a required change for this PR, but something I think we could do, if we're touching the code anyway.

The setAddonShortcut api would not be required at all, since addons would be able to ADD shortcuts via presets instead.

[shilman]

@Dschungelabenteuer this looks mergeable to me. Do you want to change "ctl-v" or should I merge as is?

[Dschungelabenteuer]

@Dschungelabenteuer this looks mergeable to me. Do you want to change "ctl-v" or should I merge as is?

@shilman It would have worked fine, but I changed it to alt + v to avoid any confusion with our holy ctrl + v 😅

@ndelangen I guess this could be a nice thing! I could try some things out in another branch later, and if I eventually come up with something interesting I could refere this pull request and maybe create another pull request?

(I am really sorry for closing and re-opening the PR, I misclicked...)

[ndelangen]

👏

[kaelig]

This doesn't work for me on macOS/Chrome.

When I override the shortcut, instead of V, it shows √.

Video:

Screen.Recording.2021-04-29.at.1.46.09.PM.mov

[Dschungelabenteuer]

I'm very sorry for that, Kaelig 😞 Is this alt + v combination a macOS's built-in shortcut?

[kaelig]

Yes, the option key is very commonly used in macOS to provide alternative characters or alternative key functionality (which is a feature I love and I miss a lot when I use Windows).

[shilman]

@apapadopoulou looks like this is where the shortcuts get added?

[apapadopoulou]

Yes, thank you!