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.

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

FIX theming bugs #5787

Merged
merged 13 commits into from
Mar 2, 2019
Merged
1 change: 0 additions & 1 deletion examples/official-storybook/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ addParameters({
options: {
hierarchySeparator: /\/|\./,
hierarchyRootSeparator: '|',
// theme: themes.dark,
},
viewports: {
...INITIAL_VIEWPORTS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default {
// Given we sort of control the props, should we export a prop type?
export const passed = ({
// eslint-disable-next-line react/prop-types
parameters,
parameters: { options, ...parameters },
}) => <pre>Parameters are {JSON.stringify(parameters, null, 2)}</pre>;
passed.title = 'passed to story';
passed.parameters = { storyParameter: 'storyParameter' };
Original file line number Diff line number Diff line change
Expand Up @@ -5195,10 +5195,6 @@ exports[`Storyshots Core|Events Force re-render 1`] = `
exports[`Storyshots Core|Parameters passed to story 1`] = `
<pre>
Parameters are {
"options": {
"hierarchyRootSeparator": "|",
"hierarchySeparator": {}
},
"a11y": {
"configure": {},
"options": {
Expand Down
2 changes: 1 addition & 1 deletion lib/channel-postmessage/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class PostmsgTransport {
});
}

const data = stringify({ key: KEY, event }, { maxDepth: 10 });
const data = stringify({ key: KEY, event }, { maxDepth: 15 });

// TODO: investigate http://blog.teamtreehouse.com/cross-domain-messaging-with-postmessage
// might replace '*' with document.location ?
Expand Down
8 changes: 7 additions & 1 deletion lib/theming/src/ensure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ 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) => {
Expand All @@ -32,7 +38,7 @@ export const ensure = (input: any): Theme => {
if (!input) {
return light;
} else {
const missing = deletedDiff(light, input);
const missing = deletedDiff(base, input);
if (Object.keys(missing).length) {
logger.warn(
stripIndent`
Expand Down
1 change: 1 addition & 0 deletions lib/ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"react-resize-detector": "^3.2.1",
"recompose": "^0.30.0",
"semver": "^5.6.0",
"telejson": "^2.1.1",
"util-deprecate": "^1.0.2"
},
"devDependencies": {
Expand Down
9 changes: 6 additions & 3 deletions lib/ui/src/components/layout/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,18 @@ class Layout extends Component {
constructor(props) {
super(props);

const { options } = this.props;
const { bounds, options } = props;

const { resizerNav, resizerPanel } = persistance.get();

this.state = {
isDragging: false,
resizerNav: resizerNav || { x: 200, y: 0 },
resizerPanel:
resizerPanel || (options.panelPosition === 'bottom' ? { x: 0, y: 400 } : { x: 400, y: 0 }),
resizerPanel ||
(options.panelPosition === 'bottom'
? { x: 0, y: bounds.height - 400 }
: { x: bounds.width - 400, y: 0 }),
};
}

Expand Down Expand Up @@ -323,7 +326,7 @@ class Layout extends Component {
}
}
if (isPanelRight && !isPanelHidden) {
if (bounds.width - 200 < panelX) {
if (bounds.width - 200 < panelX || panelX === 0) {
mutation.resizerPanel = {
x: bounds.width - 200,
y: 0,
Expand Down
4 changes: 2 additions & 2 deletions lib/ui/src/core/addons.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ export default ({ provider, store }) => {
return ensurePanel(api.getPanels(), selectedPanel, selectedPanel);
},
setSelectedPanel: panelName => {
store.setState({ selectedPanel: panelName });
store.setState({ selectedPanel: panelName }, { persistence: 'session' });
},
};

return {
api,
state: {
selectedPanel: ensurePanel(api.getPanels()),
selectedPanel: ensurePanel(api.getPanels(), store.getState().selectedPanel),
},
};
};
7 changes: 5 additions & 2 deletions lib/ui/src/core/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export class Provider extends Component {
// Initialize the state to be the initial (persisted) state of the store.
// This gives the modules the chance to read the persisted state, apply their defaults
// and override if necessary
this.state = store.getInitialState();
ndelangen marked this conversation as resolved.
Show resolved Hide resolved

const apiData = {
navigate,
Expand All @@ -62,6 +61,8 @@ export class Provider extends Component {
storyId,
};

this.state = store.getInitialState();

this.modules = [
initChannel,
initAddons,
Expand Down Expand Up @@ -90,7 +91,9 @@ export class Provider extends Component {
api.on(SET_STORIES, data => {
api.setStories(data.stories);

const options = api.getParameters(storyId, 'options');
const options = storyId
? api.getParameters(storyId, 'options')
: api.getParameters(Object.keys(data.stories)[0], 'options');

api.setOptions(options);
});
Expand Down
82 changes: 1 addition & 81 deletions lib/ui/src/core/init-provider-api.js
Original file line number Diff line number Diff line change
@@ -1,86 +1,6 @@
import pick from 'lodash.pick';
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
import deprecate from 'util-deprecate';

import { create } from '@storybook/theming';

const deprecationMessage = (optionsMap, prefix) =>
`The options { ${Object.keys(optionsMap).join(', ')} } are deprecated -- use ${
prefix ? `${prefix}'s` : ''
} { ${Object.values(optionsMap).join(', ')} } instead.`;

const deprecatedThemeOptions = {
name: 'brandTitle',
url: 'brandUrl',
};
const applyDeprecatedThemeOptions = deprecate(({ name, url, theme }) => {
const vars = {
brandTitle: name,
brandUrl: url,
brandImage: null,
};

return { theme: create(vars, theme) };
}, deprecationMessage(deprecatedThemeOptions));
const checkDeprecatedThemeOptions = options => {
if (Object.keys(deprecatedThemeOptions).find(key => !!options[key])) {
return applyDeprecatedThemeOptions(options);
}
return {};
};

const deprecatedLayoutOptions = {
goFullScreen: 'isFullscreen',
showStoriesPanel: 'showNav',
showAddonPanel: 'showPanel',
addonPanelInRight: 'panelPosition',
};
const applyDeprecatedLayoutOptions = deprecate(options => {
const layoutUpdate = {};

['goFullScreen', 'showStoriesPanel', 'showAddonPanel'].forEach(option => {
if (typeof options[option] !== 'undefined') {
layoutUpdate[deprecatedLayoutOptions[option]] = options[option];
}
});
if (options.addonPanelInRight) {
layoutUpdate.panelPosition = 'right';
}
return layoutUpdate;
}, deprecationMessage(deprecatedLayoutOptions));
const checkDeprecatedLayoutOptions = options => {
if (Object.keys(deprecatedLayoutOptions).find(key => typeof options[key] !== 'undefined')) {
return applyDeprecatedLayoutOptions(options);
}
return {};
};

export default ({ provider, api, store }) => {
export default ({ provider, api }) => {
const providerAPI = {
...api,

setOptions: options => {
const { layout, ui, selectedPanel } = store.getState();

if (options) {
const updatedLayout = {
...layout,
...pick(options, Object.keys(layout)),
...checkDeprecatedLayoutOptions(options),
};

const updatedUi = {
...ui,
...pick(options, Object.keys(ui)),
...checkDeprecatedThemeOptions(options),
};

store.setState({
layout: updatedLayout,
ui: updatedUi,
selectedPanel: options.panel || options.selectedPanel || selectedPanel,
});
}
},
};

provider.handleAPI(providerAPI);
Expand Down
15 changes: 0 additions & 15 deletions lib/ui/src/core/initial-state.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,6 @@
import { themes } from '@storybook/theming';

import merge from '../libs/merge';

const initial = {
ui: {
enableShortcuts: true,
sortStoriesByKind: false,
sidebarAnimations: true,
theme: themes.normal,
},
layout: {
isToolshown: true,
isFullscreen: false,
showPanel: true,
showNav: true,
panelPosition: 'bottom',
},
customQueryParams: {},
storiesConfigured: false,
};
Expand Down
110 changes: 109 additions & 1 deletion lib/ui/src/core/layout.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,64 @@
import pick from 'lodash.pick';

import deprecate from 'util-deprecate';

import { create, themes } from '@storybook/theming';
import merge from '../libs/merge';

const deprecatedThemeOptions = {
name: 'brandTitle',
url: 'brandUrl',
};
const deprecatedLayoutOptions = {
goFullScreen: 'isFullscreen',
showStoriesPanel: 'showNav',
showAddonPanel: 'showPanel',
addonPanelInRight: 'panelPosition',
};

const deprecationMessage = (optionsMap, prefix) =>
`The options { ${Object.keys(optionsMap).join(', ')} } are deprecated -- use ${
prefix ? `${prefix}'s` : ''
} { ${Object.values(optionsMap).join(', ')} } instead.`;

const applyDeprecatedThemeOptions = deprecate(({ name, url, theme }) => {
const vars = {
brandTitle: name,
brandUrl: url,
brandImage: null,
};

return { theme: create(vars, theme) };
}, deprecationMessage(deprecatedThemeOptions));

const applyDeprecatedLayoutOptions = deprecate(options => {
const layoutUpdate = {};

['goFullScreen', 'showStoriesPanel', 'showAddonPanel'].forEach(option => {
if (typeof options[option] !== 'undefined') {
layoutUpdate[deprecatedLayoutOptions[option]] = options[option];
}
});
if (options.addonPanelInRight) {
layoutUpdate.panelPosition = 'right';
}
return layoutUpdate;
}, deprecationMessage(deprecatedLayoutOptions));

const checkDeprecatedThemeOptions = options => {
if (Object.keys(deprecatedThemeOptions).find(key => !!options[key])) {
return applyDeprecatedThemeOptions(options);
}
return {};
};

const checkDeprecatedLayoutOptions = options => {
if (Object.keys(deprecatedLayoutOptions).find(key => typeof options[key] !== 'undefined')) {
return applyDeprecatedLayoutOptions(options);
}
return {};
};

export default function({ store }) {
const api = {
toggleFullscreen(toggled) {
Expand Down Expand Up @@ -69,7 +130,54 @@ export default function({ store }) {
};
});
},

setOptions: options => {
const { layout, ui, selectedPanel } = store.getState();

if (options) {
const updatedLayout = {
...layout,
...pick(options, Object.keys(layout)),
...checkDeprecatedLayoutOptions(options),
};

const updatedUi = {
...ui,
...pick(options, Object.keys(ui)),
...checkDeprecatedThemeOptions(options),
};

store.setState(
{
layout: updatedLayout,
ui: updatedUi,
selectedPanel: options.panel || options.selectedPanel || selectedPanel,
},
{ persistence: 'permanent' }
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we perma-persist all options, including the ones that were session-persisted above?

I'm OK with this for now for simplicity, but we might want to do something more subtle in the future (which will probably require improvements to the persistence API)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I briefly experimented with persisting all the layout & ui options, but decided to not increase the size of this PR even more.

But it was actually really nice, things like full-screen, nav, panelPosition being preserved over refreshes...

On my todo list after V5 launch.

}
},
};

return { api };
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);

return { api, state };
}
5 changes: 3 additions & 2 deletions lib/ui/src/core/store.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
// TODO -- make this TS?

import { localStorage, sessionStorage } from 'global';
import { parse, stringify } from 'telejson';

export const STORAGE_KEY = '@storybook/ui/store';

function get(storage) {
const serialized = storage.getItem(STORAGE_KEY);
return serialized ? JSON.parse(serialized) : {};
return serialized ? parse(serialized) : {};
}

function set(storage, value) {
storage.setItem(STORAGE_KEY, JSON.stringify(value));
storage.setItem(STORAGE_KEY, stringify(value, { maxDepth: 50 }));
}

function update(storage, patch) {
Expand Down
Loading