Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shortcuts: Require modifier key to trigger shortcuts (F,A,D,S,T,/) #25625

Merged
merged 11 commits into from
Jan 18, 2024
Merged

Shortcuts: Require modifier key to trigger shortcuts (F,A,D,S,T,/) #25625

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6584aa4
Update shortcuts.ts
cdedreuille Jan 17, 2024
d561fac
Modify some addons shortcuts
cdedreuille Jan 17, 2024
bf7f397
Update manager.spec.ts
cdedreuille Jan 17, 2024
84e7933
Update preview-web.spec.ts
cdedreuille Jan 17, 2024
de1d939
Update shortcuts.test.js
cdedreuille Jan 17, 2024
f75bbf1
Update shortcuts.test.js
cdedreuille Jan 17, 2024
edc3da3
Update to alt
cdedreuille Jan 17, 2024
ccc3fc4
Update MIGRATION.md
cdedreuille Jan 17, 2024
428d030
Update Search.tsx
cdedreuille Jan 17, 2024
6c386be
Update shortcuts
cdedreuille Jan 18, 2024
5acdeb6
Merge branch 'next' into charles-update-shortcuts
cdedreuille Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions code/addons/outline/src/OutlineSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { memo, useCallback, useEffect } from 'react';
import { useGlobals, useStorybookApi } from '@storybook/manager-api';
import { controlOrMetaKey, useGlobals, useStorybookApi } from '@storybook/manager-api';
import { IconButton } from '@storybook/components';
import { OutlineIcon } from '@storybook/icons';
import { ADDON_ID, PARAM_KEY } from './constants';
Expand All @@ -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: [controlOrMetaKey(), 'O'],
actionName: 'outline',
showInMenu: false,
action: toggleOutline,
Expand Down
4 changes: 2 additions & 2 deletions code/addons/viewport/src/shortcuts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { API } from '@storybook/manager-api';
import { controlOrMetaKey, type API } from '@storybook/manager-api';
import { ADDON_ID } from './constants';
import { globals as defaultGlobals } from './preview';

Expand Down Expand Up @@ -38,7 +38,7 @@ export const registerShortcuts = async (

await api.setAddonShortcut(ADDON_ID, {
label: 'Next viewport',
defaultShortcut: ['V'],
defaultShortcut: [controlOrMetaKey(), 'V'],
actionName: 'next',
action: () => {
updateGlobals({
Expand Down
26 changes: 13 additions & 13 deletions code/e2e-tests/manager.spec.ts
View file
Open in desktop
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('Control+s');
await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();
await sbPage.page.locator('html').press('s');
await sbPage.page.locator('html').press('Control+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('Control+t');
await expectToolbarVisibility(false);
await sbPage.page.locator('html').press('t');
await sbPage.page.locator('html').press('Control+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('Control+a');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Control+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('Control+a');
await expect(sbPage.page.locator('#storybook-panel-root')).not.toBeVisible();
await sbPage.page.locator('html').press('a');
await sbPage.page.locator('html').press('Control+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('Control+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('Control+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('Control+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('Control+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('Control+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
View file
Open in desktop
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('Control+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('Control+s');
await expect(sbPage.page.locator('.sidebar-container')).not.toBeVisible();
});
});
12 changes: 6 additions & 6 deletions code/lib/manager-api/src/modules/shortcuts.ts
View file
Open in desktop
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: [controlOrMetaKey(), 'F'],
togglePanel: [controlOrMetaKey(), 'A'],
panelPosition: [controlOrMetaKey(), 'D'],
toggleNav: [controlOrMetaKey(), 'S'],
toolbar: [controlOrMetaKey(), 'T'],
search: [controlOrMetaKey(), 'K'],
cdedreuille marked this conversation as resolved.
Show resolved Hide resolved
focusNav: ['1'],
focusIframe: ['2'],
focusPanel: ['3'],
Expand Down
16 changes: 8 additions & 8 deletions code/lib/manager-api/src/tests/shortcuts.test.js
View file
Open in desktop
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', ['control', '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', ['control', '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(['control', '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', ['control', '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(['control', '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(['control', 'F']);
expect(api.getShortcutKeys().togglePanel).toEqual(['control', '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(['control', 'F']);
expect(api.getShortcutKeys().togglePanel).toEqual(['B']);
expect(
api.getShortcutKeys()[`${mockAddonShortcut.addon}-${mockAddonShortcut.shortcut.actionName}`]
Expand Down