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

onActionCallback feature request #27

Closed
friksa opened this issue Jun 4, 2016 · 10 comments
Closed

onActionCallback feature request #27

friksa opened this issue Jun 4, 2016 · 10 comments

Comments

@friksa
Copy link

friksa commented Jun 4, 2016

We are using this project as part of a web conferencing product. When you are busy teaching and someone makes a request to open their microphone, a modal popup message is very disruptive. Also, you may not want to open their microphone immediately... you may want to finish a thought before accepting their request.

Your toaster popup would be ideal if you could add an action button with a callback similar to onHideCallback. If the presenter clicks on the action button, then onActionCallback is fired and the microphone is opened. If the toast is dismissed, the request is ignored.

The API could be something like this:

var toast : Toast = {
    type: 'info',
    title: 'Mic Request',
    body: 'Stabzs wants to speak.  Open Mic?',
    showCloseButton: true,
    actionButton: 'Yes',
    onHideCallback: function() { console.log('no'); },
    onActionCallback: function() { console.log('yes'); }
};

this.toasterService.pop(toast);

We do not need more than 1 action, but you may want to consider it from the start and rather use this kind of an API:

var toast : Toast = {
    type: 'info',
    title: 'Mic Request',
    body: 'Stabzs wants to speak.  Open Mic?',
    showCloseButton: true,
    actions: [
      {text: 'Yes', callback: function() { console.log('yes'); } },
      {text: 'Ignore', callback: function() { console.log('ignored'); } }
    ],
    onHideCallback: function() { console.log('no'); }
};

this.toasterService.pop(toast);
@Stabzs
Copy link
Owner

Stabzs commented Jun 4, 2016

This is certainly a very desirable type of behavior, but I do not believe that additional API expansion is needed.

Such behavior can be added via bodyOutputType: Component. This body type is specifically designed to allow you to render components as the body of the toast.

Why is this more desirable than what you've described?

  • It keeps the API clean..less parameters means less needed documentation and less confusion!
  • Fewer surfaces that need testing within the library itself. That means more stability.
  • Proper separation of concerns. These types of actions probably shouldn't be buried within the library. How is the library to deal with checkboxes next? Or select boxes? Removing the burden of managing these callbacks from the library and placing them in the injected component means that each use case can be tested and maintained independently (and reused) by the calling application.

Unfortunately, there was a display bug in the toast icons that caused the icons to overlay the entire body. This has been corrected by commit 14f1286.

In addition, i have added an additional use case to the https://github.com/Stabzs/Angular2-Toaster/tree/master/demo/systemjs demo to demonstrate how to use this type of component injection. True to the use case you describe above, it allows for state toggling and that state is outputted to the UI. Please review it and see if it meets your needs.

The library has been updated to version 0.3.4-rc.1 in light of this change. In addition, the systemjs demo has also been updated to 0.3.4-rc.1. Please see the https://github.com/Stabzs/Angular2-Toaster/blob/master/CHANGELOG.md for additional details.

@friksa
Copy link
Author

friksa commented Jun 5, 2016

Completely agree with your logic. This almost works for me... I am just missing a small piece of know-how.

This is my class defined in a separate file:

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

@Component({
  selector: 'gw-requestmic',
  template: `
        <div>Allow {{name}} to speak?</div>
        <div>
            <button (click)="onMicOpen.emit([uuid, true])">Open</button>
            <button (click)="onMicOpen.emit([uuid, false])">Close</button>
        </div>`
})
export class GwRequestMic {
  @Input() name: string;
  @Input() uuid: string;

  @Output() onMicOpen: EventEmitter = new EventEmitter();
}

How do I pass the input properties and listen to the events?

    var toast: Toast = {
      type: 'info',
      title: 'Mic Request',
      showCloseButton: true,
      body: GwRequestMic,
      toastContainerId: 1,
      bodyOutputType: BodyOutputType.Component
    }
    this.toasterService.pop(toast);

It seems like it would be nice to support BodyOutputType.Template as a bodyOutputType:

    var toast: Toast = {
      type: 'info',
      title: 'Mic Request',
      showCloseButton: true,
      body: `
        <gw-requestmic
            [name]="Stabzs"
            [uuid]="XXX"
            (onMicOpen)="onMicOpen($event)"
        >
        </gw-requestmic>`,
      toastContainerId: 1,
      bodyOutputType: BodyOutputType.Template
    }
    this.toasterService.pop(toast);

@Stabzs
Copy link
Owner

Stabzs commented Jun 6, 2016

I've been spending some time thinking about how to handle this.

I could be completely wrong, but my understanding of Angular 2 is that what you've described as a "Template" option is pretty much impossible. There's no such thing as an arbitrary $compile service anymore which makes rendering any directive from a string impossible. The ComponentResolver service doesn't offer that type of flexibility.

I am currently exploring alternative ways to bind inputs and outputs to the rendered component. Keep in mind that this is more logic than was ever intended to live in a toast, but there still may be a reasonable way to handle it. You might be forced to use injected services to control your data however.

I'll continue to look into ways to solve this more gracefully.

@friksa
Copy link
Author

friksa commented Jun 6, 2016

I agree that this is putting toasts on steroids and it is a great opportunity to reduce the use of modal alert popups.

Does something like this help?
http://stackoverflow.com/questions/34635269/how-to-pass-input-params-to-an-angular-2-component-created-with-dynamiccompon

This project might hold some clues too:
https://github.com/dougludlow/ng2-bs3-modal

Otherwise, the original actions idea seems to be a viable alternative?

Thanks again for a great project!

@Stabzs
Copy link
Owner

Stabzs commented Jun 7, 2016

Thanks for both links.

The case you're describing is somewhat unique, since you want the ability to control inputs and outputs. In the first example, this only works if you have the same inputs/outputs to create every time, based on the same parameters.

The second doesn't seem to be passing data via any sort of input/output context. So I don't think that's a good strategy.

The original actions proposal still has the same problems I outline above. The largest issue of all is of course that there's no way to support future action expansion as needed since the responsibility lies in the wrong place.

A possible solution would be a function callback that is used to register data. Another option might be a service that you inject into your component. I'll continue to play with the concept.

@friksa
Copy link
Author

friksa commented Jun 17, 2016

How about adding a free-form data object? Something like this:

var t = {
  type: 'info',
  title: title,
  tapToDismiss: false,
  showCloseButton: false,
  timeout: 0,
  body: GwRequestMicComponent,
  data: {
    name: 'Stabzs',
    uuid: 'xxx',
    onAction: eventEmmitter
  }
  toastContainerId: 2,
  bodyOutputType: BodyOutputType.Component
};
this.toasterService.pop(t);

Then the component could be something like this:

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

@Component({
  selector: 'gw-request-mic',
  template: `
        <div>Allow {{data.name}} to speak?</div>
        <div>
            <button (click)="onAction(true)">Yes</button>
            <button (click)="onAction(false)">No</button>
        </div>`
})
export class GwRequestMicComponent {
  @Input() data;

  onAction(b:boolean) {
    this.data.onAction.emit(this.data.uuid, b); //or just call a service directly to take action
  }
}

@Stabzs
Copy link
Owner

Stabzs commented Jun 17, 2016

In this case how would the data know to register the action emitter? In addition, is there a large difference between this and a shared service?

@friksa
Copy link
Author

friksa commented Jun 17, 2016

You would register the listeners before passing it into the data structure. A shared service is cleaner for most situations though.

The real challenge is getting dynamic data to the component. I see that you updated the example TestComponent4 to take dynamic data... but I am not sure where you actually provide the data based on different users making the request. That will totally solve the issue. :)

@Stabzs
Copy link
Owner

Stabzs commented Nov 21, 2016

@friksa apologies for letting this sit so long.

After letting the Angular 2 community and changes bake in, it still seems like the best way to handle this would be a shared service. I'm open to any new insight if you have it. If not, I don't believe that this is something that can/should be enhanced within the library and instead should be handled via services.

@Stabzs
Copy link
Owner

Stabzs commented Dec 3, 2016

Changes to Component bodies in 1.1.0 allow for the toast instance to be passed to the rendered component. This allows for shared services to interact with the toast (see this plunker). I believe this is sufficient enough for most cases to handle these scenarios without needing to resort to manually registering events with the toast instance itself, since that is a compile-time constant.

@Stabzs Stabzs closed this as completed Dec 3, 2016
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

2 participants