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

Knobs-addon: Angular Components lose interactivity after adjusting property via knob #7242

Closed
rkara opened this issue Jul 1, 2019 · 22 comments

Comments

@rkara
Copy link

rkara commented Jul 1, 2019

Describe the bug
When adjusting properties on an Angular component via a Knob, the Angular component interactions start having issues.

To Reproduce
Steps to reproduce the behavior:

  1. Configure a story using the code snippet below
  2. Change a property using the knob add-on (change the first label to "error")
  3. Click the Second Tab in the preview pane
  4. Notice the tab content does not change
  5. Click the tab content
  6. Notice the tab switches

The example leverages @angular/material, but I came across this via proprietary components as well.

Expected behavior
Component should continue to work as expected after changing property via knob.

Code snippets

storiesOf('Tabs', module)
  .addDecorator(
    moduleMetadata({
      imports: [
        BrowserAnimationsModule,
        MatTabsModule,
        MatIconModule,
      ],
    }),
  )
  .add(
    'with knobs',
    () => ({
      template: `
      <mat-tab-group>
        <mat-tab *ngFor="let item of items">
          <ng-container *matTabLabel>
            <mat-icon>{{ item.icon }}</mat-icon>
            <span>{{ item.label }}</span>
          </ng-container>
          <ng-container *matTabContent>
            Content
          </ng-container>
        </mat-tab>
      </mat-tab-group>
    `,
      props: {
        items: object('items', [
          {
            label: 'first',
            icon: 'home',
          },
          {
            label: 'second',
            icon: 'face',
          },
          {
            label: 'third',
            icon: 'group_work',
          },
          {
            label: 'fourth',
            icon: 'grade',
          },
          {
            label: 'fifth',
            icon: 'feedback',
          },
        ], 'TABS-GROUP'),
      }
    }),
    { notes: 'My notes' }
  );

System:

  • OS: Windows 10
  • Browser: Chrome
  • Framework: Angular
  • Addons: @storybook/addon-knobs
  • Version: 5.1.9
@stale
Copy link

stale bot commented Jul 23, 2019

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!

@stale stale bot added the inactive label Jul 23, 2019
@sebastian-marinescu
Copy link

sebastian-marinescu commented Jul 31, 2019

We are using storybook with a custom component-library and just stumbled upon this issue.
Before we had 5.0.11 and I remember it working - now after updating to 5.1.9, this doesn't seem to work. We noticed it in a custom accordion-component, where after adjusting the title or content via a text-knob, the toggle-click doesn't work anymore.

@stale stale bot removed the inactive label Jul 31, 2019
@sebastian-marinescu
Copy link

I've also tested 5.1.10, and down to 5.1.5, than reverted back.
I can confirm, that downgrading to 5.0.11, makes it work again.

@stale
Copy link

stale bot commented Aug 21, 2019

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!

@stale stale bot added the inactive label Aug 21, 2019
@andrei-ilyukovich
Copy link

@sebastian-marinescu @rkara your storybook behavior most likely caused by fix of that issue #4098
Here is the fix which is part of 5.1.* version: #6094

@stale stale bot removed the inactive label Sep 3, 2019
@shilman shilman modified the milestones: 5.1.x, 5.2.x Sep 23, 2019
@stale
Copy link

stale bot commented Oct 14, 2019

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!

@stale stale bot added the inactive label Oct 14, 2019
@kroeder kroeder added the todo label Oct 14, 2019
@stale stale bot removed the inactive label Oct 14, 2019
@literalpie
Copy link
Contributor

literalpie commented Oct 30, 2019

I'm still seeing this issue in 5.2.5.

tried with 5.3.0-alpha.34, but the knob wasn't showing up at all! I think that's a different issue though...

edit: I'm only seeing this issue when using addon-docs. That might change things

@literalpie
Copy link
Contributor

I believe the problem involves code being run outside the angular zone, which causes change detection to not be run correctly.

In the function that makes changes that should be detected, I added NgZone.assertInAngularZone(). If the function gets called before a knob is changed, there isn't a problem. If I call the function after changing a knob, an error is thrown.

@shilman shilman modified the milestones: 5.2.x, 5.3.x Jan 11, 2020
@mgameover
Copy link

Hi everyone,
I believeI've faced a similar issue.

Steps to reproduce the behavior:

  1. Configure the story as shown in the code snippet
  2. Click on the button and verify that button's text is being updated
  3. Change "visible" property to false and back to true
  4. Click on the button
    Result: button's text is not updated.
storiesOf("COMPONENTS|button", module)
  .addDecorator(withKnobs)
  .add("basic", () => ({
    template: `
      <button *ngIf="visible" (click)="clickHandler($event);toggleState=!toggleState;">
        State: {{ toggleState }}
      </button>`,
    props: {
      visible: boolean("visible", true),     
      clickHandler: action("click event"),
    },
  }));

That happens with all components with *ngIf directive applied. It doesn't render any DOM changes made by "hidden & shown" components.

System:

  • OS: Windows 10
  • Browser: Chrome
  • Framework: angular 9.1.1
  • @storybook/angular": "^5.3.18"
  • @storybook/addon-actions": "^5.3.18"
  • @storybook/addon-knobs": "^5.3.18"

Any help is appreciated. Thanks.

@mgameover
Copy link

mgameover commented Apr 14, 2020

Just checked that with the official sample (storybook/examples/angular-cli v6.0.0-alpha.33).
The same thing, it stops working after changing visibility using *ngIf.

But it works with v5.0.11
So, I believe @andrei-ilyukovich is right about the root cause - comment.

@mgameover
Copy link

Hi everyone,

Is there any news there?
As I can see, the issue is still there. I've just checked v6.0.0-beta.6.
It looks pretty critical to me. Are there any plans to fix that?

Thanks.

@aloulouamine
Copy link

Hi,

Any workaround for this issue ?

Thanks.

@shilman
Copy link
Member

shilman commented May 28, 2020

Hi gang, We’ve just released addon-controls in 6.0-beta!

Controls are portable, auto-generated knobs that are intended to replace addon-knobs long term.

Please upgrade and try them out today. Thanks for your help and support getting this stable for release!

@shilman
Copy link
Member

shilman commented Jun 1, 2020

For anybody who is interested in Controls but don't know where to start, I've created a quick & dirty step-by-step walkthrough to go from a fresh CRA project to a working demo. Check it out:

=> Storybook Controls w/ CRA & TypeScript

There are also some "knobs to controls" migration docs in the Controls README:

=> How do I migrate from addon-knobs?

@mgameover
Copy link

Hi guys,
I can confirm that there is no such issue with @storybook/addon-controls.
But does it mean that there won't be any fix for v5?

@shilman
Copy link
Member

shilman commented Jun 3, 2020

@mgameover thanks for testing that--great to hear that it works! will accept PRs for fixes in knobs. SB6 going into RC soon. i'll be maintaining controls, and looking for knobs maintainers since we're not deprecating that yet

@Silverium
Copy link

Hi all,

I've found a workaround. Use functions!!
As you may know, React is all about functions and so are knobs, with this approach, you have instant responsiveness:

...
  .add('your story here', () => ({
    template: `
        <your-component-here
                [customHeight]="height()">
        </your-component-here>
    `,
    props: {
      height: () => number('height', 50)
    },
  }));

@shilman shilman removed this from the 5.3.x milestone Jul 30, 2020
@mgameover
Copy link

Hi all,
I've just checked @storybook/addon-knobs v6.0.1, the issue is still there.
BUT, the workaround suggested by @Silverium works like a charm.
Thanks!

@igorbaimos
Copy link

Hi there,
The workaround suggested by @Silverium indeed resolves the lost interactivity issue.
However, I can see now message arrived at manager constantly appearing in the console log.

@Marklb
Copy link
Member

Marklb commented Sep 4, 2020

@igorbaimos Is that console.log in your code, because I don't see anything in the workaround that would cause a console.log.


I started a PR(#12382) that fixes this problem without needing the workaround, but I am not positive that it is the right way to solve this, so if anyone knows how NgZone works fairly well, can you checkout the PR. I have a decent understanding of how it works, but not enough to be confident that I am not causing it to include to much in the Zone.

When I read @literalpie's comment(#7242 (comment)) about how a function was not being called inside an NgZone it gave me an idea what the problem was. By making sure the props are always run from inside the zone, it seems to fix the problem.

What I think is happening without the PR is the following:

  • Initially the story creates the component while bootstrapping, so it is still in the zone when it sets the props.
  • When an update to the story is triggered by an addon, such as a knob, the change is triggered from a Storybook channel, which is not in the zone. By setting component props outside the zone I think it is propagating the problem into the child component.
  • Now if an output, such as a click event, is re-initialized when the props are set outside the zone, Angular's changedetector will know nothing about the click, because the event wouldn't have been hooked by the zone.

Based on the steps I mentioned above, the code in #7242 (comment) wouldn't work after toggling the boolean knob. When the visible boolean becomes true, it re-initializes the button outside the zone. Since it isn't inside the zone when the click event is registered for the re-initialized button, Angular's change detector has no knowledge of it, because the addEventListener hooked by NgZone wouldn't be used.

If anything I said is wrong, let me know, because I am basing my PR on that logic.

@igorbaimos
Copy link

@Marklb
The console.log is not in my code. I see the messages message arrived at manager storybookjs/knobs/set written constantly to the console log (level: Verbose). And those are originating from here: return console.debug.apply(console, [message].concat(rest)); (see vendors~main.2e042f7df5eb4429be2b.bundle.js:209308).
I assume that this is happening because a new knob object is being created on every change detection cycle.

My knob is defined as follows props: { a_property: () => select() }.
The relevant stack trace is:

knob (KnobManager.js:164)
select (index.js:119)
my-component.stories.ts (knob declaration)
DynamicComponents.html (input property binding)
debugUpdateDirectives (Core.js:30043)

Note: if I don't use function as knob definition the console.log mentioned above is written only once.

@shilman
Copy link
Member

shilman commented Dec 1, 2020

Ermahgerd!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.2.0-alpha.0 containing PR #12382 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests