react-theming / storybook-addon-material-ui

Addon for storybook wich wrap material-ui components into MuiThemeProvider. :page_with_curl: This helps and simplifies development of material-ui based components.
https://github.com/sm-react/storybook-addon-material-ui
MIT License
567 stars 130 forks source link

[v1] How to keep custom theme selected while browsing stories #87

Open pandaiolo opened 5 years ago

pandaiolo commented 5 years ago

I don't know if i'm doing this correctly, but it seemed to me that when switching to another story, the theme is reset to index 0. It means that to review a theme for a bunch of components, I have to switch story / reselect said theme each time.

We have more than a hundred themes for while-label customers, so the process is a bit cumbersome.

I have hacked a hacky workaround for this, under the form of an addon, as following:

import addons from '@storybook/addons'
import {
  EVENT_ID_DATA,
  EVENT_ID_BACK,
  EVENT_ID_INIT
} from 'storybook-addon-material-ui/dist/config'

const INITIAL_THEME = { themeInd: 1 }
const THEME_RESET_DELAY = 50

addons.register('keepTheme', api => {
  const channel = addons.getChannel()
  let themeData = null

  const onThemeChange = data => {
    if (data.themeInd == 0) {
      keepTheme()
    } else {
      themeData = data
    }
  }

  const keepTheme =() => {
    setTimeout(() => {
      channel.emit(EVENT_ID_BACK, themeData || INITIAL_THEME)
      channel.emit(EVENT_ID_DATA, themeData || INITIAL_THEME)
    }, THEME_RESET_DELAY)
  }

  // Keep theme data in local variable when it changes
  channel.on(EVENT_ID_DATA, onThemeChange)
  channel.on(EVENT_ID_BACK, onThemeChange)

  api.onStory(() => keepTheme())
})

It is far from perfect but it is the most stable I've came up with, even though theme at index 0 flashes at each new story, before selecting the required theme.

Basically, it always switches off the theme 0 to either theme 1 or user requested theme, each time there is a new story visited.

(I tried without immediately switching to theme != 0, but it was buggy because I could tell if theme 0 was a reset or user selected... and I did not have that much time to figure it out)

Anyway, is it a feature that you would consider for the addon? Would you like a PR? Any specific implementation design you have in mind, if so?

Or I just missed an option? :-)

Thanks!

usulpro commented 5 years ago

Hi! @pandaiolo thank you for your attention to this issue. Indeed, keeping selected theme for each story is the feature which is not implemented yet. And it's on the first place in my roadmap, but I didn't start it yet :smile:

PR is really welcome. Would you mind submitting it? Don't worry about your solution I'll check it and continue if it's necessary. (I guess we would like to use storybookAPI.onStory(fn) to detect story switching)

pandaiolo commented 5 years ago

@UsulPro that's great! I used that onStory hook in my above solution, but the material-ui addon is reseting the index to 0 when a story changes, leading to a race condition that led me to add a setTimeout... not the sexiest implementation (also because it's visible to the user).

So I was wondering why the theme was reset, and because I am new to storybook, I am not sure exactly, but I suspect MUI decorator are reloaded each time, that's right? So it would fire a new store, with its initial value, each time, that's right?

Hence some kind of variable in the global scope seem necessary? I have yet to figure out more details on how your addon work.

usulpro commented 5 years ago

Yes, story component did mount each time when selecting new story.

Hence some kind of variable in the global scope seem necessary?

...hm I trying to remember how it was implemented in previous version of addon. Do you need to keep one same theme while browsing stories or you want to remember different themes, one for each story? In the first case we can keep theme index in the query string. You can test how it works in this demo. Additionally it allows to share links with selected themes and stories.

If it helps you can find how it was implemented here: https://github.com/react-theming/storybook-addon-material-ui/blob/84907ea5d43d397c6374e37bcda4701a0661c84b/src/containers/PanelContainer.js#L195-L197

pandaiolo commented 5 years ago

Hum, that's a good option. Yes, my use case is to keep the same set of themes, select a specific one that I want to check on different component/stories

usulpro commented 5 years ago

Possibly you could start this feature? I'll join a bit later this week :smile:

pandaiolo commented 5 years ago

I had a look but not totally obvious to me where to start. I probably won't have time to get this done in the short term.

poisins commented 4 years ago

Had some issues with original workaround when selecting first theme (index 0), so played around a bit (without going deeper in addons source code as I'm quite new to storybook as such).

Here's my register.js. Working with addons version 0.9.0-alpha.21, and has same flickering issue.

import addons from '@storybook/addons';
import {
    EVENT_ID_DATA,
    EVENT_ID_BACK,
    ADDON_ID
} from 'storybook-addon-material-ui/dist/config';

// keeps selected storybook theme when switching stories
// not the prettiest implementation (has small flicker on story change), but does the work
addons.register(\`${ADDON_ID}-theme-keeper\`, api => {
    const channel = addons.getChannel();
    let currentThemeInd = null;

    const onStoryLoad = (data) => {
        // use stored or one we are switching to
        data.themeInd = currentThemeInd ?? data.themeInd;

        // switch to desired theme
        channel.emit(EVENT_ID_BACK, data);
    }

    const onThemeChange = (data) => {
        // store locally
        currentThemeInd = data.themeInd;
    }

    channel.on(EVENT_ID_DATA, onStoryLoad); // EVENT_ID_DATA is triggered during story load/switch
    channel.on(EVENT_ID_BACK, onThemeChange); // EVENT_ID_BACK is triggered when switching theme
});