Set ngClass/ngStyle from props

[spaceribs]

Apologies, I'm still a bit of a noob writing stories with Storybook. When building out angular component stories, I'd like to be able to set a class to style the component in different ways. Currently I'm using templates to perform that:

export const Flashing = args => ({
  moduleMetadata: {
    imports: [DotModule],
  },
  template: `
  <app-dot
    [color]="color"
    [ngClass]="ngClass"
  ></app-dot>
  `,
  props: {
    color: '#00F',
    ngClass: DotState.Flashing
    ...args,
  },
});

I'd actually much prefer the following format, but it doesn't seem to work:

export const Flashing = args => ({
  moduleMetadata: {
    imports: [DotModule],
  },
  component: DotComponent,
  props: {
    color: '#00F',
    ngClass: ['flashing'],
    ...args,
  },
});

I'd be happy to help if this makes any sense to implement or there are no proposed alternatives.

[github-actions]

Automention: Hey @MaximSagan @kroeder, you've been tagged! Can you give a hand here?

[Marklb]

I have been thinking about this and my main hesitation is adding custom implementations for directives, because if the functionality isn't exactly the same or someone has another directive/component that thinks it can inject a reference to the directive, it may be confusing.

If we could make directives work without a template story, then this would be possible, by just using the directive. I don't know how well it would work yet, but I wonder if we could do something like I am trying to do in #12438 in the core renderer. That way we could let Angular handle the rendering from a template and be able to support directives. It may be easier to know if we could support this, after Storybook's Angular build process is updated.

I haven't seen any dynamic Component libraries support directives, without limitations. It makes sense to not build a template string for rendering a component in an app, but for Storybook's use case, it may make sense to do that.

[spaceribs]

Makes sense, I definitely feel as though the component param is an oversimplification of what angular considers to be just another directive, from the docs:

A component is technically a directive. However, components are so distinctive and central to Angular applications that Angular defines the @component() decorator, which extends the @directive() decorator with template-oriented features.

We could reconceptualize this as a collection:

directives: [
    {
        directive: DotComponent,
        props: { color: '#00F' }
    },
    {
        directive: NgClass,
        props: { ngClass: ['flashing'] }
    }
]

I don't love it, but it's certainly closer to how Angular works architecturally.

EDIT: i do want to add that I've purposefully left out discussing structural directives within the above example, which may further complicate things 😓

[Marklb]

@spaceribs As far as how they would be defined, something like that is what I was thinking.

The blocker that I am thinking we would need, unless we dynamically create the story component from a template in the renderer, is explained in this issue: angular/angular#8785. We would basically just be composing the directives and there just doesn't seem to be an easy way to do it from js still, for some reason.

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[spaceribs]

@ThibaudAV @Marklb per: #13215 (comment) can we close this or does it still need to be documented?

[ThibaudAV]

No, I don't think so :/ . What has been done in #13215 (comment) does not correct this problem.
IMHO: 2 functionalities are opposed because we want to do everything in a single props object, which makes it impossible to support (actually) this issue.

Currently props properties can be of these types:

• angular input / output
• internal component properties (property that is not an input/output of current component)

Current behaviour : (simple explanation)

This component ->

@Component({selector: 'my-component', template: '' }) 
class MyComponent {

@Input() input: string;

@Output() output = new EventEmitter<string>();

localProp = 'only for template'
}

With this story ->

export const Default = args => ({
  component: MyComponent,
  props: {
    input: 'new input value',
    output: action('output'),
    localProp: 'new localProp'
  },
});

Generates this template: <my-component [input]="input" (output)="output($event)"></my-component> which is added to internal angular component : storybook-wrapper

The storybook-wrapper component contains all props values as properties, which allows it to pass them on to components present in its template (as a real use of this one in angular). For props remaining that are not input/output (like localProp) storybook-wrapper retrieves the instance of the child component and modifies it directly.

It should be possible to remove local properties of props in order to consider all default props as a template attribute.
and thus be able to add class, ngClass, style, other directive and even why not an *ngIf
Local props could go into another localProps (componentInternalProps 🤷‍♂️ ) object
I think it would be more logical and intuitive.

But this causes braking changes.

There may be other possibilities. And even this one requires some feedback before doing a PR. i think 🤔

[spaceribs]

damn @ThibaudAV you totally got my hopes up with:

With this solution it works without any problems. If my quick test is correct

let me know of course if I can help plan out this functionality within the grander scheme of things @Marklb

[ThibaudAV]

😄 haha sorry. I didn't have all the subtlety in mind yet.
I've got some 2 pr's in progress here. I am waiting for feedback and also the end of holidays to finish them. Maybe after 🤷‍♂️

[ndelangen]

Is this still an issue in 7.0 beta @spaceribs @ThibaudAV ?

[Marklb]

@ndelangen Yes. There is no way to identify a property in props as a directive. There is a chance the requested syntax would currently work, but the directive would most likely only receive the initial value and not see additional props changes.

I have a fairly large change I want to make to the Angular renderer, which I hope to propose in a PR soon, that would potentially make supporting this a little easier. The change I will be proposing reads more information about the compiled template and lets Angular handle more property updating from the template, instead of all properties being directly updated on the component instance.

[ndelangen]

You should probably talk to @kroeder and @valentinpalkovic about this PR you have in mind! 👍

[valentinpalkovic]

@Marklb Let me know if you need any support.