Simpler/Declarative Interface for Launching Modals

[gregjacobs]

(Copying/pasting from email :))

Hey Pawel, Jesus,

Nice to meet you guys! And thanks for the initial info.

So one thing that developers have come to me with is trouble using the modals in ng-bootstrap. The imperative open() method, needing to pass a reference to an <ng-template>, and responding to a 'result' promise turns out to be a bit of a complex interface. And since open() needs to be called imperatively, it also requires some extra plumbing if we want to hook up a modal to an ngrx/store's state.

What I'd like to propose is a simpler declarative interface to the modal. Something along the lines of this:

<ngb-modal [visible]="visible">
    My modal content goes here
</ngb-modal>

This is actually how we designed our modal component at my last company (we had an internal component library), and it worked really well. We also had some other inputs and events to listen to as well, where you could allow all of the current ngbModal API as attributes on the tag. Here is more of a full example:

<ngb-modal 
    [(visible)]="visible"
    [keyboard]="false"
    backdropClass="my-backdrop-class"
    windowClass="my-window-class"
    (showBegin)="onShowBegin()"  
    (show)="onShow()"
    (hideBegin)="onHideBegin()"
    (hide)="onHide()" 
    (dismiss)="onDismiss()"
>
    My modal content goes here

    <button (click)="submit()">Submit</button>
    <button (click)="visible = false">Cancel</button> 
</ngb-modal>

Just a couple notes on the above:

1. (showBegin) and (hideBegin) would to respond to the start of modal animation when showing/hiding
2. (dismiss) would be to respond specifically to the modal being closed by the 'esc' key, or if the modal allows itself to be closed by clicking the backdrop

What do you guys think?

Best,
Greg

[pkozlowski-opensource]

@gregjacobs this definitively makes sense and I can see how this simplify use-cases where we just want to show a modal from a template (probably majority of cases). To move it forward we would need more detailed design - the first thing that comes to my mind is that it can't be a component but rather a directive sitting on <ng-template>:

<ng-template [(ngbModal)]="visibleExp">
   My modal content goes here
</ng-template> 

instead of:

<ngb-modal [(visible)]="visible">
   My modal content goes here
</ngb-modal>

The reason for this is that we don't want to instantiate / change detect content of a modal if it is not shown.

If we go with the <ng-template [(ngbModal)]="visibleExp"> API the whole thing would be simple to implement on top of the existing NgbModal service, here is a POC: https://stackblitz.com/edit/angular-84tjbu?file=app/ngb-modal-directive.ts

Unfortunately it seems like we are hitting angular/angular#15634 (I was aware of this one for quite some time). We could obviously work-around it but I would rather work towards fixing this issue in Angular, as this is quite a common scenario.

Ao, it looks like for now we are a bit blocked on Angular core, but I will work towards resolving this issue. It might take a bit of time though and probably will be fixed only in Ivy...

[gregjacobs]

Hey Pawel,

Yeah I hear you on change detecting the modal content even when the modal isn't visible if we go the component route. We actually ran into this same issue with our internal modal component, which was (oddly enough) easy to solve in AngularJS with how transclusion worked, but not as much when we updated it for Angular 2+.

However, I wonder how important it is to worry about change detecting the modal's content even when it's not shown. Unless there are a lot of bindings in that modal, there's really not going to be any noticeable performance difference. The outer page itself will most likely have many more bindings than the modal itself, unless the modal is showing grid data or something of the sort.

Perhaps a workaround for this particular issue though could be something like the following for users that may want to optimize this, which we could put in the examples:

<ngb-modal [(visible)]="visible">
    <my-modal-content-component *ngIf="visible"></my-modal-content-component>
</ngb-modal>

It's not 100% ideal, but I still think the <ngb-modal> component would simplify the interface greatly and would solve probably 95% of use cases. And users could either use this *ngIf workaround or fall back to the imperative interface if need be.

What do you think?

[benouat]

The reason for this is that we don't want to instantiate / change detect content of a modal if it is not shown. (@pkozlowski-opensource)

I still think the component would simplify the interface greatly (@gregjacobs)

What about trying to mix both ideas ?

<ngb-modal *isVisible="visible"></ngb-modal>

[gregjacobs]

@benouat Well that's just a very good idea :)

The only problem with that might be that ngb-modal wouldn't be able to reset the flag back to false if the modal was closed by the user hitting the 'esc' key since there's no two-way binding. The developer would need to handle that manually so that the modal could be re-opened again. Maybe not too big of a deal?

[pkozlowski-opensource]

However, I wonder how important it is to worry about change detecting the modal's content even when it's not shown. Unless there are a lot of bindings in that modal, there's really not going to be any noticeable performance difference. The outer page itself will most likely have many more bindings than the modal itself, unless the modal is showing grid data or something of the sort.

@gregjacobs I'm afraid that I've reviewed too many applications where it was well noticeable performance issue.

The good news is that I think I've got a work-around for the Angular issue (proper fix is several weeks / months away...). I will try this in the coming days.