Angular template support for Storybook #2690

alterx · 2018-01-08T19:40:24Z

What I did

Initial work to enable writing stories as follows:

storiesOf('Button', module)
  .add('with text', () => ({
    moduleMetadata: {
      entryComponents: [Button],
      declarations: [Button],
    },
    props: {
      text: 'Hello Button',
      onClick: (event) => {
        console.log('some bindings work');
        console.log(event)
      }
    },
    template: `
      <h1> This is a template </h1>
      <button-component [text]="text" (onClick)="onClick($event)"></button-component>
    `
  }))

How to test

yarn bootstrap --reset --core
cd examples/angular-cli
yarn storybook
open http://localhost:9009

Let's use JS for this, don't see a compelling reason to pull lodash. Also, I don't see it in the deps and it's probably working because most projects use it but it could potentially break if it's not there. Signed-off-by: Carlos Vega <[email protected]>

…ybook into angular-template-support

amcdnl · 2018-01-08T19:43:12Z

@alterx - I don't think template and component should be used at the same time.

amcdnl

LGTM

alterx · 2018-01-08T19:51:28Z

@amcdnl we currently need the component for:
a) Adding it to our NgModule
b) Getting the metadata

Even if we make the user provide the bindings as a separate object we'd still need the component to add it to our inner module.

codecov · 2018-01-08T19:53:47Z

Codecov Report

Merging #2690 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2690      +/-   ##
==========================================
- Coverage   34.36%   34.35%   -0.01%     
==========================================
  Files         390      390              
  Lines        8771     8772       +1     
  Branches      899      903       +4     
==========================================
  Hits         3014     3014              
+ Misses       5170     5150      -20     
- Partials      587      608      +21

Impacted Files	Coverage Δ
addons/knobs/src/angular/helpers.js	`0% <0%> (ø)`	⬆️
addons/knobs/src/components/types/Array.js	`34.14% <0%> (ø)`	⬆️
addons/knobs/src/components/types/Select.js	`7.93% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/containers/layout.js	`12.5% <0%> (ø)`	⬆️
lib/components/src/table/table.js	`83.33% <0%> (ø)`	⬆️
...tive/src/preview/components/StoryListView/index.js	`0% <0%> (ø)`	⬆️
addons/info/src/components/types/Enum.js	`0% <0%> (ø)`	⬆️
...ents/stories_panel/stories_tree/tree_decorators.js	`34.37% <0%> (ø)`	⬆️
.../ui/components/stories_panel/stories_tree/index.js	`38.73% <0%> (ø)`	⬆️
addons/actions/src/lib/types/function/index.js	`77.77% <0%> (ø)`	⬆️
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e8c6dc...590677f. Read the comment docs.

Let's use JS for this, don't see a compelling reason to pull lodash. Also, I don't see it in the deps and it's probably working because most projects use it but it could potentially break if it's not there. Signed-off-by: Carlos Vega <[email protected]>

…ybook into angular-template-support

igor-dv · 2018-01-08T19:57:10Z

@alterx , what if a template contains few components? Maybe worth adding them to the moduleMetadata ?
(also we have to check it with v4 🤦‍♂️ )

alterx · 2018-01-08T20:00:51Z

@igor-dv yeah, in that case we would add them to themoduleMetadata (it does it like that right now) but the part that worries me (and it's not yet resolved) is that we also need the actual metadata from each component so we can provide the prop.

This is an initial shot at the issue, I'd like @amcdnl to provide a real use case to see how we can implement these details

amcdnl · 2018-01-08T20:25:31Z

@alterx - I think you mis-understood me. I mean't for this example:

storiesOf('Button', module)
  .add('with text', () => ({
    moduleMetadata: {
       imports: [ButtonModule]
    },
    props: {
      text: 'Hello Button',
      onClick: (event) => {
        console.log('some bindings work');
        console.log(event)
      }
    },
    template: `
      <h1> This is a template </h1>
      <button-component [text]="text" (onClick)="onClick($event)"></button-component>
    `
  }))

Your example is confusing to me, seems like that would go in moduleMetadata's declarations in that situation.

alterx · 2018-01-08T20:31:39Z

@amcdnl yeah, I know you're talking about that example, I didn't misunderstood you. The component is still there because, currently, I'm using it to get the metadata.

alterx · 2018-01-08T20:35:01Z

This is just a contrived example, a POC. So it currently only supports creating the template using the bindings of that component we provide. That's why it has the label do-not-merge is not ready for primetime. I just submitted the PR for visibility (let people know is being worked on) and, of course, for you to test it.

amcdnl · 2018-01-08T20:48:10Z

@alterx - Why do you need it? I'm confused on that part I guess.

alterx · 2018-01-08T20:58:31Z

Because I didn't change the way this works. I just added the template parts on top of it so it still uses the component for props metadata. Hopefully it's not gonna be like that at the end. I just wanted to get it rendering a template.

Long story short: if you run the code as is and remove the component it breaks. Something that I need to fix. Wanted to be sure we can render it before adding more stuff and changing how it works.

Signed-off-by: Carlos Vega <[email protected]>

…-template-support

Signed-off-by: Carlos Vega <[email protected]>

alterx · 2018-01-10T01:26:14Z

With the use of moduleMetadata and some other changes this PR has also reduced the amount of code needed to bootstrap a component. The angular app is no longer doing manual checks on the metadata and only creates a dynamic component if the story provides a template*. The rest of the work is offloaded to Angular itself. Been wanting to do this for a while :D
As a result, the API remains almost intact (I added the template option) while the code complexity has been reduced.

*Manual metadata checks are still needed inside the knobs addon (which needs to manually deal with the change events that come from the knobs channel and send them to the angular component).

igor-dv

This is so much easier and cleaner! Did you check it with v4?

igor-dv · 2018-01-10T08:54:01Z

app/angular/src/client/preview/angular/helpers.ts

-  const NewModule: any = function() {};
-  (<IModule>NewModule).annotations = [moduleMeta];
-  return NewModule;
+const DYNAMIC_COMPONENT_SELECTOR = 'storybook-dynamic-component';


can you move it up?

igor-dv · 2018-01-10T09:15:29Z

app/angular/src/client/preview/angular/helpers.ts

+const DYNAMIC_COMPONENT_SELECTOR = 'storybook-dynamic-component';
+const createComponentFromTemplate = (
+  template: string,
+  params?: any[],


params are not used here

igor-dv · 2018-01-10T09:20:33Z

app/angular/src/client/preview/angular/helpers.ts

 import { FormsModule } from '@angular/forms';
 import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
 import { BrowserModule } from '@angular/platform-browser';
 import { AppComponent } from './components/app.component';
 import { ErrorComponent } from './components/error.component';
 import { NoPreviewComponent } from './components/no-preview.component';
 import { STORY } from './app.token';
-import { getAnnotations, getParameters, getPropMetadata } from './utils';


So, utils.ts can be deleted?

Signed-off-by: Carlos Vega <[email protected]>

…ybook into angular-template-support

alterx · 2018-01-10T14:09:04Z

Yup, checked in v4 :)

amcdnl · 2018-01-09T23:44:51Z

app/angular/src/client/preview/angular/helpers.ts

+  selector?: string
+): IComponent => {
+  const metadata = new Component({
+    selector: selector || DYNAMIC_COMPONENT_SELECTOR,


You don't have to have a selector.

amcdnl · 2018-01-10T14:12:23Z

app/angular/src/client/preview/angular/helpers.ts

+    params,
+    moduleMeta,
+    template,
+  } = getComponentMetadata(currentStory(context));

  if (!componentMeta) {


I'd add some errors like:

if (!template && (!component || typeof component !== 'function')) { throw new Error('No valid component provided'); } if (template && component) { throw new Error('Component and template are mutually exclusive.'); }

…ybook into angular-template-support

teebszet · 2019-02-28T06:41:39Z

Should there be documentation on this feature in https://storybook.js.org/basics/guide-angular/#write-your-stories?

shilman · 2019-02-28T08:47:49Z

@teebszet Any chance you can contribute some docs on this?

breadadams · 2020-01-31T03:55:39Z

Docs would be real handy, like Vue for example there are 2 ways of creating a story, whereas Angular shows the same twice.

Once I can wrap my head around this myself I'll be happy to update it 👍

alterx added 2 commits January 8, 2018 12:54

Adding support for Angular templates

38e6c50

Removed lodash

10ca7d1

Let's use JS for this, don't see a compelling reason to pull lodash. Also, I don't see it in the deps and it's probably working because most projects use it but it could potentially break if it's not there. Signed-off-by: Carlos Vega <[email protected]>

alterx added ci: do not merge angular labels Jan 8, 2018

alterx self-assigned this Jan 8, 2018

alterx added 3 commits January 8, 2018 13:41

Adding support for Angular templates

1703ea3

Removed lodash

336cdcd

Let's use JS for this, don't see a compelling reason to pull lodash. Also, I don't see it in the deps and it's probably working because most projects use it but it could potentially break if it's not there. Signed-off-by: Carlos Vega <[email protected]>

Merge branch 'angular-template-support' of github.com:storybooks/stor…

acc305f

…ybook into angular-template-support

amcdnl approved these changes Jan 8, 2018

View reviewed changes

alterx added 4 commits January 8, 2018 13:54

Adding support for Angular templates

bf25c43

Removed lodash

e5d44bd

Let's use JS for this, don't see a compelling reason to pull lodash. Also, I don't see it in the deps and it's probably working because most projects use it but it could potentially break if it's not there. Signed-off-by: Carlos Vega <[email protected]>

Removed lodash

cc708a1

Let's use JS for this, don't see a compelling reason to pull lodash. Also, I don't see it in the deps and it's probably working because most projects use it but it could potentially break if it's not there. Signed-off-by: Carlos Vega <[email protected]>

Merge branch 'angular-template-support' of github.com:storybooks/stor…

c627a3c

…ybook into angular-template-support

alterx added 5 commits January 8, 2018 15:22

Fixed tslint issues

59eadea

Signed-off-by: Carlos Vega <[email protected]>

Added initial support for multiple compoonents in templates

9503c45

Signed-off-by: Carlos Vega <[email protected]>

Fixed typo in metadata/knobs sample

fd3b613

Signed-off-by: Carlos Vega <[email protected]>

Merge branch 'master' of github.com:storybooks/storybook into angular…

083f634

…-template-support

Removed unused types

0050f2f

Signed-off-by: Carlos Vega <[email protected]>

alterx requested review from igor-dv and ralzinov January 10, 2018 01:22

alterx removed the ci: do not merge label Jan 10, 2018

Merge branch 'master' into angular-template-support

271ce3c

alterx mentioned this pull request Jan 10, 2018

Angular Feedback #2645

Closed

igor-dv reviewed Jan 10, 2018

View reviewed changes

alterx added 3 commits January 10, 2018 08:07

Removed unused files/code

b7e277f

Signed-off-by: Carlos Vega <[email protected]>

Merge branch 'angular-template-support' of github.com:storybooks/stor…

a97d9b6

…ybook into angular-template-support

Merge branch 'master' into angular-template-support

658e976

amcdnl reviewed Jan 10, 2018

View reviewed changes

alterx added 2 commits January 10, 2018 08:16

Remoived selector when generating a template-based component

1c6792f

Merge branch 'angular-template-support' of github.com:storybooks/stor…

590677f

…ybook into angular-template-support

alterx merged commit a1b9231 into master Jan 10, 2018

alterx deleted the angular-template-support branch January 10, 2018 14:38

Hypnosphi added the feature request label Jan 10, 2018

alterx mentioned this pull request Jan 11, 2018

[app/angular] Add ability to define component props binding in story. #2469

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Angular template support for Storybook #2690

Angular template support for Storybook #2690

alterx commented Jan 8, 2018 •

edited

Loading

amcdnl commented Jan 8, 2018

amcdnl left a comment

alterx commented Jan 8, 2018

codecov bot commented Jan 8, 2018 •

edited

Loading

igor-dv commented Jan 8, 2018 •

edited

Loading

alterx commented Jan 8, 2018

amcdnl commented Jan 8, 2018 •

edited

Loading

alterx commented Jan 8, 2018

alterx commented Jan 8, 2018

amcdnl commented Jan 8, 2018

alterx commented Jan 8, 2018 •

edited

Loading

alterx commented Jan 10, 2018 •

edited

Loading

igor-dv left a comment

igor-dv Jan 10, 2018

igor-dv Jan 10, 2018

igor-dv Jan 10, 2018

alterx commented Jan 10, 2018

amcdnl Jan 9, 2018

amcdnl Jan 10, 2018

teebszet commented Feb 28, 2019

shilman commented Feb 28, 2019

breadadams commented Jan 31, 2020

Angular template support for Storybook #2690

Angular template support for Storybook #2690

Conversation

alterx commented Jan 8, 2018 • edited Loading

What I did

How to test

amcdnl commented Jan 8, 2018

amcdnl left a comment

Choose a reason for hiding this comment

alterx commented Jan 8, 2018

codecov bot commented Jan 8, 2018 • edited Loading

Codecov Report

igor-dv commented Jan 8, 2018 • edited Loading

alterx commented Jan 8, 2018

amcdnl commented Jan 8, 2018 • edited Loading

alterx commented Jan 8, 2018

alterx commented Jan 8, 2018

amcdnl commented Jan 8, 2018

alterx commented Jan 8, 2018 • edited Loading

alterx commented Jan 10, 2018 • edited Loading

igor-dv left a comment

Choose a reason for hiding this comment

igor-dv Jan 10, 2018

Choose a reason for hiding this comment

igor-dv Jan 10, 2018

Choose a reason for hiding this comment

igor-dv Jan 10, 2018

Choose a reason for hiding this comment

alterx commented Jan 10, 2018

amcdnl Jan 9, 2018

Choose a reason for hiding this comment

amcdnl Jan 10, 2018

Choose a reason for hiding this comment

teebszet commented Feb 28, 2019

shilman commented Feb 28, 2019

breadadams commented Jan 31, 2020

alterx commented Jan 8, 2018 •

edited

Loading

codecov bot commented Jan 8, 2018 •

edited

Loading

igor-dv commented Jan 8, 2018 •

edited

Loading

amcdnl commented Jan 8, 2018 •

edited

Loading

alterx commented Jan 8, 2018 •

edited

Loading

alterx commented Jan 10, 2018 •

edited

Loading