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

Provide decorator support for Angular #24

Open
CodeByAlex opened this issue Mar 19, 2020 · 18 comments
Open

Provide decorator support for Angular #24

CodeByAlex opened this issue Mar 19, 2020 · 18 comments
Labels
enhancement New feature or request

Comments

@CodeByAlex
Copy link

CodeByAlex commented Mar 19, 2020

@tonai do you know what’s causing the addon to not support Angular?

@tonai
Copy link
Owner

tonai commented Mar 20, 2020

Hi @CodeByAlex in fact each framework need its custom decorator development.
I can check when I have some time.

@CodeByAlex
Copy link
Author

That would be awesome! Thanks - please let me know if I could be of help @tonai

@tonai
Copy link
Owner

tonai commented Mar 24, 2020

Hi @CodeByAlex , I started to have a look to implement Angular but there is some difficulties and maybe you can help if you want.

The decorator is here on the feat/angular branch.

The problem is that the component must rerender when the CHANGE event is triggered from the addon object.
That's why I use the ChangeDetectorRef service on ligne 35.
But the problem is that I got an error when I try to use the addon inside an Angular storybook : Unhandled Promise rejection: Can't resolve all parameters for ThemeDecorator: (?).
It seems it fails to inject the dependency in the constructor and I really don't know why.

If you want to help, notice you will have to use the storybook 6.x alpha version.
Thanks for your help.

@CodeByAlex
Copy link
Author

Hey there - I’ll take a look 👆 thanks for getting this started so quickly

@CodeByAlex
Copy link
Author

CodeByAlex commented Mar 24, 2020

@tonai Are there any special setup steps that I should know about? And do you have an example code snippet of how it would be used? Dont see a code snippet for vue so its a but hard to determine how things are supposed to be set up when actually used

@tonai
Copy link
Owner

tonai commented Mar 25, 2020

Hello @CodeByAlex here is a little repository that reproduce the problem : https://github.com/tonai/storybook-angular

@CodeByAlex
Copy link
Author

CodeByAlex commented Mar 25, 2020

Hey @tonai that repo will definitely help - thanks. One thing I’m still missing is how you're thinking the decorator will look/what code you were using to test the decorator when you saw it fail. Based on what I’ve seen, the react decorator setup is very different from the HTML set up so I’m curious what you had in mind for angular.

Also wouldn't it be easier to provide a generic decorator that works similarly to your HTML version and have the user pass in specific params like theme name and code? That way you wouldn't have to maintain framework-specific decorators and it would require less from the user.

Example usage:

decorator([
    {
        id: 'Theme one',
        code:`<style>${require(!css-loader!../theme1.css')</style>}`,
    },
    {
        id: 'Theme two',
        code:`<style>${require(!css-loader!../theme2.css')</style>}`,
    }
])

@tonai
Copy link
Owner

tonai commented Mar 25, 2020

Hum I don't really understand your question @CodeByAlex .
The setup of the decorator in the storybook is the same for all frameworks.
Example:

import { withThemes } from 'storybook-addon-themes/angular';
export default {
  title: 'Button',
  component: Button,
  decorators: [withThemes],
  parameters: {
    themes: {
      list: [
        { name: 'twitter', color: '#00aced', default: true, class: 'theme-twt' },
        { name: 'facebook', color: '#3b5998', class: 'theme-fb' },
      ]
    }
  },
};

You can see that code in the example repo: https://github.com/tonai/storybook-angular/blob/master/src/stories/1-Button.stories.ts

If you want to see the erreur, clone the repo, install deps, run npm run storybook and select a story that is not the first one.

@CodeByAlex
Copy link
Author

CodeByAlex commented Mar 25, 2020

@tonai What I mean is we have these custom Decorators for React like this:

function Decorator(props) {
  const { children, themeName } = props;
  return (
    <>
      {children}
      {themeName === 'twitter' && <link rel="stylesheet" href="twitter.css"/>}
      {themeName === 'facebook' && <link rel="stylesheet" href="facebook.css"/>}
    </>
  );
};

and HTML like this:

function getOrCreate(id) {
  const elementOnDom = document.getElementById(id);
  if (elementOnDom) {
    return elementOnDom;
  }

  const element = document.createElement('link');
  element.setAttribute('id', id);
  element.setAttribute('rel', 'stylesheet');
  return element;
}

function Decorator(props) {
  const { children } = props;

  function setStyles({ theme, themeName }) {
    const link = getOrCreate('custom-stylesheet');
    if (!theme) {
      link.parentNode && link.parentNode.removeChild(link);
    } else {
      console.log(theme);
      link.href = theme.css;
      children.appendChild(link);
    }
  }
  setStyles(props);

  return [children, setStyles];
}

What do we expect the Angular one to look like?
I didnt see it int the repo you sent over to test with.

My other question is, instead of having the user create the decorators above, why not keep a generic one internal that looks similar to the HTML version above. Then pass certain data props to that decorator like in my earlier comment:

genericDecorator([
    {
        themeName: 'Theme one',
        code:`<style>${require(!css-loader!../theme1.css')</style>}`,
    },
    {
        themeName: 'Theme two',
        code:`<style>${require(!css-loader!../theme2.css')</style>}`,
    }
])

So the new setup from a user's perspective would be this:

genericDecorator([
    {
        themeName: 'twitter',
        code:`<style>${require(!css-loader!../twitter.css')</style>}`,
    },
    {
        themeName: 'facebook',
        code:`<style>${require(!css-loader!../facebook.css')</style>}`,
    }
])

export default {
  title: 'Button',
  component: Button,
  decorators: [withThemes, genericDecorator],
  parameters: {
    themes: [
        { name: 'twitter', color: '#00aced', default: true},
        { name: 'facebook', color: '#3b5998' },
     ]
  },
};

@tonai
Copy link
Owner

tonai commented Mar 26, 2020

I don't know how custom decorators will be handled for now.
If possible I want it to look like a standard Angular component.
But before managing the custom decorator option I want to be able to display the standard addon decorator (I mean this one).

@CodeByAlex
Copy link
Author

CodeByAlex commented Mar 28, 2020

@tonai - it would be best to make your addon logic framework agnostic (the best and most widely used addons are and it makes it more maintainable). The problem here is that the decorator is expecting it to be passed a change detector reference but it could be that since you are simply trying to compile the ts., it is missing something that the angular build process normally might pick up

@tonai
Copy link
Owner

tonai commented Mar 28, 2020

@CodeByAlex You can't make the addon framwork agnostic for decorators.
The solution I propose without using a decorator is indeed framework agnostic.
Bur if you look at the code of the centered addon you will notice there is a decorator for each framework.
And tht's why I only support framework decorators when there is a need for it.
Maybe there is something missing in the tsconfig.json file to compile angular files correclty...

@CodeByAlex
Copy link
Author

CodeByAlex commented Mar 28, 2020

@tonai This for instance is a framework agnostic decorator: https://github.com/CodeByAlex/storybook-theme-knob.

@tonai
Copy link
Owner

tonai commented Mar 28, 2020

@CodeByAlex with that addon you are injetting code into the body but you are not touching the initial story.
My goal with the decorator is to wrap the initial story inside an a div element and add a class on it.
If you don't use the decorator, I add a class on the body element and that's why this version is framework agnostic.
So why proposing a decorator for each framework ? It is for example to be compatible with the storyshots addon.
I think your addon will not work properly with the storyshots addon.

@CodeByAlex
Copy link
Author

When the decorator manipulates the Dom, it manipulates what’s in the story itself which is why it works with docs. If you were to eject the story from the storybook app, it would use the default theme.

@tonai
Copy link
Owner

tonai commented Mar 30, 2020

When you capture a story with the storyshots addon, it will only capture the story itself (and not all the DOM from the body).

So all DOM manipulations you do in your addon (or with this addon without using the decorator) won't get captured.

That's why I created the decorator version to add an intermediate div englobing the story that is correclty captured by the snapshot.

@the-ult
Copy link

the-ult commented Oct 9, 2020

Hi @tonai , is there any change the Angular decorator you created will be merged?
Or do you still have some errors?

@tonai
Copy link
Owner

tonai commented Dec 3, 2020

Sorry for the late reply.
I did not retest it with the latest version of storybook.
Maybe the original update issue is not there anymore.
I will try to have a look soon

@tonai tonai added the enhancement New feature or request label Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants