Skip to content

Commit

Permalink
IMPROVE memoization of menu
Browse files Browse the repository at this point in the history
  • Loading branch information
@ndelangen
ndelangen committed Feb 25, 2020
1 parent ca7d473 commit fa3a0f1
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 33 deletions.
144 changes: 114 additions & 30 deletions lib/ui/src/containers/menu.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import memoize from 'memoizerific';
import React, { useMemo } from 'react';

import { Badge } from '@storybook/components';
import { API } from '@storybook/api';

import { shortcutToHumanString } from '../libs/shortcut';
import { MenuItemIcon } from '../components/sidebar/Menu';
Expand All @@ -15,91 +15,175 @@ const focusableUIElements = {
const shortcutToHumanStringIfEnabled = (shortcuts: string[], enableShortcuts: boolean) =>
enableShortcuts ? shortcutToHumanString(shortcuts) : null;

export const createMenu = memoize(1)(
(api, shortcutKeys, isFullscreen, showPanel, showNav, enableShortcuts) => [
{
export const useMenu = (
api: API,
isFullscreen: boolean,
showPanel: boolean,
showNav: boolean,
enableShortcuts: boolean
) => {
const shortcutKeys = api.getShortcutKeys();

const sidebarToggle = useMemo(
() => ({
id: 'S',
title: 'Show sidebar',
onClick: () => api.toggleNav(),
right: shortcutToHumanStringIfEnabled(shortcutKeys.toggleNav, enableShortcuts),
left: showNav ? <MenuItemIcon icon="check" /> : <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys, showNav]
);

const addonsToggle = useMemo(
() => ({
id: 'A',
title: 'Show addons',
onClick: () => api.togglePanel(),
right: shortcutToHumanStringIfEnabled(shortcutKeys.togglePanel, enableShortcuts),
left: showPanel ? <MenuItemIcon icon="check" /> : <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys, showPanel]
);

const addonsOrientationToggle = useMemo(
() => ({
id: 'D',
title: 'Change addons orientation',
onClick: () => api.togglePanelPosition(),
right: shortcutToHumanStringIfEnabled(shortcutKeys.panelPosition, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const fullscreenToggle = useMemo(
() => ({
id: 'F',
title: 'Go full screen',
onClick: () => api.toggleFullscreen(),
right: shortcutToHumanStringIfEnabled(shortcutKeys.fullScreen, enableShortcuts),
left: isFullscreen ? 'check' : <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys, isFullscreen]
);

const searchToggle = useMemo(
() => ({
id: '/',
title: 'Search',
onClick: () => api.focusOnUIElement(focusableUIElements.storySearchField),
right: shortcutToHumanStringIfEnabled(shortcutKeys.search, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const up = useMemo(
() => ({
id: 'up',
title: 'Previous component',
onClick: () => api.jumpToComponent(-1),
right: shortcutToHumanStringIfEnabled(shortcutKeys.prevComponent, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const down = useMemo(
() => ({
id: 'down',
title: 'Next component',
onClick: () => api.jumpToComponent(1),
right: shortcutToHumanStringIfEnabled(shortcutKeys.nextComponent, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const prev = useMemo(
() => ({
id: 'prev',
title: 'Previous story',
onClick: () => api.jumpToStory(-1),
right: shortcutToHumanStringIfEnabled(shortcutKeys.prevStory, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const next = useMemo(
() => ({
id: 'next',
title: 'Next story',
onClick: () => api.jumpToStory(1),
right: shortcutToHumanStringIfEnabled(shortcutKeys.nextStory, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const about = useMemo(
() => ({
id: 'about',
title: 'About your Storybook',
onClick: () => api.navigate('/settings/about'),
right: api.versionUpdateAvailable() && <Badge status="positive">Update</Badge>,
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const shortcuts = useMemo(
() => ({
id: 'shortcuts',
title: 'Keyboard shortcuts',
onClick: () => api.navigate('/settings/shortcuts'),
right: shortcutToHumanStringIfEnabled(shortcutKeys.shortcutsPage, enableShortcuts),
left: <MenuItemIcon />,
},
{
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

const collapse = useMemo(
() => ({
id: 'collapse',
title: 'Collapse all',
onClick: () => api.collapseAll(),
right: shortcutToHumanString(shortcutKeys.collapseAll),
left: <MenuItemIcon />,
},
]
);
}),
[api, shortcutToHumanStringIfEnabled, enableShortcuts, shortcutKeys]
);

return useMemo(
() => [
sidebarToggle,
addonsToggle,
addonsOrientationToggle,
fullscreenToggle,
searchToggle,
up,
down,
prev,
next,
about,
shortcuts,
collapse,
],
[
sidebarToggle,
addonsToggle,
addonsOrientationToggle,
fullscreenToggle,
searchToggle,
up,
down,
prev,
next,
about,
shortcuts,
collapse,
]
);
};
7 changes: 4 additions & 3 deletions lib/ui/src/containers/sidebar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import rfdc from 'rfdc';
import { Consumer, Combo, StoriesHash, Story } from '@storybook/api';

import SidebarComponent from '../components/sidebar/Sidebar';
import { createMenu } from './menu';
import { useMenu } from './menu';

type Item = StoriesHash[keyof StoriesHash];

Expand Down Expand Up @@ -140,15 +140,16 @@ const Sidebar: FunctionComponent<{}> = React.memo(() => {
return DOCS_MODE ? collapseAllStories(copy) : collapseDocsOnlyStories(copy);
}, [DOCS_MODE, storiesHash]);

const shortcutKeys = api.getShortcutKeys();
const menu = useMenu(api, isFullscreen, showPanel, showNav, enableShortcuts);

This comment has been minimized.

Sign in to view

Copy link
@Hypnosphi

Hypnosphi Sep 5, 2020

Member

@ndelangen This actually violates Rules of Hooks and breaks with React 17

This comment has been minimized.

Sign in to view

Copy link
@ndelangen

ndelangen Sep 10, 2020

Author Member

ow that's pretty bad, we should track that as a 6.1 critical bug!

return {
title: name,
url,
stories,
refs,
storyId,
viewMode,
menu: createMenu(api, shortcutKeys, isFullscreen, showPanel, showNav, enableShortcuts),
menu,
menuHighlighted: api.versionUpdateAvailable(),
};
};
Expand Down

0 comments on commit fa3a0f1

Please sign in to comment.