Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Impossible to disable fade in modal #1007

Closed
akarelas opened this issue Sep 14, 2013 · 18 comments
Closed

Impossible to disable fade in modal #1007

akarelas opened this issue Sep 14, 2013 · 18 comments

Comments

@akarelas
Copy link

It's not possible to make modals appear instantaneously (except perhaps by redefining the 'fade' rules project-wide, which I don't want to do).

I read somewhere that what's needed is to remove the 'fade' class from the modal div.

Here's how this could be done: Allow the 'windowClass' property of the modal options object to be an object (e.g. { fade: false } ), and (if object) merge it with the default { modal: true, fade: true } and put it in the modal div's ng-class.

What do you think?

@akarelas
Copy link
Author

Not necessary after all. All I had to do was add a 'dont-fade' class to that modal, and define THAT as trasition: none;

@eddiemonge
Copy link
Contributor

I think the object idea was good. What if i want a slide instead of a fade. I shouldnt have to define a css class to disable the fade AND and a class to add a new transition.

@ProLoser
Copy link
Member

ProLoser commented Dec 2, 2013

Yeah... this really seems like the wrong behavior to me. I think this is worth re-opening.

@pkozlowski-opensource
Copy link
Member

Actually a proper solution here is to override a template and add / remove all the css clases you need. Js code only controls triggering animations and not particular css classes used.

@ProLoser
Copy link
Member

ProLoser commented Dec 3, 2013

You don’t think that that solution is inversely proportional to the problem when you could just remove the fade class from the hard-coded HTML?
-- 
Dean Sofer
Sent with Airmail

On December 2, 2013 at 10:36:52 PM, Pawel Kozlowski (notifications@github.com) wrote:

Actually a proper solution here is to override a template and add / remove all the css clases you need. Js code only controls triggering animations and not particular css classes used.


Reply to this email directly or view it on GitHub.

@pkozlowski-opensource
Copy link
Member

@ProLoser not sure what you mean by "hard-coded" by currently the fade class (as well as the rest of markup) is externalized in bootstrap-specific template:
https://github.com/angular-ui/bootstrap/blob/master/template/modal/window.html

People are free to override this template (it is one liner!) to change default BS classes or even change the markup altogether to work with a different CSS framework. At the end of a day you need to put those default CSS classes somewhere so things work out of the box (still leaving full flexibility to users if they wish to change / override defaults).

@eddiemonge
Copy link
Contributor

they would be free to override it if there was an option to specify the template path. without that, you have to hard code the template in dev mode. in production the templates should be cached in a JS object or the $templateCache but its harder to test in dev mode if your apps templates arent in /templates/xyz.

@pkozlowski-opensource
Copy link
Member

@eddiemonge ok, I would really like to understand this one better as we are going to push more and more customizations via template overriding. The basic premise here is that we will never be able to support all the combination of config params people might think of and you can get very far with template customization.

What are the practical issues you are facing? What is your workflow? I guess you've read through this:
https://github.com/angular-ui/bootstrap#customize-templates

Once again, I would like to understand better what practical issues people are facing as we want those templates to be easily customizable.

@eddiemonge
Copy link
Contributor

thats fine under ideal circumstances but it makes debugging harder if, lets say there are the following conditions:

  • Dev templates are ajax loaded and only production builds them to the cache
  • Dev templates are not in html and thus need to build the complexity for grunt (although this one isnt so much of an issue). Also if you want to see the rendered template you now have to open it from the file system instead of of seeing it dev tools network sources
  • There can be multiple templates for each widget, like different templates for different modals

There are I am sure many other circumstances where having a customized path woud be acceptable.

I don't know what would go into making the templateUrl adjustable but if I did a PR and knew it would land, I'd be inclined to look into it.

@pkozlowski-opensource
Copy link
Member

What I do in practice when overriding a template is to start with the <script> tag approach which covers your point (1) and (2). When the initial design settles I'm moving it to separate files, add unit test and forget about them, loading in dev from templateCache since it is faster in practice.

Once again, I'm not saying this workflow is ideal but let's put things in perspective - we are talking about overriding widget templates which you do once in a while - I mean, it is not part of a daily work on an application.

Point (3) is harder as of today there is really no good mechanism in AngularJS to have alternative sets of templates. In any case a proper solution for this should come from AngularJS itself.

As far as PRs are concerned - those are always welcomed but can never have guarantee of being merged... I think AngularJS 1.2 brings dynamic templates / templateUrls which gives us additional flexibility. So go for it if you've got an idea if making things better but once again - can't give you any guarantees.

@ProLoser
Copy link
Member

ProLoser commented Dec 3, 2013

This is a PITA when you want multiple versions.
On Dec 3, 2013 11:54 AM, "Pawel Kozlowski" notifications@github.com wrote:

What I do in practice when overriding a template is to start with the

<script> tag approach which covers your point (1) and (2). When the initial design settles I'm moving it to separate files, add unit test and forget about them, loading in dev from templateCache since it is faster in practice. Once again, I'm not saying this workflow is ideal but let's put things in perspective - we are talking about overriding _widget_ templates which you do once in a while - I mean, it is not part of a daily work on an application. Point (3) is harder as of today there is really no good mechanism in AngularJS to have alternative sets of templates. In any case a proper solution for this should come from AngularJS itself. As far as PRs are concerned - those are always welcomed but can never have guarantee of being merged... I think AngularJS 1.2 brings dynamic templates / templateUrls which gives us additional flexibility. So go for it if you've got an idea if making things better but once again - can't give you any guarantees. — Reply to this email directly or view it on GitHubhttps://github.com//issues/1007#issuecomment-29744950 .

@pkozlowski-opensource
Copy link
Member

@ProLoser guys, I'm on your side here, trying to treat you as partners in discussion in the search for the best solution, but please, try to be more constructive. What do you mean by multiple versions? What is your real issue? What are the alternatives you can see?

@ProLoser
Copy link
Member

ProLoser commented Dec 3, 2013

I have modals throughout my app, all of them make use of fading. The login
modal however should show before the rest of the app loads so I prefer it
to show up instantaneously. The proposed solution was to simply remove the
classes from the html and use an expression or variable instead.
I have been thinking about having modalClass contain the 'fade' class by
default that you can then pass an empty or alternative string to, but that
would mean people have to re-add the fade class. I am trying to think of
more elegant solutions.
On Dec 3, 2013 12:03 PM, "Pawel Kozlowski" notifications@github.com wrote:

@ProLoser https://github.com/ProLoser guys, I'm on your side here,
trying to treat you as partners in discussion in the search for the best
solution, but please, try to be more constructive. What do you mean by
multiple versions? What is your real issue? What are the alternatives you
can see?


Reply to this email directly or view it on GitHubhttps://github.com//issues/1007#issuecomment-29745827
.

@saem
Copy link

saem commented Feb 3, 2014

Overriding with options/template as indicated by @pkozlowski-opensource makes sense, but it doesn't help in all scenarios, see below.

Currently within modal.js:140 the am-fade class is hard coded onto the backdrop. There is no way to change this in the template, this might not be the only instance of it. We can change the animation on the modal itself, but not the backdrop. We're trying to avoid angular motion as much as possible and stick with what bootstrap provides so it's easier for us to theme in the future.

@akarelas
Copy link
Author

akarelas commented Jun 8, 2014

Since a solution has not been found yet, I'm reopening this issue.

@akarelas akarelas reopened this Jun 8, 2014
@akarelas
Copy link
Author

akarelas commented Jun 8, 2014

My trasition: none; trick (see second message of this issue) does not work anymore. A solution is desperately needed.

@chrisirhc
Copy link
Contributor

Will be resolved in #2725

@theblang
Copy link

I would love to see this fixed. A modal with no transition is very snappy.

@chrisirhc chrisirhc self-assigned this Mar 28, 2015
chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Mar 30, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
@karianna karianna added this to the 0.13.0 milestone Mar 30, 2015
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 9, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
fernando-sendMail pushed a commit to fernando-sendMail/bootstrap that referenced this issue Jul 16, 2015
Note: Move backdropClass logic into compile function because otherwise
modifying classes in the compile function is broken when using an interpolated
class attribute.

Fixes angular-ui#1007
Closes angular-ui#2725
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants