$modal.open compatibility with component (angular.component method)

[zdychacek]

Hello,

I want to ask if you're considering support of angular.component method which is new API for creating components.

I'd like to have the ability to call $modal.open method with the component definition. Now I have to specify controller and template options...

My API proposal:

angular.module('myApp').component('myModal', {
  bindings: {
    greeting: '<'
  },
  template: '<div>{{$ctrl.greeting}}</div>',
  controller: function () {},
})

// open the modal
$modal.open({
  component: 'myModal',
  bindings: {
    greeting: 'hello',
  },
});

Thanks for response.

O.

[icfantv]

I believe this would force us to only support Angular 1.5+ and i'm not sure we're ready to go there yet.

[zdychacek]

@icfantv Is there any issue with coexistence of both variants? Could't proposed API coexist with current one?

[icfantv]

@zdychacek, i don't know. this would require investigation if this is, in fact, something we want to support. but we need to have the discussion first.

[wesleycho]

I'm not against this, but we'll have to determine at what stage we want to implement this. We could implement it now, and have code where this only applies to angular 1.5+, but that adds bloat for those using 1.4.

[Olgagr]

I'm trying to use existing modal with component this way:

modalInstance = this.$uibModal.open({
      template: '<my-component message="{{$ctrl.message}}"></my-component>',
      size: 'sm',
      controllerAs: '$ctrl',
      controller: () => {
        this.message = 'some message';
    });

Then in my component:

let myComponent = {
  bindings: {
    message: '@'
  }
};

However, it doesn't work :( this.message in component controller is empty. I'm not sure why.

----- edit

I've just found the problem -> arrow function in controller! :)

To make it work:

modalInstance = this.$uibModal.open({
      template: '<my-component message="{{$ctrl.message}}"></my-component>',
      size: 'sm',
      controllerAs: '$ctrl',
      controller: function() {
        this.message = 'some message';
    });

[icfantv]

@Olgagr there should be nothing wrong with having an arrow function if your transpiler is working correctly. Arrow functions are simply rewritten as you have defined above. I would encourage you to look at your transpiled code with the arrow function and see what it's generating.

[Olgagr]

@icfantv I think in this case it makes difference because $uibModal.open is run in controller class of other component. To show wider context:

class PublicationController {

    openModal() {
        modalInstance = this.$uibModal.open({
         template: '<my-component message="{{$ctrl.message}}"></my-component>',
         size: 'sm',
         controllerAs: '$ctrl',
         controller: function() {
            this.message = 'some message';
         });
    }
}

Can you see the problem here ? If I use lambda, this will be PublicationController instance.

[icfantv]

@Olgagr, i don't follow, i use arrow functions inside my ES6 classes all the time. the only thing I don't do is specify a function for my controller property - but the transpiler should be converting them all anyway.

[Olgagr]

@icfantv And this is a difference :) Arrow functions are not compiled to the same code as "normal" functions. See the difference: http://bit.ly/1S41ZuP

[icfantv]

ah, ok. good to know. thanks.

[dfsq]

@Olgagr You can't use arrow function in this case because arrow functions have lexical scope which means that this points to something else (global object or undefined in strict mode), so this is not a controller instance. In this case normal function should be used.

[dfsq]

I'm not sure this proposal has much sense or even necessary. The example @zdychacek gives can already be expressed with existing syntax:

// Proposal
$modal.open({
  component: 'myModal',
  bindings: {
    greeting: 'hello',
  },
});

// Can currently be implemented
$modal.open({
  template: '<my-modal greeting="$ctrl.greeting"></my-modal>',
  controller: function() {
    this.greeting = 'hello'
  },
  controllerAs: '$ctrl'
})

Existing syntax is very flexible and can already be used with components. At the same time, proposal means introducing two more config properties (component and bindings), which are redundant (again, because you can achieve the goal with existing ones). It is also a little confusing: it's not obvious that bindings will get merged (?) with component bindings and how this merge is going to happen. What happens if one provides both component and controller and template?

So there are questions that are not very obvious.

[Olgagr]

@dfsq Yes, I know about it (arrow functions). We've just discussed this above :)

[Olgagr]

I agree with @dfsq . I also use the existing syntax with components and so far it works pretty well.

[zdychacek]

@dfsq Hi, I would like to know, how do you access $modaInstance service from within the component's controller?

I can't see any other solution than passing it explicitly via component's bindings from the controller.

$modal.open({
  template: '<my-modal greeting="$ctrl.greeting" modal-instance="$ctrl.modalInstance"></my-modal>',
  controller: [ '$modalInstance', function ($modalInstance) {
    this.greeting = 'hello';
    this.modalInstance = $modalInstance;
  } ],
  controllerAs: '$ctrl',
});

[Olgagr]

@zdychacek Do you need the whole modal instance ? For example, to have access to close or dismiss methods you can also pass just them as bindings:

$modal.open({
  template: '<my-modal greeting="$ctrl.greeting" close="$ctrl.close()" dismiss="$ctrl.close()"></my-modal>',
  controller: [ '$modalInstance', function ($modalInstance) {
    this.greeting = 'hello';
    this.close = $modalInstance.close;
    this.dismiss = $modalInstance.dismiss;
  } ],
  controllerAs: '$ctrl',
});

and then in your component bindings:

{
   close: '&',
   dismiss: '&'
}

[zdychacek]

@Olgagr No I don't need it whole. I've only tried to model the behaviour as similar as possible to the current one and that's to have an option to inject $modalInstance service into the component's controller directly.

But I can live with the solution mentionted above :)

[Olgagr]

On thing that is surprising to me: bindings are not updated. Let's say I open modal like this:

class SomeComponentController {

    constructor() 
       this.pages = this.getPages(); 
    }

    open() {
         var _this = this;
         $modal.open({
             template: '<my-modal pages="$ctrl.pages""></my-modal>',
             controller: function () {
                   this.pages = _this.pages;
           }],
           controllerAs: '$ctrl',
        });
    }


}

my-modal component has bindings:

bindings: {
   pages: '<'
}

Now, at some point, when modal is already opened, inside SomeComponentController this.pages changes. However, inside my-modal component this pages binding is not refreshed ($onChanges method is not called). This is surprising. It should be live binding after all, right ?

[dfsq]

@Olgagr This should not happen, most likely there is something with the way you are updating SomeComponentController's pages.

Here is a quick demo: http://plnkr.co/edit/XeqlnwjePvSTn0pXPaPz?p=preview

[Olgagr]

@dfsq Thanks for demo! Yes, you're right. The bindings should be updated.

[Olgagr]

@dfsq I was able to reproduce the issue. Here is modified your plunker example:

http://plnkr.co/edit/Jq6fwT?p=preview

Instead of pushing to array, I'm changing reference. As you can see the modal binding is not updated any more. Also $onChanges hook is not fired(it is only fired once, at the beginning but not after update).

When you do the same but with a regular component the binding is updated. Also $onChanges hook is fired: https://plnkr.co/edit/Rf82S9fCvA35R1kBMAc5?p=preview

I'm trying to understand why this is happening.

[dietergeerts]

@Olgagr , you use one-way binding '<'. For your controller to see the updated reference, you need to use two-way binding: '='.

[Olgagr]

@dietergeerts Hmm, why ? One way binding is updated from parent to child.

---

Anyway, I've changed one-way binding to two-way binding but reference is still not updated: http://plnkr.co/edit/aQa0YU?p=preview

[dietergeerts]

@Olgagr, I think I misunderstood you then, I thought the change wasn't going back to the parent.

[Dashue]

I went around it like this, keeps the component ignorant of being shown in a dialog.

adminApp.component('componentModal',
{
    bindings: {
        title: "@",
        component: "@"
    },
    transclude: true,
    template: "<a ng-click='$ctrl.open()' ng-transclude></a>",
    controller($uibModal: angular.ui.bootstrap.IModalService) {
        var title = this.title;
        var component = this.component;

        this.open = () => {
            $uibModal.open({
                template: [
                    '<div class="modal-content">',
                    '   <div class="modal-header">',
                    '       <a class="close" ng-click="$dismiss()">×</a>',
                    '       <h4 class="modal-title">'+ title +'</h4>',
                    '   </div>',
                    '   <div class="modal-body" style="overflow: auto;">',
                    '       <' + component + '></' + component + '>',
                    '   </div>',
                    '</div>'
                ].join(''),
                backdrop: 'static',
                keyboard: true,
                size: 'l',
            });
        }
    }
});

<component-modal title="My Title" component="my-component">
    <i class="fa fa-plus" tooltip="Details"></i>
</component-modal>

[jeserkin]

@wesleycho so this will be done and it should work with ng1.4.x with directives of type 'E', correct?

[wesleycho]

Yes.

[jeserkin]

@wesleycho will this change be propagated from say version 0.14.3 or will it be available only in latest library vesion?

[icfantv]

@jeserkin, we do not back port changes. We only release new, latest versions. We simply don't have the manpower as volunteers to support multiple versions.

[Olgagr]

Does @zdychacek API proposal is the way we want to implement this ?

[jeserkin]

@Olgagr as far as I can tell, it can't be just passed into template. Component/Directive of type element must be specified. I would assume, that $uibModalInstance can be passed same way into component controller as into directly specified controller. But I reviewed this library code long ago, since then it changed much.

Since I'm lately more in favor of component based approach, then I'm not sure, how good would it be to leave both approaches available, seems enforcing newer supported way is more correct.

[wesleycho]

Here is the API signature to be implemented - it will be used like

$modal.open({
  component: 'myComponent'
});

where myComponent is defined either via module.component or module.directive with restrict: 'E' (or at the least, allowing it to be used as an element directive). This component will automatically be set with the attribute $resolve and have value $resolve, which is an object containing all of the resolved from the resolve object passed into $modal.open, as mentioned in the (documentation](http://angular-ui.github.io/bootstrap/#/modal).

One will be able to use $resolve like so:

module.component('myComponent', {
  controller: 'MyController',
  bindings: {
    $resolve: '<'
  }
});

If this sounds sensible, I will look to implement this within the next few weeks (or someone else can implement it).

[Olgagr]

@wesleycho What about component lifecycle and & bindings ? Will it work with this solution ?

[wesleycho]

@Olgagr I was considering that - my assumption is that component users will eschew using controller, controllerAs, etc. from the $modal API in favor of moving that into the component rendered, and using the resolve data to construct everything desired.

Can you give an example as to how you were thinking of using this?

[Olgagr]

@wesleycho Yes, I can give you very specific example, because I have an Angular application that uses almost only components. I have a WYSIWYG editor, where user can put image. When he clicks image the modal window is shown, where user can crop image. The definition of this cropping component (that I would like to load in modal window) looks like this:

import template from './image_editor.html';
import controller from './image_editor.controller';
import './image_editor.scss';

let imageEditorComponent = {
  bindings: {
    onSave: '&',
    image: '<',
    savingErrors: '<'
  },
  template,
  controller
};

export default imageEditorComponent;

As you can see, this component is not responsible for saving changes on image. It should delegate this to parent component. On the other hand, parent component should notify this component, if there are any errors when saving cropped image.

Right now, I'm calling this component from parent this way:

var _this = this;

this.modalInstance = this.$uibModal.open({
      template: `
        <image-editor
          on-save="$ctrl.save(dataURL, imageDimentions)"
          image="$ctrl.image"
          saving-errors="$ctrl.savingErrors">
        </image-editor>
      `,
      controllerAs: '$ctrl',
      controller: function ImageEditorController() {
        this.save = (dataURL, imageDimentions) => {//do something};
        this.image = _this.image;
        this.savingErrors = _this.errors;
      }
    });
  }

And this is working, except that savingErrors is never updated ($onChanges is not fired).

[wesleycho]

Thank you for this example.

This use case would demand the library not make the assumption - I'm ok with that. We will need to support a bindings object then, which will look like this:

$uibModal.open({
  component: 'myComponent',
  controller: 'MyModalController',
  bindings: {
    myAttribute: 'myStringReferenceTo$scopeVariable'
  }
});

The bindings object here will just add the attributes and values to the component, where the attribute name will be the properties and the values will be the values of the object.

Does this sound amenable?

As for $onChanges, where are you interested in having it working, in the modal controller? I don't think it makes sense for it to be supported in the modal controller. We should add support for $onDestroy I think though, but that is a separate issue to this one.

[Olgagr]

@wesleycho Lifecycle hooks should work in the component you assign to the modal. They are so useful and if the component doesn't fire them, it will be confusing.

[wesleycho]

So they're not working in the component currently when in use? Can you file another issue, that sounds like a bug.

[Olgagr]

@wesleycho I'd already done it (#5881), but the issue was closed as duplicate of this discussion.

[stephen-nba]

Hi, i'm new to Angular and this angular-ui-bootstrap library, my project uses the new component structure of Angular 1.5.

Can someone please summarise this discussion for me? I can sort of create a component modal but there are some aspects of data binding that wont work? Is there a concise example of how I would use it for now until the feature is implemented in 2.1? Thanks!

Never mind! Looks like there is a SO thread here:

http://stackoverflow.com/questions/36401375/how-to-use-angular-component-with-ui-bootstrap-modal-in-angular-1-5

[wesleycho]

This isn't the place for such questions. There are examples in this issue on how to do this - otherwise, it is encouraged to ask help questions in places such as Stack Overflow, IRC, Gitter, or Slack.

[jeserkin]

@wesleycho will this new component based solution still need controller to be assigned in modal open action? Component as well as it predecessor directive of type element (restrict: 'E') already have controller. Can't it be used. So that last example here would look like:

$uibModal.open({
  component: 'myComponent',
  bindings: { // Previous resolve property (I assume)
    myAttribute: 'myStringReferenceTo$scopeVariable'
  }
});

[Olgagr]

@jeserkin How can you pass bindings to component/directive without this controller in modal open action ?

[mattdarveniza]

@Olgagr the controller should be defined on your component, so it'd receive its bindings through the bindings object, just as in the case of ui-router.

[wesleycho]

Let's make sure to get this right.

I think for @Olgagr's use case, how we want to do this should be

$uibModal.open({
  component: 'differentComponent',
  resolve: {
    ...
  }
})

where differentComponent contains the desired components and the default attribute-value pair will be $resolve="$resolve (which is already published onto $scope).

I think this makes the most sense since all that controller stuff isn't needed here since that should be encapsulated by the component. This may require a bit of extra verbosity in some cases due to having to define the component, but it isn't necessary to change to this syntax - this is just offering a different option for those who desire it. It is wholly backwards compatible with pre-Angular 1.5 because it doesn't rely on anything Angular 1.5 specific in the library itself, and provides simplicity in API form.

Thoughts?

[jeserkin]

I might be getting that wrong way, but the end result would mean, that when I instantiate modal open action I pass in a resolve object containing the elements, that will be injected into component responsible for modalInstance, correct? If so, then how will resolve object be injected into component? Like before as for responsible controller, where every resolve property was a function and result was independent injectable for that exact responsible controller or in some other way? Maybe a more complete example could be provided for the vision?

[wesleycho]

@jeserkin in the example of the proposal I posted, the modal would create

<different-component $resolve="$resolve"></different-component>

and one would be responsible for determining how the resolve is handled inside the component.

[mattdarveniza]

@wesleycho your example is exactly how I'd envision using this. Resolve's should automatically map to the component's bindings when provided, eliminating the need for a wrapping controller or template.

[wesleycho]

I have opened an implementation in #6179 - if there is any comment about implementation anyone wants to add, feel free to comment in that PR.

[mogharsallah]

Awesome!, I was just searching for this possibility 👍 Many thanks