diff --git a/addons/knobs/src/components/__tests__/Panel.js b/addons/knobs/src/components/__tests__/Panel.js index 6fc9321af286..60bd11af4649 100644 --- a/addons/knobs/src/components/__tests__/Panel.js +++ b/addons/knobs/src/components/__tests__/Panel.js @@ -3,7 +3,7 @@ import { shallow, mount } from 'enzyme'; import { STORY_CHANGED } from '@storybook/core-events'; import { TabsState } from '@storybook/components'; -import { ThemeProvider, themes } from '@storybook/theming'; +import { ThemeProvider, themes, convert } from '@storybook/theming'; import Panel from '../Panel'; import { CHANGE, SET } from '../../shared'; import PropForm from '../PropForm'; @@ -191,7 +191,7 @@ describe('Panel', () => { // We have to do a full mount. const root = mount( - + ); @@ -225,7 +225,7 @@ describe('Panel', () => { it('should have one tab per groupId and an empty ALL tab when all are defined', () => { const root = mount( - + ); @@ -265,7 +265,7 @@ describe('Panel', () => { it('the ALL tab should have its own additional content when there are knobs both with and without a groupId', () => { const root = mount( - + ); diff --git a/examples/official-storybook/config.js b/examples/official-storybook/config.js index ca3ad277bc7d..a3e716b72f2e 100644 --- a/examples/official-storybook/config.js +++ b/examples/official-storybook/config.js @@ -1,6 +1,6 @@ import React from 'react'; import { storiesOf, configure, addDecorator, addParameters } from '@storybook/react'; -import { Global, ThemeProvider, themes, createReset } from '@storybook/theming'; +import { Global, ThemeProvider, themes, createReset, create, convert } from '@storybook/theming'; import { withCssResources } from '@storybook/addon-cssresources'; import { withA11y } from '@storybook/addon-a11y'; @@ -32,7 +32,7 @@ addDecorator(withA11y); addDecorator(withNotes); addDecorator(storyFn => ( - + {storyFn()} @@ -49,9 +49,10 @@ addParameters({ options: { hierarchySeparator: /\/|\./, hierarchyRootSeparator: '|', + theme: create({ colorPrimary: 'hotpink', colorSecondary: 'orangered' }), }, backgrounds: [ - { name: 'storybook app', value: themes.normal.background.app, default: true }, + { name: 'storybook app', value: themes.light.appBg, default: true }, { name: 'light', value: '#eeeeee' }, { name: 'dark', value: '#222222' }, ], diff --git a/lib/theming/src/base.ts b/lib/theming/src/base.ts index 061566cf47fd..f89258ea2911 100644 --- a/lib/theming/src/base.ts +++ b/lib/theming/src/base.ts @@ -94,6 +94,42 @@ export const typography = { }, }; +export interface ThemeVars { + base: 'light' | 'dark'; + + colorPrimary?: string; + colorSecondary?: string; + + // UI + appBg?: string; + appContentBg?: string; + appBorderColor?: string; + appBorderRadius?: number; + + // Typography + fontBase?: string; + fontCode?: string; + + // Text colors + textColor?: string; + textInverseColor?: string; + + // Toolbar default and active colors + barTextColor?: string; + barSelectedColor?: string; + barBg?: string; + + // Form colors + inputBg?: string; + inputBorder?: string; + inputTextColor?: string; + inputBorderRadius?: number; + + brandTitle?: string; + brandUrl?: string; + brandImage?: string; +} + export type Color = typeof color; export type Background = typeof background; export type Typography = typeof typography; diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index dce9514df68c..a4d44f016a3d 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -1,52 +1,21 @@ // This generates theme variables in the correct shape for the UI - -import { Theme, Brand, color, Color, background, typography } from './base'; -import { easing, animation } from './animation'; -import { create as createSyntax } from './modules/syntax'; import { chromeLight, chromeDark } from 'react-inspector'; import { opacify } from 'polished'; -interface Rest { - [key: string]: any; -} - -interface ThemeVar { - base?: 'light' | 'dark'; +import lightThemeVars from './themes/light'; +import darkThemeVars from './themes/dark'; - colorPrimary?: string; - colorSecondary?: string; - - // UI - appBg?: string; - appContentBg?: string; - appBorderColor?: string; - appBorderRadius?: number; - - // Typography - fontBase?: string; - fontCode?: string; - - // Text colors - textColor?: string; - textInverseColor?: string; - - // Toolbar default and active colors - barTextColor?: string; - barSelectedColor?: string; - barBg?: string; +import { Theme, color, Color, background, typography, ThemeVars } from './base'; +import { easing, animation } from './animation'; +import { create as createSyntax } from './modules/syntax'; - // Form colors - inputBg?: string; - inputBorder?: string; - inputTextColor?: string; - inputBorderRadius?: number; +const themes: { light: ThemeVars; dark: ThemeVars } = { light: lightThemeVars, dark: darkThemeVars }; - brandTitle?: string; - brandUrl?: string; - brandImage?: string; +interface Rest { + [key: string]: any; } -const createColors = (vars: ThemeVar): Color => ({ +const createColors = (vars: ThemeVars): Color => ({ // Changeable colors primary: vars.colorPrimary, secondary: vars.colorSecondary, @@ -110,76 +79,117 @@ const darkSyntaxColors = { blue2: '#00009f', }; -export const create = (vars: ThemeVar, rest?: Rest): Theme => ({ - base: vars.base, - color: createColors(vars), - background: { - app: vars.appBg || background.app, - content: vars.appContentBg || color.lightest, - hoverable: vars.base === 'light' ? 'rgba(0,0,0,.05)' : 'rgba(250,250,252,.1)' || background.hoverable, - - positive: background.positive, - negative: background.negative, - warning: background.warning, - }, - typography: { - fonts: { - base: vars.fontBase || typography.fonts.base, - mono: vars.fontCode || typography.fonts.mono, +export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): ThemeVars => { + const inherit: ThemeVars = { + ...themes.light, + ...(themes[vars.base] || {}), + ...vars, + ...{ base: themes[vars.base] ? vars.base : 'light' }, + }; + return { + ...rest, + ...inherit, + ...{ barSelectedColor: vars.barSelectedColor || inherit.colorSecondary }, + }; +}; + +export const convert = (inherit: ThemeVars = lightThemeVars): Theme => { + const { + base, + colorPrimary, + colorSecondary, + appBg, + appContentBg, + appBorderColor, + appBorderRadius, + fontBase, + fontCode, + textColor, + textInverseColor, + barTextColor, + barSelectedColor, + barBg, + inputBg, + inputBorder, + inputTextColor, + inputBorderRadius, + brandTitle, + brandUrl, + brandImage, + ...rest + } = inherit; + + return { + ...(rest || {}), + + base, + color: createColors(inherit), + background: { + app: appBg, + content: appContentBg, + hoverable: base === 'light' ? 'rgba(0,0,0,.05)' : 'rgba(250,250,252,.1)' || background.hoverable, + + positive: background.positive, + negative: background.negative, + warning: background.warning, }, - weight: typography.weight, - size: typography.size, - }, - animation, - easing, - - input: { - border: vars.inputBorder || color.border, - background: vars.inputBg || color.lightest, - color: vars.inputTextColor || color.defaultText, - borderRadius: vars.inputBorderRadius || vars.appBorderRadius || 4, - }, - - // UI - layoutMargin: 10, - appBorderColor: vars.appBorderColor || color.border, - appBorderRadius: vars.appBorderRadius || 4, - - // Toolbar default/active colors - barTextColor: vars.barTextColor || color.mediumdark, - barSelectedColor: vars.barSelectedColor || color.secondary, - barBg: vars.barBg || color.lightest, - - // Brand logo/text - brand: { - title: vars.brandTitle, - url: vars.brandUrl, - image: vars.brandImage, - }, - - code: createSyntax({ - colors: vars.base === 'light' ? lightSyntaxColors : darkSyntaxColors, - mono: vars.fontCode || typography.fonts.mono, - }), - - // Addon actions theme - // API example https://github.com/xyc/react-inspector/blob/master/src/styles/themes/chromeLight.js - addonActionsTheme: { - ...(vars.base === 'light' ? chromeLight : chromeDark), - - BASE_FONT_FAMILY: vars.fontCode || typography.fonts.mono, - BASE_FONT_SIZE: typography.size.s2 - 1, - BASE_LINE_HEIGHT: '18px', - BASE_BACKGROUND_COLOR: 'transparent', - BASE_COLOR: vars.textColor || color.darkest, - ARROW_COLOR: opacify(0.2, vars.appBorderColor || color.border), - ARROW_MARGIN_RIGHT: 4, - ARROW_FONT_SIZE: 8, - TREENODE_FONT_FAMILY: vars.fontCode || typography.fonts.mono, - TREENODE_FONT_SIZE: typography.size.s2 - 1, - TREENODE_LINE_HEIGHT: '18px', - TREENODE_PADDING_LEFT: 12, - }, - - ...(rest || {}), -}); + typography: { + fonts: { + base: fontBase, + mono: fontCode, + }, + weight: typography.weight, + size: typography.size, + }, + animation, + easing, + + input: { + border: inputBorder, + background: inputBg, + color: inputTextColor, + borderRadius: inputBorderRadius, + }, + + // UI + layoutMargin: 10, + appBorderColor, + appBorderRadius, + + // Toolbar default/active colors + barTextColor, + barSelectedColor: barSelectedColor || colorSecondary, + barBg, + + // Brand logo/text + brand: { + title: brandTitle, + url: brandUrl, + image: brandImage, + }, + + code: createSyntax({ + colors: base === 'light' ? lightSyntaxColors : darkSyntaxColors, + mono: fontCode, + }), + + // Addon actions theme + // API example https://github.com/xyc/react-inspector/blob/master/src/styles/themes/chromeLight.js + addonActionsTheme: { + ...(base === 'light' ? chromeLight : chromeDark), + + BASE_FONT_FAMILY: fontCode, + BASE_FONT_SIZE: typography.size.s2 - 1, + BASE_LINE_HEIGHT: '18px', + BASE_BACKGROUND_COLOR: 'transparent', + BASE_COLOR: textColor, + ARROW_COLOR: opacify(0.2, appBorderColor), + ARROW_MARGIN_RIGHT: 4, + ARROW_FONT_SIZE: 8, + TREENODE_FONT_FAMILY: fontCode, + TREENODE_FONT_SIZE: typography.size.s2 - 1, + TREENODE_LINE_HEIGHT: '18px', + TREENODE_PADDING_LEFT: 12, + }, + }; +}; diff --git a/lib/theming/src/ensure.ts b/lib/theming/src/ensure.ts index 70fe47718127..7cfb9ea1f999 100644 --- a/lib/theming/src/ensure.ts +++ b/lib/theming/src/ensure.ts @@ -3,42 +3,15 @@ import { logger } from '@storybook/client-logger'; import { deletedDiff } from 'deep-object-diff'; import { stripIndent } from 'common-tags'; -import mergeWith from 'lodash.mergewith'; -import isEqual from 'lodash.isequal'; - import light from './themes/light'; -import { Theme } from './base'; - -const base = { - ...light, - animation: {}, - brand: {}, -}; - -// merge with concatenating arrays, but no duplicates -const merge = (a: any, b: any) => - mergeWith({}, a, b, (objValue: any, srcValue: any) => { - if (Array.isArray(srcValue) && Array.isArray(objValue)) { - srcValue.forEach(s => { - const existing = objValue.find(o => o === s || isEqual(o, s)); - if (!existing) { - objValue.push(s); - } - }); - - return objValue; - } - if (Array.isArray(objValue)) { - return objValue; - } - return undefined; - }); +import { Theme, ThemeVars } from './base'; +import { convert } from './create'; -export const ensure = (input: any): Theme => { +export const ensure = (input: ThemeVars): Theme => { if (!input) { - return light; + return convert(light); } else { - const missing = deletedDiff(base, input); + const missing = deletedDiff(light, input); if (Object.keys(missing).length) { logger.warn( stripIndent` @@ -50,6 +23,6 @@ export const ensure = (input: any): Theme => { ); } - return merge(light, input); + return convert(input); } }; diff --git a/lib/theming/src/tests/create.test.js b/lib/theming/src/tests/create.test.js new file mode 100644 index 000000000000..94494ceea090 --- /dev/null +++ b/lib/theming/src/tests/create.test.js @@ -0,0 +1,145 @@ +import { create, convert } from '../create'; +import darkThemeVars from '../themes/dark'; +import lightThemeVars from '../themes/light'; + +describe('create base', () => { + it('should create a theme with minimal viable theme', () => { + const result = create({ base: 'light' }); + + expect(result).toBeDefined(); + }); + it('should pick `light` when `base` is missing', () => { + const result = create({ base: undefined }); + + expect(result.base).toBe('light'); + }); + it('should pick `light` when nothing is given', () => { + const result = create(); + + expect(result.base).toBe('light'); + }); + it('should pick `dark` when base is dark', () => { + const result = create({ base: 'dark' }); + + expect(result.base).toBe('dark'); + }); + it('should pick `light` when base is a unknown value', () => { + const result = create({ base: 'foobar' }); + + expect(result.base).toBe('light'); + }); +}); + +describe('create merge', () => { + it('should merge colorPrimary', () => { + const result = create({ base: 'light', colorPrimary: 'orange' }); + + expect(result).toHaveProperty('colorPrimary', 'orange'); + }); + it('should merge colorSecondary', () => { + const result = create({ base: 'light', colorSecondary: 'orange' }); + + expect(result).toHaveProperty('colorSecondary', 'orange'); + }); + it('should merge appBg', () => { + const result = create({ base: 'light', appBg: 'orange' }); + + expect(result).toHaveProperty('appBg', 'orange'); + }); +}); + +describe('create brand', () => { + it('should have default', () => { + const result = create({ base: 'light' }); + + expect(result.brandImage).not.toBeDefined(); + expect(result.brandTitle).not.toBeDefined(); + expect(result.brandUrl).not.toBeDefined(); + }); + it('should accept null', () => { + const result = create({ base: 'light', brandTitle: null, brandUrl: null, brandImage: null }); + + expect(result).toMatchObject({ + brandImage: null, + brandTitle: null, + brandUrl: null, + }); + }); + it('should accept values', () => { + const result = create({ + base: 'light', + brandImage: 'https://placehold.it/350x150', + brandTitle: 'my custom storybook', + brandUrl: 'https://example.com', + }); + + expect(result).toMatchObject({ + brandImage: 'https://placehold.it/350x150', + brandTitle: 'my custom storybook', + brandUrl: 'https://example.com', + }); + }); +}); + +describe('create extend', () => { + it('should allow custom props', () => { + const result = create( + { + base: 'light', + }, + { + myCustomProperty: 42, + } + ); + + expect(result.myCustomProperty).toEqual(42); + }); + it('should not allow overriding known properties with custom props', () => { + const result = create( + { + base: 'light', + }, + { + base: 42, + } + ); + + expect(result.base).toEqual('light'); + }); +}); + +describe('convert', () => { + it('should return the default theme when no params', () => { + const result = convert(); + + expect(result.base).toEqual('light'); + }); + it('should return a valid dark theme', () => { + const result = convert(darkThemeVars); + + expect(result.base).toEqual('dark'); + expect(result).toMatchObject({ + color: expect.objectContaining({ + primary: '#FF4785', + secondary: '#1EA7FD', + }), + background: expect.objectContaining({ + app: '#2f2f2f', + }), + }); + }); + it('should return a valid light theme', () => { + const result = convert(lightThemeVars); + + expect(result.base).toEqual('light'); + expect(result).toMatchObject({ + color: expect.objectContaining({ + primary: '#FF4785', + secondary: '#1EA7FD', + }), + background: expect.objectContaining({ + app: '#F6F9FC', + }), + }); + }); +}); diff --git a/lib/theming/src/themes/dark.ts b/lib/theming/src/themes/dark.ts index 5c64255032da..39ed282514f8 100644 --- a/lib/theming/src/themes/dark.ts +++ b/lib/theming/src/themes/dark.ts @@ -1,8 +1,6 @@ -import { create } from '../create'; -import { color, typography } from '../base'; +import { color, typography, ThemeVars } from '../base'; -export default create({ - // Is this a light theme or a dark theme? +const theme: ThemeVars = { base: 'dark', // Storybook-specific color palette @@ -33,4 +31,6 @@ export default create({ inputBorder: 'rgba(0,0,0,.3)', inputTextColor: color.lightest, inputBorderRadius: 4, -}); +}; + +export default theme; diff --git a/lib/theming/src/themes/light.ts b/lib/theming/src/themes/light.ts index f635af5ca6ba..fec3cd175c30 100644 --- a/lib/theming/src/themes/light.ts +++ b/lib/theming/src/themes/light.ts @@ -1,8 +1,6 @@ -import { create } from '../create'; -import { color, typography, background } from '../base'; +import { color, typography, background, ThemeVars } from '../base'; -export default create({ - // Is this a light theme or a dark theme? +const theme: ThemeVars = { base: 'light', // Storybook-specific color palette @@ -33,4 +31,6 @@ export default create({ inputBorder: color.border, inputTextColor: color.darkest, inputBorderRadius: 4, -}); +}; + +export default theme; diff --git a/lib/ui/package.json b/lib/ui/package.json index 1bb5dd7c0f4e..c3066dc62abe 100644 --- a/lib/ui/package.json +++ b/lib/ui/package.json @@ -28,6 +28,7 @@ "@storybook/core-events": "5.0.0-beta.3", "@storybook/router": "5.0.0-beta.3", "@storybook/theming": "5.0.0-beta.3", + "fast-deep-equal": "^2.0.1", "fuzzy-search": "^3.0.1", "global": "^4.3.2", "lodash.debounce": "^4.0.8", diff --git a/lib/ui/src/components/sidebar/SidebarHeading.stories.js b/lib/ui/src/components/sidebar/SidebarHeading.stories.js index 17f0960d90b9..3bd9b36297d6 100644 --- a/lib/ui/src/components/sidebar/SidebarHeading.stories.js +++ b/lib/ui/src/components/sidebar/SidebarHeading.stories.js @@ -1,10 +1,11 @@ import React from 'react'; -import { themes, ThemeProvider } from '@storybook/theming'; +import { themes, ThemeProvider, convert } from '@storybook/theming'; import { action } from '@storybook/addon-actions'; import SidebarHeading from './SidebarHeading'; -const { light: theme } = themes; +const { light } = themes; +const theme = convert(light); export default { component: SidebarHeading, diff --git a/lib/ui/src/core/layout.js b/lib/ui/src/core/layout.js index d6bf8ef8ce9f..ae308c73c0c6 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -1,14 +1,16 @@ import pick from 'lodash.pick'; import deprecate from 'util-deprecate'; +import deepEqual from 'fast-deep-equal'; -import { create, themes } from '@storybook/theming'; +import { themes } from '@storybook/theming'; import merge from '../libs/merge'; const deprecatedThemeOptions = { name: 'brandTitle', url: 'brandUrl', }; + const deprecatedLayoutOptions = { goFullScreen: 'isFullscreen', showStoriesPanel: 'showNav', @@ -21,14 +23,12 @@ const deprecationMessage = (optionsMap, prefix) => prefix ? `${prefix}'s` : '' } { ${Object.values(optionsMap).join(', ')} } instead.`; -const applyDeprecatedThemeOptions = deprecate(({ name, url, theme }) => { - const vars = { +const applyDeprecatedThemeOptions = deprecate(({ name, url }) => { + return { brandTitle: name, brandUrl: url, brandImage: null, }; - - return { theme: create(vars, theme) }; }, deprecationMessage(deprecatedThemeOptions)); const applyDeprecatedLayoutOptions = deprecate(options => { @@ -59,6 +59,23 @@ const checkDeprecatedLayoutOptions = options => { return {}; }; +const initial = { + ui: { + enableShortcuts: true, + sortStoriesByKind: false, + sidebarAnimations: true, + }, + layout: { + isToolshown: true, + isFullscreen: false, + showPanel: true, + showNav: true, + panelPosition: 'bottom', + }, + theme: themes.light, +}; + +let hasSetOptions = false; export default function({ store }) { const api = { toggleFullscreen(toggled) { @@ -132,7 +149,13 @@ export default function({ store }) { }, setOptions: options => { - const { layout, ui, selectedPanel } = store.getState(); + // The very first time the user sets their options, we don't consider what is in the store. + // At this point in time, what is in the store is what we *persisted*. We did that in order + // to avoid a FOUC (e.g. initial rendering the wrong theme while we waited for the stories to load) + // However, we don't want to have a memory about these things, otherwise we see bugs like the + // user setting a name for their storybook, persisting it, then never being able to unset it + // without clearing localstorage. See https://github.com/storybooks/storybook/issues/5857 + const { layout, ui, selectedPanel, theme } = hasSetOptions ? store.getState() : initial; if (options) { const updatedLayout = { @@ -144,40 +167,40 @@ export default function({ store }) { const updatedUi = { ...ui, ...pick(options, Object.keys(ui)), + }; + + const updatedTheme = { + ...theme, + ...options.theme, ...checkDeprecatedThemeOptions(options), }; - store.setState( - { - layout: updatedLayout, - ui: updatedUi, - selectedPanel: options.panel || options.selectedPanel || selectedPanel, - }, - { persistence: 'permanent' } - ); + const modification = {}; + + if (!deepEqual(ui, updatedUi)) { + modification.ui = updatedUi; + } + if (!deepEqual(layout, updatedLayout)) { + modification.layout = updatedLayout; + } + if (!deepEqual(theme, updatedTheme)) { + modification.theme = updatedTheme; + } + if (!deepEqual(selectedPanel, options.selectedPanel)) { + modification.selectedPanel = options.selectedPanel; + } + + if (Object.keys(modification).length) { + store.setState(modification, { persistence: 'permanent' }); + } + + hasSetOptions = true; } }, }; - const fromState = pick(store.getState(), 'layout', 'ui', 'selectedPanel'); - - const initial = { - ui: { - enableShortcuts: true, - sortStoriesByKind: false, - sidebarAnimations: true, - theme: themes.normal, - }, - layout: { - isToolshown: true, - isFullscreen: false, - showPanel: true, - showNav: true, - panelPosition: 'bottom', - }, - }; - - const state = merge(fromState, initial); + const persisted = pick(store.getState(), 'layout', 'ui', 'selectedPanel', 'theme'); + const state = merge(initial, persisted); return { api, state }; } diff --git a/lib/ui/src/index.js b/lib/ui/src/index.js index 990a8cd13128..2e62922d4aaf 100644 --- a/lib/ui/src/index.js +++ b/lib/ui/src/index.js @@ -24,7 +24,7 @@ const Root = ({ provider }) => ( {locationData => ( {({ state }) => ( - + )} diff --git a/lib/ui/src/settings/shortcuts.test.js b/lib/ui/src/settings/shortcuts.test.js index c791f10c055f..afad8d70ca0d 100644 --- a/lib/ui/src/settings/shortcuts.test.js +++ b/lib/ui/src/settings/shortcuts.test.js @@ -2,7 +2,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { render } from 'react-testing-library'; -import { ThemeProvider, themes } from '@storybook/theming'; +import { ThemeProvider, themes, convert } from '@storybook/theming'; import ShortcutsScreen from './shortcuts'; // A limited set of keys we use in this test file @@ -26,7 +26,7 @@ const makeActions = () => ({ describe('ShortcutsScreen', () => { it('renders correctly', () => { const comp = shallow( - + ); @@ -35,7 +35,7 @@ describe('ShortcutsScreen', () => { it('handles a full mount', () => { const comp = render( - + ); diff --git a/scripts/compile-js.js b/scripts/compile-js.js index 1ebba2962c76..a25c9907aab9 100644 --- a/scripts/compile-js.js +++ b/scripts/compile-js.js @@ -34,7 +34,9 @@ function babelify(options = {}) { const { watch = false, silent = true, errorCallback } = options; if (!fs.existsSync('src')) { - if (!silent) console.log('No src dir'); + if (!silent) { + console.log('No src dir'); + } return; }