Skip to content

Commit

Permalink
Merge pull request #25625 from storybookjs/charles-update-shortcuts
Browse files Browse the repository at this point in the history
Shortcuts: Require modifier key to trigger shortcuts (`F`,`A`,`D`,`S`,`T`,`/`)
  • Loading branch information
cdedreuille authored Jan 18, 2024
2 parents 03956f1 + 5acdeb6 commit b03191d
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 46 deletions.
23 changes: 14 additions & 9 deletions MIGRATION.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<h1>Migration</h1>

- [From version 7.x to 8.0.0](#from-version-7x-to-800)
- [Default keyboard shortcuts changed](#default-keyboard-shortcuts-changed)
- [Manager addons are now rendered with React 18](#manager-addons-are-now-rendered-with-react-18)
- [Removal of `storiesOf`-API](#removal-of-storiesof-api)
- [Removed deprecated shim packages](#removed-deprecated-shim-packages)
Expand Down Expand Up @@ -70,17 +71,17 @@
- [Addon author changes](#addon-author-changes)
- [Removed `config` preset](#removed-config-preset-1)
- [From version 7.5.0 to 7.6.0](#from-version-750-to-760)
- [CommonJS with Vite is deprecated](#commonjs-with-vite-is-deprecated)
- [Using implicit actions during rendering is deprecated](#using-implicit-actions-during-rendering-is-deprecated)
- [typescript.skipBabel deprecated](#typescriptskipbabel-deprecated)
- [Primary doc block accepts of prop](#primary-doc-block-accepts-of-prop)
- [Addons no longer need a peer dependency on React](#addons-no-longer-need-a-peer-dependency-on-react)
- [CommonJS with Vite is deprecated](#commonjs-with-vite-is-deprecated)
- [Using implicit actions during rendering is deprecated](#using-implicit-actions-during-rendering-is-deprecated)
- [typescript.skipBabel deprecated](#typescriptskipbabel-deprecated)
- [Primary doc block accepts of prop](#primary-doc-block-accepts-of-prop)
- [Addons no longer need a peer dependency on React](#addons-no-longer-need-a-peer-dependency-on-react)
- [From version 7.4.0 to 7.5.0](#from-version-740-to-750)
- [`storyStoreV6` and `storiesOf` is deprecated](#storystorev6-and-storiesof-is-deprecated)
- [`storyIndexers` is replaced with `experimental_indexers`](#storyindexers-is-replaced-with-experimental_indexers)
- [`storyStoreV6` and `storiesOf` is deprecated](#storystorev6-and-storiesof-is-deprecated)
- [`storyIndexers` is replaced with `experimental_indexers`](#storyindexers-is-replaced-with-experimental_indexers)
- [From version 7.0.0 to 7.2.0](#from-version-700-to-720)
- [Addon API is more type-strict](#addon-api-is-more-type-strict)
- [Addon-controls hideNoControlsWarning parameter is deprecated](#addon-controls-hidenocontrolswarning-parameter-is-deprecated)
- [Addon API is more type-strict](#addon-api-is-more-type-strict)
- [Addon-controls hideNoControlsWarning parameter is deprecated](#addon-controls-hidenocontrolswarning-parameter-is-deprecated)
- [From version 6.5.x to 7.0.0](#from-version-65x-to-700)
- [7.0 breaking changes](#70-breaking-changes)
- [Dropped support for Node 15 and below](#dropped-support-for-node-15-and-below)
Expand Down Expand Up @@ -384,6 +385,10 @@

## From version 7.x to 8.0.0

### Default keyboard shortcuts changed

The default keyboard shortcuts have changed to avoid any conflicts with the browser's default shortcuts or when you are directly typing in the Manager. If you want to get the new default shortcuts, you can reset your shortcuts in the keyboard shortcuts panel by pressing the `Restore default` button.

### Manager addons are now rendered with React 18

The UI added to the manager via addons is now rendered with React 18.
Expand Down
4 changes: 2 additions & 2 deletions code/addons/outline/src/OutlineSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ export const OutlineSelector = memo(function OutlineSelector() {

useEffect(() => {
api.setAddonShortcut(ADDON_ID, {
label: 'Toggle Outline [O]',
defaultShortcut: ['O'],
label: 'Toggle Outline',
defaultShortcut: ['alt', 'O'],
actionName: 'outline',
showInMenu: false,
action: toggleOutline,
Expand Down
8 changes: 4 additions & 4 deletions code/addons/viewport/src/shortcuts.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { API } from '@storybook/manager-api';
import { type API } from '@storybook/manager-api';
import { ADDON_ID } from './constants';
import { globals as defaultGlobals } from './preview';

Expand Down Expand Up @@ -27,7 +27,7 @@ export const registerShortcuts = async (
) => {
await api.setAddonShortcut(ADDON_ID, {
label: 'Previous viewport',
defaultShortcut: ['shift', 'V'],
defaultShortcut: ['alt', 'shift', 'V'],
actionName: 'previous',
action: () => {
updateGlobals({
Expand All @@ -38,7 +38,7 @@ export const registerShortcuts = async (

await api.setAddonShortcut(ADDON_ID, {
label: 'Next viewport',
defaultShortcut: ['V'],
defaultShortcut: ['alt', 'V'],
actionName: 'next',
action: () => {
updateGlobals({
Expand All @@ -49,7 +49,7 @@ export const registerShortcuts = async (

await api.setAddonShortcut(ADDON_ID, {
label: 'Reset viewport',
defaultShortcut: ['alt', 'V'],
defaultShortcut: ['alt', 'control', 'V'],
actionName: 'reset',
action: () => {
updateGlobals(defaultGlobals);
Expand Down
26 changes: 13 additions & 13 deletions code/e2e-tests/manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ test.describe('Manager UI', () => {
await expect(sbPage.page.locator('.sidebar-container')).toBeVisible();

// toggle with keyboard shortcut
await sbPage.page.locator('html').press('s');
await sbPage.page.locator('html').press('Alt+s');
await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();
await sbPage.page.locator('html').press('s');
await sbPage.page.locator('html').press('Alt+s');
await expect(sbPage.page.locator('.sidebar-container')).toBeVisible();

// toggle with menu item
Expand Down Expand Up @@ -51,9 +51,9 @@ test.describe('Manager UI', () => {
await expectToolbarVisibility(true);

// toggle with keyboard shortcut
await sbPage.page.locator('html').press('t');
await sbPage.page.locator('html').press('Alt+t');
await expectToolbarVisibility(false);
await sbPage.page.locator('html').press('t');
await sbPage.page.locator('html').press('Alt+t');
await expectToolbarVisibility(true);

// toggle with menu item
Expand All @@ -75,9 +75,9 @@ test.describe('Manager UI', () => {
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();

// toggle with keyboard shortcut
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Alt+a');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Alt+a');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();
});

Expand All @@ -90,9 +90,9 @@ test.describe('Manager UI', () => {
await expect(sbPage.page.locator('#storybook-panel-root')).toBeVisible();

// toggle with keyboard shortcut
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Alt+a');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Alt+a');
await expect(sbPage.page.locator('#storybook-panel-root')).toBeVisible();

// toggle with menu item
Expand All @@ -114,16 +114,16 @@ test.describe('Manager UI', () => {
await expect(sbPage.page.locator('#storybook-panel-root')).toBeVisible();

// toggle position with keyboard shortcut
await sbPage.page.locator('html').press('d');
await sbPage.page.locator('html').press('Alt+d');
await expect(sbPage.page.locator('#storybook-panel-root')).toBeVisible();
// TODO: how to assert panel position?

// hide with keyboard shortcut
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Alt+a');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();

// toggling position should also show the panel again
await sbPage.page.locator('html').press('d');
await sbPage.page.locator('html').press('Alt+d');
await expect(sbPage.page.locator('#storybook-panel-root')).toBeVisible();
});
});
Expand All @@ -138,11 +138,11 @@ test.describe('Manager UI', () => {
await expect(sbPage.page.locator('.sidebar-container')).toBeVisible();

// toggle with keyboard shortcut
await sbPage.page.locator('html').press('f');
await sbPage.page.locator('html').press('Alt+f');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();
await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();

await sbPage.page.locator('html').press('f');
await sbPage.page.locator('html').press('Alt+f');
await expect(sbPage.page.locator('#storybook-panel-root')).toBeVisible();
await expect(sbPage.page.locator('.sidebar-container')).toBeVisible();

Expand Down
4 changes: 2 additions & 2 deletions code/e2e-tests/preview-web.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test.describe('preview-web', () => {

// click outside, to remove focus from the input of the story, then press S to toggle sidebar
await sbPage.previewRoot().click();
await sbPage.previewRoot().press('s');
await sbPage.previewRoot().press('Alt+s');
await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();
});

Expand All @@ -47,7 +47,7 @@ test.describe('preview-web', () => {

await expect(sbPage.page.locator('.sidebar-container')).toBeVisible();

await sbPage.previewRoot().getByRole('button').getByText('Submit').first().press('s');
await sbPage.previewRoot().getByRole('button').getByText('Submit').first().press('Alt+s');
await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();
});
});
14 changes: 7 additions & 7 deletions code/lib/manager-api/src/modules/shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,12 @@ type API_AddonShortcutLabels = Record<string, string>;
type API_AddonShortcutDefaults = Record<string, API_KeyCollection>;

export const defaultShortcuts: API_Shortcuts = Object.freeze({
fullScreen: ['F'],
togglePanel: ['A'],
panelPosition: ['D'],
toggleNav: ['S'],
toolbar: ['T'],
search: ['/'],
fullScreen: ['alt', 'F'],
togglePanel: ['alt', 'A'],
panelPosition: ['alt', 'D'],
toggleNav: ['alt', 'S'],
toolbar: ['alt', 'T'],
search: [controlOrMetaKey(), 'K'],
focusNav: ['1'],
focusIframe: ['2'],
focusPanel: ['3'],
Expand All @@ -139,7 +139,7 @@ export const defaultShortcuts: API_Shortcuts = Object.freeze({
prevStory: ['alt', 'ArrowLeft'],
nextStory: ['alt', 'ArrowRight'],
shortcutsPage: [controlOrMetaKey(), 'shift', ','],
aboutPage: [','],
aboutPage: [controlOrMetaKey(), ','],
escape: ['escape'], // This one is not customizable
collapseAll: [controlOrMetaKey(), 'shift', 'ArrowUp'],
expandAll: [controlOrMetaKey(), 'shift', 'ArrowDown'],
Expand Down
16 changes: 8 additions & 8 deletions code/lib/manager-api/src/tests/shortcuts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('shortcuts api', () => {
const { api, state } = initShortcuts({ store });
store.setState(state);

expect(api.getDefaultShortcuts()).toHaveProperty('fullScreen', ['F']);
expect(api.getDefaultShortcuts()).toHaveProperty('fullScreen', ['alt', 'F']);
});

it('gets defaults including addon ones', async () => {
Expand All @@ -67,7 +67,7 @@ describe('shortcuts api', () => {
await api.setAddonShortcut(mockAddonSecondShortcut.addon, mockAddonSecondShortcut.shortcut);
await api.setAddonShortcut(mockSecondAddonShortcut.addon, mockSecondAddonShortcut.shortcut);

expect(api.getDefaultShortcuts()).toHaveProperty('fullScreen', ['F']);
expect(api.getDefaultShortcuts()).toHaveProperty('fullScreen', ['alt', 'F']);
expect(api.getDefaultShortcuts()).toHaveProperty(
`${mockAddonShortcut.addon}-${mockAddonShortcut.shortcut.actionName}`,
mockAddonShortcut.shortcut.defaultShortcut
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('shortcuts api', () => {
const { api, state } = initShortcuts({ store });
store.setState(state);

expect(api.getShortcutKeys().fullScreen).toEqual(['F']);
expect(api.getShortcutKeys().fullScreen).toEqual(['alt', 'F']);
});

it('sets addon shortcut with default value', async () => {
Expand All @@ -161,7 +161,7 @@ describe('shortcuts api', () => {
await api.setAddonShortcut(mockAddonSecondShortcut.addon, mockAddonSecondShortcut.shortcut);
await api.setAddonShortcut(mockSecondAddonShortcut.addon, mockSecondAddonShortcut.shortcut);

expect(api.getDefaultShortcuts()).toHaveProperty('fullScreen', ['F']);
expect(api.getDefaultShortcuts()).toHaveProperty('fullScreen', ['alt', 'F']);
expect(api.getDefaultShortcuts()).toHaveProperty(
`${mockAddonShortcut.addon}-${mockAddonShortcut.shortcut.actionName}`,
mockAddonShortcut.shortcut.defaultShortcut
Expand All @@ -184,7 +184,7 @@ describe('shortcuts api', () => {
store.setState(state);

expect(api.getShortcutKeys().fullScreen).toEqual(['Z']);
expect(api.getShortcutKeys().togglePanel).toEqual(['A']);
expect(api.getShortcutKeys().togglePanel).toEqual(['alt', 'A']);
});

it('sets defaults, ignoring anything persisted that is out of date', () => {
Expand Down Expand Up @@ -234,8 +234,8 @@ describe('shortcuts api', () => {
await api.setShortcut(`${addon}-${shortcut.actionName}`, ['I']);

await api.restoreAllDefaultShortcuts();
expect(api.getShortcutKeys().fullScreen).toEqual(['F']);
expect(api.getShortcutKeys().togglePanel).toEqual(['A']);
expect(api.getShortcutKeys().fullScreen).toEqual(['alt', 'F']);
expect(api.getShortcutKeys().togglePanel).toEqual(['alt', 'A']);
expect(api.getShortcutKeys()[`${addon}-${shortcut.actionName}`]).toEqual(
shortcut.defaultShortcut
);
Expand Down Expand Up @@ -269,7 +269,7 @@ describe('shortcuts api', () => {
`${mockAddonShortcut.addon}-${mockAddonShortcut.shortcut.actionName}`
);

expect(api.getShortcutKeys().fullScreen).toEqual(['F']);
expect(api.getShortcutKeys().fullScreen).toEqual(['alt', 'F']);
expect(api.getShortcutKeys().togglePanel).toEqual(['B']);
expect(
api.getShortcutKeys()[`${mockAddonShortcut.addon}-${mockAddonShortcut.shortcut.actionName}`]
Expand Down
19 changes: 18 additions & 1 deletion code/ui/manager/src/components/sidebar/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,15 @@ const FocusKey = styled.code(({ theme }) => ({
color: theme.base === 'light' ? theme.color.dark : theme.textMutedColor,
userSelect: 'none',
pointerEvents: 'none',
display: 'flex',
alignItems: 'center',
gap: 4,
}));

const FocusKeyCmd = styled.span({
fontSize: '14px',
});

const ClearIcon = styled.div(({ theme }) => ({
position: 'absolute',
top: 0,
Expand Down Expand Up @@ -352,7 +359,17 @@ export const Search = React.memo<{
</SearchIconWrapper>
{/* @ts-expect-error (TODO) */}
<Input {...inputProps} />
{enableShortcuts && !isOpen && <FocusKey>{searchShortcut}</FocusKey>}
{enableShortcuts && !isOpen && (
<FocusKey>
{searchShortcut === '⌘ K' ? (
<>
<FocusKeyCmd></FocusKeyCmd>K
</>
) : (
searchShortcut
)}
</FocusKey>
)}
{isOpen && (
<ClearIcon onClick={() => clearSelection()}>
<CloseIcon />
Expand Down

0 comments on commit b03191d

Please sign in to comment.