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

Modal: string argument to the open method is confusing #727

Closed
pkozlowski-opensource opened this issue Sep 13, 2016 · 5 comments
Closed

Comments

@pkozlowski-opensource
Copy link
Member

See http://stackoverflow.com/q/39464345/1418796

Really, the string argument is only good for unit-testing but not for any real-life applications. We should remove it altogether or at least clarify in the docs.

On top of this we need to support components as arguments to the open method, see #680

@felixfbecker
Copy link

How about something like

ngbModal.open(MyModalComponent).then(value => ...

where the component is passed the modal instance as an @input?

to avoid having to specify @Input it could also be possible to subclass a ModalComponent:

import {ModalComponent} from '@ng-bootstrap/ng-bootstrap'

@Component({})
class MyModalComponent extends ModalComponent {
  ngOnInit() {
    this.modal.close(value) // or
    this.modal.dismiss(value)
  }
}

where ModalComponent is something like:

abstract class MyModalComponent {
  @Input()
  public modal: ModalInstance
}

@felixfbecker
Copy link

Actually it would be very ng1-ish to pass the modal instance as an input. Better would be to have two outputs, close and dismiss.

@felixfbecker
Copy link

This here does something similar: http://plnkr.co/edit/ZAZqZu?p=preview

@pkozlowski-opensource pkozlowski-opensource modified the milestones: alpha.7, beta.0 (Feature-complete) Sep 23, 2016
@SanderElias
Copy link

My vote would go on removing the string argument completely, as it's just utterly confusing.
Adding the component support to the open method makes a lot of sense. This will take out quite some boileplate code that is now needed to support the usecase of #680

@pkozlowski-opensource
Copy link
Member Author

For anyone interested in this issue, here is the design doc for introducing components as arguments to the open method: https://docs.google.com/document/d/1puwnSknaW3bLg_Ww2ewuILneCom1wbMTArvfFq0YFV8/edit?usp=sharing

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

3 participants