From 24f9ec6b9fbcd1d580ceabc392e30196a9f1f6d5 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 11:23:00 +0100 Subject: [PATCH 01/16] ADD ability for users to specify baseTheme, and the theme createFn will merge in the appropriate themeVars --- lib/theming/src/create.ts | 156 +++++++++++++++++--------------- lib/theming/src/themes/dark.ts | 12 ++- lib/theming/src/themes/light.ts | 12 ++- 3 files changed, 101 insertions(+), 79 deletions(-) diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index dce9514df68c..71f3aae9ddb2 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -6,6 +6,11 @@ import { create as createSyntax } from './modules/syntax'; import { chromeLight, chromeDark } from 'react-inspector'; import { opacify } from 'polished'; +import { themeVars as lightThemeVars } from './themes/light'; +import { themeVars as darkThemeVars } from './themes/dark'; + +const base: { light: ThemeVar; dark: ThemeVar } = { light: lightThemeVars, dark: darkThemeVars }; + interface Rest { [key: string]: any; } @@ -110,76 +115,85 @@ 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, +function pick(b: B, key: K): B[K] { + return b[key]; +} + +export const create = (vars: ThemeVar, rest?: Rest): Theme => { + const inherit = { ...vars, ...(base[vars.base] || base.light), ...base.light }; + const p = pick.bind(null, inherit); + + return { + base: vars.base, + color: createColors(vars), + background: { + app: p('appBg'), + content: p('appContentBg'), + 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: p('fontBase'), + mono: p('fontCode'), + }, + weight: typography.weight, + size: typography.size, + }, + animation, + easing, + + input: { + border: p('inputBorder'), + background: p('inputBg'), + color: p('inputTextColor'), + borderRadius: p('inputBorderRadius'), }, - 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 || {}), -}); + // UI + layoutMargin: 10, + appBorderColor: p('appBorderColor'), + appBorderRadius: p('appBorderRadius'), + + // Toolbar default/active colors + barTextColor: p('barTextColor'), + barSelectedColor: p('barSelectedColor'), + barBg: p('barBg'), + + // Brand logo/text + brand: { + title: p('brandTitle'), + url: p('brandUrl'), + image: p('brandImage'), + }, + + code: createSyntax({ + colors: vars.base === 'light' ? lightSyntaxColors : darkSyntaxColors, + mono: p('fontCode'), + }), + + // 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: p('fontCode'), + BASE_FONT_SIZE: typography.size.s2 - 1, + BASE_LINE_HEIGHT: '18px', + BASE_BACKGROUND_COLOR: 'transparent', + BASE_COLOR: p('textColor'), + ARROW_COLOR: opacify(0.2, p('appBorderColor')), + ARROW_MARGIN_RIGHT: 4, + ARROW_FONT_SIZE: 8, + TREENODE_FONT_FAMILY: p('fontCode'), + TREENODE_FONT_SIZE: typography.size.s2 - 1, + TREENODE_LINE_HEIGHT: '18px', + TREENODE_PADDING_LEFT: 12, + }, + + ...(rest || {}), + }; +}; diff --git a/lib/theming/src/themes/dark.ts b/lib/theming/src/themes/dark.ts index 5c64255032da..f686300a6514 100644 --- a/lib/theming/src/themes/dark.ts +++ b/lib/theming/src/themes/dark.ts @@ -1,10 +1,7 @@ import { create } from '../create'; import { color, typography } from '../base'; -export default create({ - // Is this a light theme or a dark theme? - base: 'dark', - +export const themeVars = { // Storybook-specific color palette colorPrimary: '#FF4785', // coral colorSecondary: '#1EA7FD', // ocean @@ -33,4 +30,11 @@ export default create({ inputBorder: 'rgba(0,0,0,.3)', inputTextColor: color.lightest, inputBorderRadius: 4, +}; + +export default create({ + // Is this a light theme or a dark theme? + base: 'dark', + + ...themeVars, }); diff --git a/lib/theming/src/themes/light.ts b/lib/theming/src/themes/light.ts index f635af5ca6ba..b7d190471ae4 100644 --- a/lib/theming/src/themes/light.ts +++ b/lib/theming/src/themes/light.ts @@ -1,10 +1,7 @@ import { create } from '../create'; import { color, typography, background } from '../base'; -export default create({ - // Is this a light theme or a dark theme? - base: 'light', - +export const themeVars = { // Storybook-specific color palette colorPrimary: '#FF4785', // coral colorSecondary: '#1EA7FD', // ocean @@ -33,4 +30,11 @@ export default create({ inputBorder: color.border, inputTextColor: color.darkest, inputBorderRadius: 4, +}; + +export default create({ + // Is this a light theme or a dark theme? + base: 'light', + + ...themeVars, }); From aa03da5f7930f2883dece841f1f3701119ad0efd Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 11:56:27 +0100 Subject: [PATCH 02/16] FIX circular dependencies --- lib/theming/src/create.ts | 16 +++++++------- lib/theming/src/themes/dark-vars.ts | 32 +++++++++++++++++++++++++++ lib/theming/src/themes/dark.ts | 33 +--------------------------- lib/theming/src/themes/light-vars.ts | 33 ++++++++++++++++++++++++++++ lib/theming/src/themes/light.ts | 32 +-------------------------- scripts/compile-js.js | 4 +++- 6 files changed, 78 insertions(+), 72 deletions(-) create mode 100644 lib/theming/src/themes/dark-vars.ts create mode 100644 lib/theming/src/themes/light-vars.ts diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index 71f3aae9ddb2..b12aea720caf 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -1,13 +1,13 @@ // 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'; -import { themeVars as lightThemeVars } from './themes/light'; -import { themeVars as darkThemeVars } from './themes/dark'; +import { themeVars as lightThemeVars } from './themes/light-vars'; +import { themeVars as darkThemeVars } from './themes/dark-vars'; + +import { Theme, color, Color, background, typography } from './base'; +import { easing, animation } from './animation'; +import { create as createSyntax } from './modules/syntax'; const base: { light: ThemeVar; dark: ThemeVar } = { light: lightThemeVars, dark: darkThemeVars }; @@ -120,12 +120,12 @@ function pick(b: B, key: K): B[K] { } export const create = (vars: ThemeVar, rest?: Rest): Theme => { - const inherit = { ...vars, ...(base[vars.base] || base.light), ...base.light }; + const inherit: ThemeVar = { ...vars, ...(base[vars.base] || base.light), ...base.light }; const p = pick.bind(null, inherit); return { base: vars.base, - color: createColors(vars), + color: createColors(inherit), background: { app: p('appBg'), content: p('appContentBg'), diff --git a/lib/theming/src/themes/dark-vars.ts b/lib/theming/src/themes/dark-vars.ts new file mode 100644 index 000000000000..4d275b93e032 --- /dev/null +++ b/lib/theming/src/themes/dark-vars.ts @@ -0,0 +1,32 @@ +import { color, typography } from '../base'; + +export const themeVars = { + // Storybook-specific color palette + colorPrimary: '#FF4785', // coral + colorSecondary: '#1EA7FD', // ocean + + // UI + appBg: '#2f2f2f', + appContentBg: '#333', + appBorderColor: 'rgba(255,255,255,.1)', + appBorderRadius: 4, + + // Fonts + fontBase: typography.fonts.base, + fontCode: typography.fonts.mono, + + // Text colors + textColor: color.lightest, + textInverseColor: color.darkest, + + // Toolbar default and active colors + barTextColor: '#999999', + barSelectedColor: color.secondary, + barBg: color.darkest, + + // Form colors + inputBg: '#3f3f3f', + inputBorder: 'rgba(0,0,0,.3)', + inputTextColor: color.lightest, + inputBorderRadius: 4, +}; diff --git a/lib/theming/src/themes/dark.ts b/lib/theming/src/themes/dark.ts index f686300a6514..0251c4409b76 100644 --- a/lib/theming/src/themes/dark.ts +++ b/lib/theming/src/themes/dark.ts @@ -1,36 +1,5 @@ import { create } from '../create'; -import { color, typography } from '../base'; - -export const themeVars = { - // Storybook-specific color palette - colorPrimary: '#FF4785', // coral - colorSecondary: '#1EA7FD', // ocean - - // UI - appBg: '#2f2f2f', - appContentBg: '#333', - appBorderColor: 'rgba(255,255,255,.1)', - appBorderRadius: 4, - - // Fonts - fontBase: typography.fonts.base, - fontCode: typography.fonts.mono, - - // Text colors - textColor: color.lightest, - textInverseColor: color.darkest, - - // Toolbar default and active colors - barTextColor: '#999999', - barSelectedColor: color.secondary, - barBg: color.darkest, - - // Form colors - inputBg: '#3f3f3f', - inputBorder: 'rgba(0,0,0,.3)', - inputTextColor: color.lightest, - inputBorderRadius: 4, -}; +import { themeVars } from './dark-vars'; export default create({ // Is this a light theme or a dark theme? diff --git a/lib/theming/src/themes/light-vars.ts b/lib/theming/src/themes/light-vars.ts new file mode 100644 index 000000000000..bb7787a71fc1 --- /dev/null +++ b/lib/theming/src/themes/light-vars.ts @@ -0,0 +1,33 @@ +import { color, typography, background } from '../base'; + +export const themeVars = { + // Storybook-specific color palette + colorPrimary: '#FF4785', // coral + colorSecondary: '#1EA7FD', // ocean + + // UI + appBg: background.app, + appContentBg: color.lightest, + appBorderColor: color.border, + appBorderRadius: 4, + + // Fonts + fontBase: typography.fonts.base, + fontCode: typography.fonts.mono, + + // Text colors + textColor: color.darkest, + textInverseColor: color.lightest, + + // Toolbar default and active colors + barTextColor: color.mediumdark, + barSelectedColor: color.secondary, + barBg: color.lightest, + + // Form colors + inputBg: color.lightest, + inputBorder: color.border, + inputTextColor: color.darkest, + inputBorderRadius: 4, +}; + diff --git a/lib/theming/src/themes/light.ts b/lib/theming/src/themes/light.ts index b7d190471ae4..c51ebd945bf1 100644 --- a/lib/theming/src/themes/light.ts +++ b/lib/theming/src/themes/light.ts @@ -1,36 +1,6 @@ import { create } from '../create'; -import { color, typography, background } from '../base'; -export const themeVars = { - // Storybook-specific color palette - colorPrimary: '#FF4785', // coral - colorSecondary: '#1EA7FD', // ocean - - // UI - appBg: background.app, - appContentBg: color.lightest, - appBorderColor: color.border, - appBorderRadius: 4, - - // Fonts - fontBase: typography.fonts.base, - fontCode: typography.fonts.mono, - - // Text colors - textColor: color.darkest, - textInverseColor: color.lightest, - - // Toolbar default and active colors - barTextColor: color.mediumdark, - barSelectedColor: color.secondary, - barBg: color.lightest, - - // Form colors - inputBg: color.lightest, - inputBorder: color.border, - inputTextColor: color.darkest, - inputBorderRadius: 4, -}; +import { themeVars } from './dark-vars'; export default create({ // Is this a light theme or a dark theme? 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; } From 5373dbac9c43d3cb555dcb96c3650ada4a26ea9a Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 15:02:58 +0100 Subject: [PATCH 03/16] RENAME ThemeVar to ThemeVars && REMOVE the pick & p function --- lib/theming/src/create.ts | 61 ++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index b12aea720caf..7561ed51d4de 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -2,20 +2,20 @@ import { chromeLight, chromeDark } from 'react-inspector'; import { opacify } from 'polished'; -import { themeVars as lightThemeVars } from './themes/light-vars'; -import { themeVars as darkThemeVars } from './themes/dark-vars'; +import { themeVars as lightThemeVarss } from './themes/light-vars'; +import { themeVars as darkThemeVarss } from './themes/dark-vars'; import { Theme, color, Color, background, typography } from './base'; import { easing, animation } from './animation'; import { create as createSyntax } from './modules/syntax'; -const base: { light: ThemeVar; dark: ThemeVar } = { light: lightThemeVars, dark: darkThemeVars }; +const base: { light: ThemeVars; dark: ThemeVars } = { light: lightThemeVarss, dark: darkThemeVarss }; interface Rest { [key: string]: any; } -interface ThemeVar { +interface ThemeVars { base?: 'light' | 'dark'; colorPrimary?: string; @@ -51,7 +51,7 @@ interface ThemeVar { brandImage?: string; } -const createColors = (vars: ThemeVar): Color => ({ +const createColors = (vars: ThemeVars): Color => ({ // Changeable colors primary: vars.colorPrimary, secondary: vars.colorSecondary, @@ -115,20 +115,15 @@ const darkSyntaxColors = { blue2: '#00009f', }; -function pick(b: B, key: K): B[K] { - return b[key]; -} - -export const create = (vars: ThemeVar, rest?: Rest): Theme => { - const inherit: ThemeVar = { ...vars, ...(base[vars.base] || base.light), ...base.light }; - const p = pick.bind(null, inherit); +export const create = (vars: ThemeVars, rest?: Rest): Theme => { + const inherit: ThemeVars = { ...vars, ...(base[vars.base] || base.light), ...base.light }; return { base: vars.base, color: createColors(inherit), background: { - app: p('appBg'), - content: p('appContentBg'), + app: inherit.appBg, + content: inherit.appContentBg, hoverable: vars.base === 'light' ? 'rgba(0,0,0,.05)' : 'rgba(250,250,252,.1)' || background.hoverable, positive: background.positive, @@ -137,8 +132,8 @@ export const create = (vars: ThemeVar, rest?: Rest): Theme => { }, typography: { fonts: { - base: p('fontBase'), - mono: p('fontCode'), + base: inherit.fontBase, + mono: inherit.fontCode, }, weight: typography.weight, size: typography.size, @@ -147,32 +142,32 @@ export const create = (vars: ThemeVar, rest?: Rest): Theme => { easing, input: { - border: p('inputBorder'), - background: p('inputBg'), - color: p('inputTextColor'), - borderRadius: p('inputBorderRadius'), + border: inherit.inputBorder, + background: inherit.inputBg, + color: inherit.inputTextColor, + borderRadius: inherit.inputBorderRadius, }, // UI layoutMargin: 10, - appBorderColor: p('appBorderColor'), - appBorderRadius: p('appBorderRadius'), + appBorderColor: inherit.appBorderColor, + appBorderRadius: inherit.appBorderRadius, // Toolbar default/active colors - barTextColor: p('barTextColor'), - barSelectedColor: p('barSelectedColor'), - barBg: p('barBg'), + barTextColor: inherit.barTextColor, + barSelectedColor: inherit.barSelectedColor, + barBg: inherit.barBg, // Brand logo/text brand: { - title: p('brandTitle'), - url: p('brandUrl'), - image: p('brandImage'), + title: inherit.brandTitle, + url: inherit.brandUrl, + image: inherit.brandImage, }, code: createSyntax({ colors: vars.base === 'light' ? lightSyntaxColors : darkSyntaxColors, - mono: p('fontCode'), + mono: inherit.fontCode, }), // Addon actions theme @@ -180,15 +175,15 @@ export const create = (vars: ThemeVar, rest?: Rest): Theme => { addonActionsTheme: { ...(vars.base === 'light' ? chromeLight : chromeDark), - BASE_FONT_FAMILY: p('fontCode'), + BASE_FONT_FAMILY: inherit.fontCode, BASE_FONT_SIZE: typography.size.s2 - 1, BASE_LINE_HEIGHT: '18px', BASE_BACKGROUND_COLOR: 'transparent', - BASE_COLOR: p('textColor'), - ARROW_COLOR: opacify(0.2, p('appBorderColor')), + BASE_COLOR: inherit.textColor, + ARROW_COLOR: opacify(0.2, inherit.appBorderColor), ARROW_MARGIN_RIGHT: 4, ARROW_FONT_SIZE: 8, - TREENODE_FONT_FAMILY: p('fontCode'), + TREENODE_FONT_FAMILY: inherit.fontCode, TREENODE_FONT_SIZE: typography.size.s2 - 1, TREENODE_LINE_HEIGHT: '18px', TREENODE_PADDING_LEFT: 12, From c7fbf473ef85bc56ae651a4cf1b99dd001aa6727 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 15:17:37 +0100 Subject: [PATCH 04/16] FIX issue where the `base` property wouldn't be set if non was given --- lib/theming/src/create.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index 7561ed51d4de..d39ed98225f0 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -116,15 +116,22 @@ const darkSyntaxColors = { }; export const create = (vars: ThemeVars, rest?: Rest): Theme => { - const inherit: ThemeVars = { ...vars, ...(base[vars.base] || base.light), ...base.light }; + const inherit: ThemeVars = { + ...vars, + ...(base[vars.base] || base.light), + ...base.light, + ...{ + base: 'light', + }, + }; return { - base: vars.base, + base: inherit.base, color: createColors(inherit), background: { app: inherit.appBg, content: inherit.appContentBg, - hoverable: vars.base === 'light' ? 'rgba(0,0,0,.05)' : 'rgba(250,250,252,.1)' || background.hoverable, + hoverable: inherit.base === 'light' ? 'rgba(0,0,0,.05)' : 'rgba(250,250,252,.1)' || background.hoverable, positive: background.positive, negative: background.negative, @@ -166,14 +173,14 @@ export const create = (vars: ThemeVars, rest?: Rest): Theme => { }, code: createSyntax({ - colors: vars.base === 'light' ? lightSyntaxColors : darkSyntaxColors, + colors: inherit.base === 'light' ? lightSyntaxColors : darkSyntaxColors, mono: inherit.fontCode, }), // Addon actions theme // API example https://github.com/xyc/react-inspector/blob/master/src/styles/themes/chromeLight.js addonActionsTheme: { - ...(vars.base === 'light' ? chromeLight : chromeDark), + ...(inherit.base === 'light' ? chromeLight : chromeDark), BASE_FONT_FAMILY: inherit.fontCode, BASE_FONT_SIZE: typography.size.s2 - 1, From 2aad74cca4a6fb408cf5577ddf006f425d4c1b4d Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 15:18:02 +0100 Subject: [PATCH 05/16] CLEANUP the creation of default themes --- lib/theming/src/themes/dark.ts | 4 ---- lib/theming/src/themes/light.ts | 5 ----- 2 files changed, 9 deletions(-) diff --git a/lib/theming/src/themes/dark.ts b/lib/theming/src/themes/dark.ts index 0251c4409b76..9e8f201a07cd 100644 --- a/lib/theming/src/themes/dark.ts +++ b/lib/theming/src/themes/dark.ts @@ -1,9 +1,5 @@ import { create } from '../create'; -import { themeVars } from './dark-vars'; export default create({ - // Is this a light theme or a dark theme? base: 'dark', - - ...themeVars, }); diff --git a/lib/theming/src/themes/light.ts b/lib/theming/src/themes/light.ts index c51ebd945bf1..d74ce937cab1 100644 --- a/lib/theming/src/themes/light.ts +++ b/lib/theming/src/themes/light.ts @@ -1,10 +1,5 @@ import { create } from '../create'; -import { themeVars } from './dark-vars'; - export default create({ - // Is this a light theme or a dark theme? base: 'light', - - ...themeVars, }); From 04b5071cfade67434b1fef26e670ee545e61dd18 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 15:53:31 +0100 Subject: [PATCH 06/16] MOVE ThemeVars to base && FIX custom properties overriding known properties && FIX merge order of vars & defaults --- lib/theming/src/base.ts | 36 +++++++++++++++++ lib/theming/src/create.ts | 58 +++++----------------------- lib/theming/src/themes/dark-vars.ts | 6 ++- lib/theming/src/themes/light-vars.ts | 7 ++-- 4 files changed, 54 insertions(+), 53 deletions(-) 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 d39ed98225f0..b9bc4aef28e0 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -2,55 +2,19 @@ import { chromeLight, chromeDark } from 'react-inspector'; import { opacify } from 'polished'; -import { themeVars as lightThemeVarss } from './themes/light-vars'; -import { themeVars as darkThemeVarss } from './themes/dark-vars'; +import { themeVars as lightThemeVars } from './themes/light-vars'; +import { themeVars as darkThemeVars } from './themes/dark-vars'; -import { Theme, color, Color, background, typography } from './base'; +import { Theme, color, Color, background, typography, ThemeVars } from './base'; import { easing, animation } from './animation'; import { create as createSyntax } from './modules/syntax'; -const base: { light: ThemeVars; dark: ThemeVars } = { light: lightThemeVarss, dark: darkThemeVarss }; +const base: { light: ThemeVars; dark: ThemeVars } = { light: lightThemeVars, dark: darkThemeVars }; interface Rest { [key: string]: any; } -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; -} - const createColors = (vars: ThemeVars): Color => ({ // Changeable colors primary: vars.colorPrimary, @@ -115,17 +79,17 @@ const darkSyntaxColors = { blue2: '#00009f', }; -export const create = (vars: ThemeVars, rest?: Rest): Theme => { +export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): Theme => { const inherit: ThemeVars = { - ...vars, - ...(base[vars.base] || base.light), ...base.light, - ...{ - base: 'light', - }, + ...(base[vars.base] || base.light), + ...vars, + ...{ base: base[vars.base] ? vars.base : 'light' }, }; return { + ...(rest || {}), + base: inherit.base, color: createColors(inherit), background: { @@ -195,7 +159,5 @@ export const create = (vars: ThemeVars, rest?: Rest): Theme => { TREENODE_LINE_HEIGHT: '18px', TREENODE_PADDING_LEFT: 12, }, - - ...(rest || {}), }; }; diff --git a/lib/theming/src/themes/dark-vars.ts b/lib/theming/src/themes/dark-vars.ts index 4d275b93e032..d6126df9318d 100644 --- a/lib/theming/src/themes/dark-vars.ts +++ b/lib/theming/src/themes/dark-vars.ts @@ -1,6 +1,8 @@ -import { color, typography } from '../base'; +import { color, typography, ThemeVars } from '../base'; + +export const themeVars: ThemeVars = { + base: 'dark', -export const themeVars = { // Storybook-specific color palette colorPrimary: '#FF4785', // coral colorSecondary: '#1EA7FD', // ocean diff --git a/lib/theming/src/themes/light-vars.ts b/lib/theming/src/themes/light-vars.ts index bb7787a71fc1..5c89195a4159 100644 --- a/lib/theming/src/themes/light-vars.ts +++ b/lib/theming/src/themes/light-vars.ts @@ -1,6 +1,8 @@ -import { color, typography, background } from '../base'; +import { color, typography, background, ThemeVars } from '../base'; + +export const themeVars: ThemeVars = { + base: 'light', -export const themeVars = { // Storybook-specific color palette colorPrimary: '#FF4785', // coral colorSecondary: '#1EA7FD', // ocean @@ -30,4 +32,3 @@ export const themeVars = { inputTextColor: color.darkest, inputBorderRadius: 4, }; - From caa78fb3e7d279bb507c8eb2c820a649eec46ccf Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 15:53:56 +0100 Subject: [PATCH 07/16] ADD tests --- lib/theming/src/tests/create.test.js | 109 +++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 lib/theming/src/tests/create.test.js diff --git a/lib/theming/src/tests/create.test.js b/lib/theming/src/tests/create.test.js new file mode 100644 index 000000000000..83986ab9cd77 --- /dev/null +++ b/lib/theming/src/tests/create.test.js @@ -0,0 +1,109 @@ +import { create } from '../create'; + +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.color).toHaveProperty('primary', 'orange'); + }); + it('should merge colorSecondary', () => { + const result = create({ base: 'light', colorSecondary: 'orange' }); + + expect(result.color).toHaveProperty('secondary', 'orange'); + }); + it('should merge appBg', () => { + const result = create({ base: 'light', appBg: 'orange' }); + + expect(result.background).toHaveProperty('app', 'orange'); + }); +}); + +describe('create brand', () => { + it('should have default', () => { + const result = create({ base: 'light' }); + + expect(result.brand).toEqual({ + image: undefined, + title: undefined, + url: undefined, + }); + }); + it('should accept null', () => { + const result = create({ base: 'light', brandTitle: null, brandUrl: null, brandImage: null }); + + expect(result.brand).toEqual({ + image: null, + title: null, + url: 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.brand).toEqual({ + image: 'https://placehold.it/350x150', + title: 'my custom storybook', + url: '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'); + }); +}); From 1c270924fcbb077943956d0eca2e48f530a454ea Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 17:35:17 +0100 Subject: [PATCH 08/16] FIX a bug with animations not being stringify-able --- lib/theming/src/create.ts | 2 +- lib/theming/src/ensure.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index b9bc4aef28e0..4e2fe16e9167 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -126,7 +126,7 @@ export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): Theme // Toolbar default/active colors barTextColor: inherit.barTextColor, - barSelectedColor: inherit.barSelectedColor, + barSelectedColor: vars.barSelectedColor || inherit.colorSecondary, barBg: inherit.barBg, // Brand logo/text diff --git a/lib/theming/src/ensure.ts b/lib/theming/src/ensure.ts index 70fe47718127..f053ca1742c2 100644 --- a/lib/theming/src/ensure.ts +++ b/lib/theming/src/ensure.ts @@ -38,6 +38,7 @@ export const ensure = (input: any): Theme => { if (!input) { return light; } else { + // debugger; const missing = deletedDiff(base, input); if (Object.keys(missing).length) { logger.warn( @@ -50,6 +51,6 @@ export const ensure = (input: any): Theme => { ); } - return merge(light, input); + return merge(light, { ...input, animation: light.animation }); } }; From dd3c382e1d472567210e33efbb15e75b9ed2e95a Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 21:05:01 +0100 Subject: [PATCH 09/16] REFACTOR change the theme that users import & set to be ThemeVars (was Theme) This way the object easy much easier and more performant to serialise & less trickery is needed to make it work well --- examples/official-storybook/config.js | 7 +- lib/theming/src/create.ts | 92 ++++++++++++------- lib/theming/src/ensure.ts | 40 ++------ lib/theming/src/tests/create.test.js | 70 ++++++++++---- lib/theming/src/themes/dark-vars.ts | 34 ------- lib/theming/src/themes/dark.ts | 37 +++++++- lib/theming/src/themes/light-vars.ts | 34 ------- lib/theming/src/themes/light.ts | 37 +++++++- .../sidebar/SidebarHeading.stories.js | 5 +- lib/ui/src/core/layout.js | 2 +- 10 files changed, 195 insertions(+), 163 deletions(-) delete mode 100644 lib/theming/src/themes/dark-vars.ts delete mode 100644 lib/theming/src/themes/light-vars.ts 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/create.ts b/lib/theming/src/create.ts index 4e2fe16e9167..4b0164d302fb 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -2,14 +2,14 @@ import { chromeLight, chromeDark } from 'react-inspector'; import { opacify } from 'polished'; -import { themeVars as lightThemeVars } from './themes/light-vars'; -import { themeVars as darkThemeVars } from './themes/dark-vars'; +import lightThemeVars from './themes/light'; +import darkThemeVars from './themes/dark'; import { Theme, color, Color, background, typography, ThemeVars } from './base'; import { easing, animation } from './animation'; import { create as createSyntax } from './modules/syntax'; -const base: { light: ThemeVars; dark: ThemeVars } = { light: lightThemeVars, dark: darkThemeVars }; +const themes: { light: ThemeVars; dark: ThemeVars } = { light: lightThemeVars, dark: darkThemeVars }; interface Rest { [key: string]: any; @@ -79,23 +79,51 @@ const darkSyntaxColors = { blue2: '#00009f', }; -export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): Theme => { +export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): ThemeVars => { const inherit: ThemeVars = { - ...base.light, - ...(base[vars.base] || base.light), + ...themes.light, + ...(themes[vars.base] || themes.light), ...vars, - ...{ base: base[vars.base] ? vars.base : 'light' }, + ...{ base: themes[vars.base] ? vars.base : 'light' }, }; + return { ...rest, ...inherit }; +}; + +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: inherit.base, + base, color: createColors(inherit), background: { - app: inherit.appBg, - content: inherit.appContentBg, - hoverable: inherit.base === 'light' ? 'rgba(0,0,0,.05)' : 'rgba(250,250,252,.1)' || background.hoverable, + 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, @@ -103,8 +131,8 @@ export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): Theme }, typography: { fonts: { - base: inherit.fontBase, - mono: inherit.fontCode, + base: fontBase, + mono: fontCode, }, weight: typography.weight, size: typography.size, @@ -113,48 +141,48 @@ export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): Theme easing, input: { - border: inherit.inputBorder, - background: inherit.inputBg, - color: inherit.inputTextColor, - borderRadius: inherit.inputBorderRadius, + border: inputBorder, + background: inputBg, + color: inputTextColor, + borderRadius: inputBorderRadius, }, // UI layoutMargin: 10, - appBorderColor: inherit.appBorderColor, - appBorderRadius: inherit.appBorderRadius, + appBorderColor, + appBorderRadius, // Toolbar default/active colors - barTextColor: inherit.barTextColor, - barSelectedColor: vars.barSelectedColor || inherit.colorSecondary, - barBg: inherit.barBg, + barTextColor, + barSelectedColor: barSelectedColor || colorSecondary, + barBg, // Brand logo/text brand: { - title: inherit.brandTitle, - url: inherit.brandUrl, - image: inherit.brandImage, + title: brandTitle, + url: brandUrl, + image: brandImage, }, code: createSyntax({ - colors: inherit.base === 'light' ? lightSyntaxColors : darkSyntaxColors, - mono: inherit.fontCode, + 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: { - ...(inherit.base === 'light' ? chromeLight : chromeDark), + ...(base === 'light' ? chromeLight : chromeDark), - BASE_FONT_FAMILY: inherit.fontCode, + BASE_FONT_FAMILY: fontCode, BASE_FONT_SIZE: typography.size.s2 - 1, BASE_LINE_HEIGHT: '18px', BASE_BACKGROUND_COLOR: 'transparent', - BASE_COLOR: inherit.textColor, - ARROW_COLOR: opacify(0.2, inherit.appBorderColor), + BASE_COLOR: textColor, + ARROW_COLOR: opacify(0.2, appBorderColor), ARROW_MARGIN_RIGHT: 4, ARROW_FONT_SIZE: 8, - TREENODE_FONT_FAMILY: inherit.fontCode, + 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 f053ca1742c2..7cfb9ea1f999 100644 --- a/lib/theming/src/ensure.ts +++ b/lib/theming/src/ensure.ts @@ -3,43 +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 { - // debugger; - const missing = deletedDiff(base, input); + const missing = deletedDiff(light, input); if (Object.keys(missing).length) { logger.warn( stripIndent` @@ -51,6 +23,6 @@ export const ensure = (input: any): Theme => { ); } - return merge(light, { ...input, animation: light.animation }); + return convert(input); } }; diff --git a/lib/theming/src/tests/create.test.js b/lib/theming/src/tests/create.test.js index 83986ab9cd77..94494ceea090 100644 --- a/lib/theming/src/tests/create.test.js +++ b/lib/theming/src/tests/create.test.js @@ -1,4 +1,6 @@ -import { create } from '../create'; +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', () => { @@ -32,17 +34,17 @@ describe('create merge', () => { it('should merge colorPrimary', () => { const result = create({ base: 'light', colorPrimary: 'orange' }); - expect(result.color).toHaveProperty('primary', 'orange'); + expect(result).toHaveProperty('colorPrimary', 'orange'); }); it('should merge colorSecondary', () => { const result = create({ base: 'light', colorSecondary: 'orange' }); - expect(result.color).toHaveProperty('secondary', 'orange'); + expect(result).toHaveProperty('colorSecondary', 'orange'); }); it('should merge appBg', () => { const result = create({ base: 'light', appBg: 'orange' }); - expect(result.background).toHaveProperty('app', 'orange'); + expect(result).toHaveProperty('appBg', 'orange'); }); }); @@ -50,19 +52,17 @@ describe('create brand', () => { it('should have default', () => { const result = create({ base: 'light' }); - expect(result.brand).toEqual({ - image: undefined, - title: undefined, - url: undefined, - }); + 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.brand).toEqual({ - image: null, - title: null, - url: null, + expect(result).toMatchObject({ + brandImage: null, + brandTitle: null, + brandUrl: null, }); }); it('should accept values', () => { @@ -73,10 +73,10 @@ describe('create brand', () => { brandUrl: 'https://example.com', }); - expect(result.brand).toEqual({ - image: 'https://placehold.it/350x150', - title: 'my custom storybook', - url: 'https://example.com', + expect(result).toMatchObject({ + brandImage: 'https://placehold.it/350x150', + brandTitle: 'my custom storybook', + brandUrl: 'https://example.com', }); }); }); @@ -107,3 +107,39 @@ describe('create extend', () => { 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-vars.ts b/lib/theming/src/themes/dark-vars.ts deleted file mode 100644 index d6126df9318d..000000000000 --- a/lib/theming/src/themes/dark-vars.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { color, typography, ThemeVars } from '../base'; - -export const themeVars: ThemeVars = { - base: 'dark', - - // Storybook-specific color palette - colorPrimary: '#FF4785', // coral - colorSecondary: '#1EA7FD', // ocean - - // UI - appBg: '#2f2f2f', - appContentBg: '#333', - appBorderColor: 'rgba(255,255,255,.1)', - appBorderRadius: 4, - - // Fonts - fontBase: typography.fonts.base, - fontCode: typography.fonts.mono, - - // Text colors - textColor: color.lightest, - textInverseColor: color.darkest, - - // Toolbar default and active colors - barTextColor: '#999999', - barSelectedColor: color.secondary, - barBg: color.darkest, - - // Form colors - inputBg: '#3f3f3f', - inputBorder: 'rgba(0,0,0,.3)', - inputTextColor: color.lightest, - inputBorderRadius: 4, -}; diff --git a/lib/theming/src/themes/dark.ts b/lib/theming/src/themes/dark.ts index 9e8f201a07cd..39ed282514f8 100644 --- a/lib/theming/src/themes/dark.ts +++ b/lib/theming/src/themes/dark.ts @@ -1,5 +1,36 @@ -import { create } from '../create'; +import { color, typography, ThemeVars } from '../base'; -export default create({ +const theme: ThemeVars = { base: 'dark', -}); + + // Storybook-specific color palette + colorPrimary: '#FF4785', // coral + colorSecondary: '#1EA7FD', // ocean + + // UI + appBg: '#2f2f2f', + appContentBg: '#333', + appBorderColor: 'rgba(255,255,255,.1)', + appBorderRadius: 4, + + // Fonts + fontBase: typography.fonts.base, + fontCode: typography.fonts.mono, + + // Text colors + textColor: color.lightest, + textInverseColor: color.darkest, + + // Toolbar default and active colors + barTextColor: '#999999', + barSelectedColor: color.secondary, + barBg: color.darkest, + + // Form colors + inputBg: '#3f3f3f', + inputBorder: 'rgba(0,0,0,.3)', + inputTextColor: color.lightest, + inputBorderRadius: 4, +}; + +export default theme; diff --git a/lib/theming/src/themes/light-vars.ts b/lib/theming/src/themes/light-vars.ts deleted file mode 100644 index 5c89195a4159..000000000000 --- a/lib/theming/src/themes/light-vars.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { color, typography, background, ThemeVars } from '../base'; - -export const themeVars: ThemeVars = { - base: 'light', - - // Storybook-specific color palette - colorPrimary: '#FF4785', // coral - colorSecondary: '#1EA7FD', // ocean - - // UI - appBg: background.app, - appContentBg: color.lightest, - appBorderColor: color.border, - appBorderRadius: 4, - - // Fonts - fontBase: typography.fonts.base, - fontCode: typography.fonts.mono, - - // Text colors - textColor: color.darkest, - textInverseColor: color.lightest, - - // Toolbar default and active colors - barTextColor: color.mediumdark, - barSelectedColor: color.secondary, - barBg: color.lightest, - - // Form colors - inputBg: color.lightest, - inputBorder: color.border, - inputTextColor: color.darkest, - inputBorderRadius: 4, -}; diff --git a/lib/theming/src/themes/light.ts b/lib/theming/src/themes/light.ts index d74ce937cab1..fec3cd175c30 100644 --- a/lib/theming/src/themes/light.ts +++ b/lib/theming/src/themes/light.ts @@ -1,5 +1,36 @@ -import { create } from '../create'; +import { color, typography, background, ThemeVars } from '../base'; -export default create({ +const theme: ThemeVars = { base: 'light', -}); + + // Storybook-specific color palette + colorPrimary: '#FF4785', // coral + colorSecondary: '#1EA7FD', // ocean + + // UI + appBg: background.app, + appContentBg: color.lightest, + appBorderColor: color.border, + appBorderRadius: 4, + + // Fonts + fontBase: typography.fonts.base, + fontCode: typography.fonts.mono, + + // Text colors + textColor: color.darkest, + textInverseColor: color.lightest, + + // Toolbar default and active colors + barTextColor: color.mediumdark, + barSelectedColor: color.secondary, + barBg: color.lightest, + + // Form colors + inputBg: color.lightest, + inputBorder: color.border, + inputTextColor: color.darkest, + inputBorderRadius: 4, +}; + +export default theme; 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..c2f67bebc773 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -166,7 +166,7 @@ export default function({ store }) { enableShortcuts: true, sortStoriesByKind: false, sidebarAnimations: true, - theme: themes.normal, + theme: themes.light, }, layout: { isToolshown: true, From 13b225727e3cbac638c1eb1ab6f7fffd914e575b Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 21:06:20 +0100 Subject: [PATCH 10/16] OPTIMIZE so the theme doesn't update every story change --- lib/ui/package.json | 1 + lib/ui/src/core/layout.js | 33 ++++++++++++++++++++++++--------- lib/ui/src/index.js | 2 +- 3 files changed, 26 insertions(+), 10 deletions(-) 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/core/layout.js b/lib/ui/src/core/layout.js index c2f67bebc773..ad9657669016 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -1,6 +1,7 @@ import pick from 'lodash.pick'; import deprecate from 'util-deprecate'; +import deepEqual from 'fast-deep-equal'; import { create, themes } from '@storybook/theming'; import merge from '../libs/merge'; @@ -132,7 +133,7 @@ export default function({ store }) { }, setOptions: options => { - const { layout, ui, selectedPanel } = store.getState(); + const { layout, ui, selectedPanel, theme } = store.getState(); if (options) { const updatedLayout = { @@ -144,17 +145,31 @@ 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' }); + } } }, }; 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 }) => ( - + )} From 8b28b301638f26f7e6fba1716dd461c94c702b53 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 4 Mar 2019 21:56:30 +0100 Subject: [PATCH 11/16] FIX tests --- addons/knobs/src/components/__tests__/Panel.js | 8 ++++---- lib/ui/src/settings/shortcuts.test.js | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) 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/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( - + ); From fac9ee3fd98af08faa964f4701ecd8ac33143076 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 5 Mar 2019 00:04:42 +0100 Subject: [PATCH 12/16] FIX incorrect optimization --- lib/ui/src/core/layout.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/ui/src/core/layout.js b/lib/ui/src/core/layout.js index ad9657669016..8f6566eb6b80 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -3,7 +3,7 @@ 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 = { @@ -22,14 +22,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 => { @@ -154,16 +152,16 @@ export default function({ store }) { }; const modification = {}; - if (deepEqual(ui, updatedUi)) { + if (!deepEqual(ui, updatedUi)) { modification.ui = updatedUi; } - if (deepEqual(layout, updatedLayout)) { + if (!deepEqual(layout, updatedLayout)) { modification.layout = updatedLayout; } - if (deepEqual(theme, updatedTheme)) { + if (!deepEqual(theme, updatedTheme)) { modification.theme = updatedTheme; } - if (deepEqual(selectedPanel, options.selectedPanel)) { + if (!deepEqual(selectedPanel, options.selectedPanel)) { modification.selectedPanel = options.selectedPanel; } From 2aec10af700b989d8acc8db382cdd70b4aa5e562 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Tue, 5 Mar 2019 00:50:59 +0100 Subject: [PATCH 13/16] ADD special case for `barSelectedColor`, since it should be using `colorSecondary` as fallback --- lib/theming/src/create.ts | 8 ++++++-- lib/ui/src/core/layout.js | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/theming/src/create.ts b/lib/theming/src/create.ts index 4b0164d302fb..a4d44f016a3d 100644 --- a/lib/theming/src/create.ts +++ b/lib/theming/src/create.ts @@ -82,11 +82,15 @@ const darkSyntaxColors = { export const create = (vars: ThemeVars = { base: 'light' }, rest?: Rest): ThemeVars => { const inherit: ThemeVars = { ...themes.light, - ...(themes[vars.base] || themes.light), + ...(themes[vars.base] || {}), ...vars, ...{ base: themes[vars.base] ? vars.base : 'light' }, }; - return { ...rest, ...inherit }; + return { + ...rest, + ...inherit, + ...{ barSelectedColor: vars.barSelectedColor || inherit.colorSecondary }, + }; }; export const convert = (inherit: ThemeVars = lightThemeVars): Theme => { diff --git a/lib/ui/src/core/layout.js b/lib/ui/src/core/layout.js index 8f6566eb6b80..9f576e542c47 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -10,6 +10,7 @@ const deprecatedThemeOptions = { name: 'brandTitle', url: 'brandUrl', }; + const deprecatedLayoutOptions = { goFullScreen: 'isFullscreen', showStoriesPanel: 'showNav', @@ -152,6 +153,7 @@ export default function({ store }) { }; const modification = {}; + if (!deepEqual(ui, updatedUi)) { modification.ui = updatedUi; } From ddead74aefff849fe2ead99dbffaa210eba9739f Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 5 Mar 2019 11:12:09 +1100 Subject: [PATCH 14/16] FIX the location of the default theme on state --- lib/ui/src/core/layout.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/src/core/layout.js b/lib/ui/src/core/layout.js index 9f576e542c47..51d6bef0b530 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -181,7 +181,6 @@ export default function({ store }) { enableShortcuts: true, sortStoriesByKind: false, sidebarAnimations: true, - theme: themes.light, }, layout: { isToolshown: true, @@ -190,6 +189,7 @@ export default function({ store }) { showNav: true, panelPosition: 'bottom', }, + theme: themes.light, }; const state = merge(fromState, initial); From e5696dc489403c2970c733e9ba914f5d48717f52 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 5 Mar 2019 11:27:24 +1100 Subject: [PATCH 15/16] Ensure the theme is insta-loaded from local storage --- lib/ui/src/core/layout.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/src/core/layout.js b/lib/ui/src/core/layout.js index 51d6bef0b530..52b9dd5c701b 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -174,7 +174,7 @@ export default function({ store }) { }, }; - const fromState = pick(store.getState(), 'layout', 'ui', 'selectedPanel'); + const fromState = pick(store.getState(), 'layout', 'ui', 'selectedPanel', 'theme'); const initial = { ui: { @@ -192,7 +192,7 @@ export default function({ store }) { theme: themes.light, }; - const state = merge(fromState, initial); + const state = merge(initial, fromState); return { api, state }; } From eeb024fa65b0136db38523d4b4ab2b1873776305 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 5 Mar 2019 12:05:07 +1100 Subject: [PATCH 16/16] FIX Don't retain persisted UI state For #5857 --- lib/ui/src/core/layout.js | 48 +++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/lib/ui/src/core/layout.js b/lib/ui/src/core/layout.js index 52b9dd5c701b..ae308c73c0c6 100644 --- a/lib/ui/src/core/layout.js +++ b/lib/ui/src/core/layout.js @@ -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, theme } = 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 = { @@ -170,29 +193,14 @@ export default function({ store }) { if (Object.keys(modification).length) { store.setState(modification, { persistence: 'permanent' }); } - } - }, - }; - - const fromState = pick(store.getState(), 'layout', 'ui', 'selectedPanel', 'theme'); - const initial = { - ui: { - enableShortcuts: true, - sortStoriesByKind: false, - sidebarAnimations: true, - }, - layout: { - isToolshown: true, - isFullscreen: false, - showPanel: true, - showNav: true, - panelPosition: 'bottom', + hasSetOptions = true; + } }, - theme: themes.light, }; - const state = merge(initial, fromState); + const persisted = pick(store.getState(), 'layout', 'ui', 'selectedPanel', 'theme'); + const state = merge(initial, persisted); return { api, state }; }