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 @@ -5178,10 +5178,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/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 = {};

this.modules = [
initChannel,
initAddons,
Expand All @@ -74,7 +75,7 @@ export class Provider extends Component {
].map(initModule => initModule(apiData));

// Create our initial state by combining the initial state of all modules, then overlaying any saved state
const state = getInitialState(...this.modules.map(m => m.state));
const state = store.getInitialState(getInitialState(...this.modules.map(m => m.state)));

// Get our API by combining the APIs exported by each module
const combo = Object.assign({ navigate }, ...this.modules.map(m => m.api));
Expand All @@ -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
13 changes: 8 additions & 5 deletions lib/ui/src/core/init-provider-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,14 @@ export default ({ provider, api, store }) => {
...checkDeprecatedThemeOptions(options),
};

store.setState({
layout: updatedLayout,
ui: updatedUi,
selectedPanel: options.panel || options.selectedPanel || selectedPanel,
});
store.setState(
{
layout: updatedLayout,
ui: updatedUi,
selectedPanel: options.panel || options.selectedPanel || selectedPanel,
},
{ persistence: 'permanent' }
);
}
},
};
Expand Down
3 changes: 2 additions & 1 deletion lib/ui/src/core/shortcuts.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { navigator, document } from 'global';
import { PREVIEW_KEYDOWN } from '@storybook/core-events';

import getInitialState from './initial-state';
import { shortcutMatchesShortcut, eventToShortcut } from '../libs/shortcut';

export const isMacLike = () =>
Expand Down Expand Up @@ -31,7 +32,7 @@ export default function initShortcuts({ store }) {
const api = {
// Getting and setting shortcuts
getShortcutKeys() {
return store.getState().shortcuts;
return store.getState().shortcuts || getInitialState().shortcuts;
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
},
async setShortcuts(shortcuts) {
await store.setState({ shortcuts }, { persistence: 'permanent' });
Expand Down
11 changes: 7 additions & 4 deletions lib/ui/src/core/store.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
// TODO -- make this TS?

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

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

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 All @@ -29,11 +32,11 @@ export default class Store {

// The assumption is that this will be called once, to initialize the React state
// when the module is instanciated
getInitialState() {
getInitialState(base = {}) {
// We don't only merge at the very top level (the same way as React setState)
// when you set keys, so it makes sense to do the same in combining the two storage modes
// Really, you shouldn't store the same key in both places
return { ...get(localStorage), ...get(sessionStorage) };
return merge(base, { ...get(localStorage), ...get(sessionStorage) });
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
}

getState() {
Expand Down
22 changes: 1 addition & 21 deletions lib/ui/src/core/stories.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,6 @@
import mergeWith from 'lodash.mergewith';
import isEqual from 'lodash.isequal';
import { toId, sanitize } from '@storybook/router/utils';

const merge = (a, b) =>
mergeWith({}, a, b, (objValue, srcValue) => {
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)) {
// eslint-disable-next-line no-console
console.log('the types mismatch, picking', objValue);
return objValue;
}
return undefined;
});
import merge from '../libs/merge';

const initStoriesApi = ({
store,
Expand Down