custom css class in $modal template

[fxck]

I did read this #441 (comment) and it never really came to a satisfying conclusion.

The thing is, you won't be changing template/modal/window.html 99% of time, what you might need to change quite often is its class.

You might have 3 sets of modals for different cases, all with a different width. And since width is defined on the parent(.modal), how could do it?

The dialogClass option as it was in the old $dialog made imho perfect sense.

[karol-f]

+1

[D3CK3R]

+1

[hallister]

I believe the idea behind removing dialogClass is to make templates "the place" where we do modifications like this. That said, I think this could be done, but perhaps with a modalTemplate or modalTemplateURL. This way we keep dialog free of DOM awareness, and still allow for custom modal classes. @pkozlowski-opensource thoughts?

[fxck]

Yes, that would work pretty well if you defined modal's width in one of the inner elements.. but you don't. And as long as this is supposed to be native javascript plusing for twitter's bootstrap, that $dialogClass is kind of a must.

[karol-f]

@fxck I agree - this is exactly the case.

[hallister]

@fxck and @blnight I'm not understanding why defining the modals width on an inner element would matter in this case. When I say modalTemplate that would actually be the <div class="modal"></div> part. So a $dialog might look like:

$scope.modalOpen = function() {
    $modal.open({
        backdrop: true,
        modalTemplate: '<div class="modal modal-width-override" ng-transclude></div>',
        templateUrl: 'myModalContent.html',
        controller: ModalInstanceCtrl
    });
});

This way we can define separate templates for separate $modal instances. If this wouldn't work, can you give me more details on your use case?

[fxck]

Yes, that's true, but back to what I initally said, you don't usually change anything but the class in the root element. So you'd end up doing this in every initialization.

modalTemplate: '<div class="modal fade one-third" ng-class="{in: animate}" ng-transclude></div>'

modalTemplate: '<div class="modal fade one-half" ng-class="{in: animate}" ng-transclude></div>'

modalTemplate: '<div class="modal fade two-thirds" ng-class="{in: animate}" ng-transclude></div>'

modalTemplate: '<div class="modal fade big-ass" ng-class="{in: animate}" ng-transclude></div>'

So tell me which is the lesser evil? modalClass or modalTemplate ?

[hallister]

@fxck Well, I definately wouldn't do things that way, I think windowTemplateUrl would be the best option. I really dislike class names directives because it crosses the line of the controller defining the view, especially since $modal is actually a service now. Defining class names inside a controller just seems... counter-intuitive to me.

Regardless, this is just my personal opinion. We'll have to wait on Pawel to decide what the right direction is.

[richardcrichardc]

+1 for a way to vary the width of the modal window.

[richardcrichardc]

Here is a patch to add an extra css class to the window div by specifying it as windowClass option when calling open. Tested with regular jQuery and jQueryLite that comes bundled with angular.

--- modal.js
+++ modal.js
@@ -138,7 +138,7 @@
           backdropDomEl = $compile(angular.element('<div modal-backdrop></div>'))($rootScope);
           body.append(backdropDomEl);
         }
-        var modalDomEl = $compile(angular.element('<div modal-window></div>').html(modal.content))(modal.scope);
+        var modalDomEl = $compile(angular.element('<div modal-window></div>').html(modal.content).addClass(modal.windowClass))(modal.scope);
         body.append(modalDomEl);


@@ -266,7 +266,8 @@
                 deferred: modalResultDeferred,
                 content: tplAndVars[0],
                 backdrop: modalOptions.backdrop,
-                keyboard: modalOptions.keyboard
+                keyboard: modalOptions.keyboard,
+                windowClass:modalOptions.windowClass
               });

             }, function resolveError(reason) {

             }, function resolveError(reason) {

[richardcrichardc]

After applying the above patch and:

1. creating a modal with windowClass set, then
2. creating a modal with with windowClass undefined

You will find the second modal has the windowClass of the first modal applied. Oh oh!

One more change is the solution:
@@ -231,7 +231,7 @@
};

             //merge and clean up options
-            modalOptions = angular.extend(defaultOptions, modalOptions);
+            modalOptions = angular.extend(angular.copy(defaultOptions), modalOptions);
             modalOptions.resolve = modalOptions.resolve || {};

This appears to be a bug in the original implementation. A side effect of merging the defaults and specified options is to replace the memorised options with the merged options. FYI @pkozlowski-opensource

[pkozlowski-opensource]

@fxck @hall5714 so, I'm not sure what is the best way of progressing here. On one hand I hate putting css class names in JS code and I agree here with @hall5714 . At the same time this seems to be the easiest option for developers using the library and I'm buying @fxck argumentation here.

So I would say - let's add this option till we can think of a better solution.

@richardcrichardc would you be up to sending a pull request, with docs and test update? If not I'm going to look into this item tomorrow, would definitively like to tackle this one before a next release.

[richardcrichardc]

What repo should I fork?

It might be quicker if you just copy and paste the changes. I wont be offended if you rename the option or anything. I'm not worried about licences - consider it public domain.

Docs:

WindowClass: Add an addition css to the DIV containing the modal. Use this to override the css in the template on a modal by modal basis, e.g. to change the width of the modal.

On 31/08/2013, at 5:04 AM, Pawel Kozlowski notifications@github.com wrote:

@fxck @hall5714 so, I'm not sure what is the best way of progressing here. On one hand I hate putting css class names in JS code and I agree here with @hall5714 . At the same time this seems to be the easiest option for developers using the library and I'm buying @fxck argumentation here.

So I would say - let's add this option till we can think of a better solution.

@richardcrichardc would you be up to sending a pull request, with docs and test update? If not I'm going to look into this item tomorrow, would definitively like to tackle this one before a next release.

—
Reply to this email directly or view it on GitHub.

[hallister]

Sounds good @pkozlowski-opensource. The one brick wall we are going to run into is Bootstrap 3's new modal layout. For example, the width of a modal is now controlled on .modal-dialog. All the border settings are on .modal-content and modal position is controlled on .modal.

So now, do we put {{ modalWindowClass }} on .modal so we can adjust position, or on .modal-dialog so we can adjust width? That's one of the main reasons I'm concerned about sending somethingClass through, because we don't know where the class should be in any given situation.