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

[Angular][Intro to Storybook] Outdated action addon API #648

Open
geromegrignon opened this issue Sep 22, 2022 · 3 comments
Open

[Angular][Intro to Storybook] Outdated action addon API #648

geromegrignon opened this issue Sep 22, 2022 · 3 comments

Comments

@geromegrignon
Copy link
Contributor

Description

The Angular Intro to Storybook action addon part (https://storybook.js.org/tutorials/intro-to-storybook/angular/en/simple-component/)is using old action() API :

export const actionsData = {
  onPinTask: action('onPinTask'),
  onArchiveTask: action('onArchiveTask'),
};

Solution

Use the new API as described in Essential Addons documentation (https://storybook.js.org/docs/react/essentials/actions):

argTypes: { onClick: { action: 'clicked' } },

It would be a good opportunity to update function naming to match Angular style guide as on prefix is not recommended as explained here: https://angular.io/guide/styleguide#style-05-16

Contribution

I'm willing to create a PR to address this topic if accepted.

@jonniebigodes
Copy link
Collaborator

@geromegrignon thanks for taking the time and effort to reach out to us with your issue. We appreciate it. The short answer is it could be updated to use it. But adding it would break the next part or lead to a similar pattern that is already used. To give you a bit more context on this, I've set up a small reproduction and going to describe what I did.

import { Component, Input, Output, EventEmitter } from '@angular/core';

import { Task } from '../models/task.model';

@Component({
  selector: 'app-task',
  template: `
    <div class="list-item {{ task?.state }}">
      <label
        [attr.aria-label]="'archiveTask-' + task.id"
        for="checked-{{ task?.id }}"
        class="checkbox"
      >
        <input
          type="checkbox"
          disabled="true"
          [defaultChecked]="task?.state === 'TASK_ARCHIVED'"
          name="checked-{{ task?.id }}"
          id="checked-{{ task?.id }}"
        />
        <span class="checkbox-custom" (click)="onArchive(task.id)"></span>
      </label>
      <label
        [attr.aria-label]="task.title + ''"
        for="title-{{ task?.id }}"
        class="title"
      >
        <input
          type="text"
          [value]="task.title"
          readonly="true"
          id="title-{{ task?.id }}"
          name="title-{{ task?.id }}"
          placeholder="Input title"
        />
      </label>
      <button
        *ngIf="task?.state !== 'TASK_ARCHIVED'"
        class="pin-button"
        [attr.aria-label]="'pinTask-' + task.id"
        (click)="onPin(task.id)"
      >
        <span class="icon-star"></span>
      </button>
    </div>
  `,
})
export class TaskComponent {
  @Input() task: Task;

  // tslint:disable-next-line: no-output-on-prefix
  @Output()
  pinTask = new EventEmitter<Event>();

  // tslint:disable-next-line: no-output-on-prefix
  @Output()
  archiveTask = new EventEmitter<Event>();

  /**
   * Component method to trigger the onPin event
   * @ignore
   * @param id string
   */
  onPin(id: any) {
    this.pinTask.emit(id);
  }
  /**
   *
   * Component method to trigger the onArchive event
   * @ignore
   * @param id string
   */
  onArchive(id: any) {
    this.archiveTask.emit(id);
  }
}

As you can see the events were renamed in order to address the Angular style guide you've mentioned and even added the ignore comment to avoid Storybook from picking up the event handler and only exposing the event itself.

  • Created the associated story as follows:
import { Meta, Story } from '@storybook/angular';

import { TaskComponent } from './task.component';

export default {
  component: TaskComponent,
  title: 'NEW API/Task',
  argTypes: {
    pinTask: {
      action: 'pinTask',
    },
    archiveTask: {
      action: 'archiveTask',
    },
  },
} as Meta;

const Template: Story = (args) => ({
  props: args,
});

export const Default = Template.bind({});
Default.args = {
  task: {
    id: '1',
    title: 'Test Task',
    state: 'TASK_INBOX',
  },
};

export const Pinned = Template.bind({});
Pinned.args = {
  task: {
    ...Default.args['task'],
    state: 'TASK_PINNED',
  },
};

export const Archived = Template.bind({});
Archived.args = {
  task: {
    ...Default.args['task'],
    state: 'TASK_ARCHIVED',
  },
};
  • Started Storybook with ng run taskbox:storybook and got the following result:
simple-task-angular-new-api.mp4
  • Followed onto the next section of the tutorial (i.e., the Composite Component ) and updated the component implementation to the following:
import { Component, Input, Output, EventEmitter } from '@angular/core';
import { Task } from '../models/task.model';

@Component({
  selector: 'app-task-list',
  template: `
    <div class="list-items">
      <app-task
        *ngFor="let task of tasksInOrder"
        [task]="task"
        (archiveTask)="archiveTaskList.emit($event)"
        (pinTask)="pinTaskList.emit($event)"
      >
      </app-task>
      <div
        *ngIf="tasksInOrder.length === 0 && !loading"
        class="wrapper-message"
      >
        <span class="icon-check"></span>
        <p class="title-message">You have no tasks</p>
        <p class="subtitle-message">Sit back and relax</p>
      </div>
      <div *ngIf="loading">
        <div *ngFor="let i of [1, 2, 3, 4, 5, 6]" class="loading-item">
          <span class="glow-checkbox"></span>
          <span class="glow-text">
            <span>Loading</span> <span>cool</span> <span>state</span>
          </span>
        </div>
      </div>
    </div>
  `,
})
export class TaskListComponent {
  /**
   * @ignore
   * Component property to define ordering of tasks
   */
  tasksInOrder: Task[] = [];

  @Input() loading = false;

  // tslint:disable-next-line: no-output-on-prefix
  @Output() pinTaskList: EventEmitter<any> = new EventEmitter();

  // tslint:disable-next-line: no-output-on-prefix
  @Output() archiveTaskList: EventEmitter<any> = new EventEmitter();

  @Input()
  set tasks(arr: Task[]) {
    const initialTasks = [
      ...arr.filter((t) => t.state === 'TASK_PINNED'),
      ...arr.filter((t) => t.state !== 'TASK_PINNED'),
    ];
    const filteredTasks = initialTasks.filter(
      (t) => t.state === 'TASK_INBOX' || t.state === 'TASK_PINNED'
    );
    this.tasksInOrder = filteredTasks.filter(
      (t) => t.state === 'TASK_INBOX' || t.state === 'TASK_PINNED'
    );
  }
}

As you can see by the code snippet the component was tweaked to follow the Angular style guide once again.

  • Created the story for this component as follows:
import {
  componentWrapperDecorator,
  moduleMetadata,
  Meta,
  Story,
} from '@storybook/angular';

import { CommonModule } from '@angular/common';

import { TaskListComponent } from './task-list.component';
import { TaskComponent } from './task.component';

import * as TaskStories from './task.stories';

export default {
  component: TaskListComponent,
  decorators: [
    moduleMetadata({
      //👇 Imports both components to allow component composition with Storybook
      declarations: [TaskListComponent, TaskComponent],
      imports: [CommonModule],
    }),
    //👇 Wraps our stories with a decorator
    componentWrapperDecorator(
      (story) => `<div style="margin: 3em">${story}</div>`
    ),
  ],
  title: 'NEW API/TaskList',
  argTypes: {
    pinTaskList: {
      action: 'pinTask',
    },
    archiveTaskList: {
      action: 'archiveTask',
    },
  },
} as Meta;

const Template: Story = (args) => ({
  props: args,
});

export const Default = Template.bind({});
Default.args = {
  tasks: [
    { ...TaskStories.Default.args?.['task'], id: '1', title: 'Task 1' },
    { ...TaskStories.Default.args?.['task'], id: '2', title: 'Task 2' },
    { ...TaskStories.Default.args?.['task'], id: '3', title: 'Task 3' },
    { ...TaskStories.Default.args?.['task'], id: '4', title: 'Task 4' },
    { ...TaskStories.Default.args?.['task'], id: '5', title: 'Task 5' },
    { ...TaskStories.Default.args?.['task'], id: '6', title: 'Task 6' },
  ],
};

export const WithPinnedTasks = Template.bind({});
WithPinnedTasks.args = {
  // Shaping the stories through args composition.
  // Inherited data coming from the Default story.
  tasks: [
    ...Default.args['tasks'].slice(0, 5),
    { id: '6', title: 'Task 6 (pinned)', state: 'TASK_PINNED' },
  ],
};

export const Loading = Template.bind({});
Loading.args = {
  tasks: [],
  loading: true,
};

export const Empty = Template.bind({});
Empty.args = {
  // Shaping the stories through args composition.
  // Inherited data coming from the Loading story.
  ...Loading.args,
  loading: false,
};
  • Restarted Storybook, and I'm presented with the following:
tasklist-new-api.mp4

As you can see, there's now an issue with the story as we're composing components, and the TaskList component requires an action incoming from Task. I tried a couple of things to see if it would work, but none panned out. Which lead me back to the starting point of passing in an action similar to what's already in play which kinda defeats the purpose.

I'm aware the API in use is outdated. To avoid adding entropy and confusion to our readers, as this is an introductory tutorial to Storybook, I went with this approach. As we're defining the action once and importing it where it's required. Reducing the amount of code necessary while preserving the necessary information.

With this, it doesn't mean that part of the issue cannot be addressed. At least we can update the component's code to address and follow the Angular style guide that you mentioned.

For reference, I've set up a small reproduction here if you're interested in taking a look at it.

Also, before moving on, if you know another way of addressing the issue, I'm more than glad to follow up with you and see how it can be added to the tutorial.

Looking forward to hearing from you.

Hope you have a great weekend.

Stay safe

@geromegrignon
Copy link
Contributor Author

Hi @jonniebigodes, thanks for the complete feedback, and sorry for the delay.
I made some attempts based on your explanation but couldn't find a solution either.

I personally don't use action addon that much but opened this issue as someone on the Storybook Discord server was facing such a problem with it.
I really appreciate such feedback.

Hope you have a great weekend too.

@do-diegoortiz
Copy link

do-diegoortiz commented May 30, 2023

Hey @jonniebigodes this issue was bugging me, so I found the solution in your small reproduction. It seems the story doesn't get when we emit in the same template, so we have to just call a method and emit from the method, like this:

(archiveTask)="onArchiveTaskList($event)"
(pinTask)="onPinTaskList($event)"

And then adding those two methods like this:

onPinTaskList(taskId: string) {
  this.pinTaskList.emit(taskId);
}

onArchiveTaskList(taskId: string) {
  this.archiveTaskList.emit(taskId);
}

I couldn't create a PR to your repo, but you can try yourself. It would be great to update the docs on this, because Storybook is already in version 7 and Angular in 16. So using these old patterns might cause issues later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants