Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($animate): remove the need to add display:block!important for ngS…
Browse files Browse the repository at this point in the history
…how/ngHide

Since ngShow/ngHide animations add and remove the .ng-hide class, having to remember
to write display:block on your own is a hassle and leads to problematic animation
code. This fix places a default on the animation for you instead.

Closes #3813
  • Loading branch information
matsko committed Jun 4, 2014
1 parent 36b2bba commit 7c011e7
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 20 deletions.
6 changes: 6 additions & 0 deletions css/angular.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@
ng\:form {
display: block;
}

/* show the element during a show/hide animation when the
* animation is ongoing, but the .ng-hide class is active */
.ng-hide-add-active, .ng-hide-remove {
display: block!important;

This comment has been minimized.

Copy link
@minipai

minipai Jun 5, 2014

This cause problem when using animate on inline or inline-block elements, etc.
I suggest use display:initial instead.

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

display:initial would set inline for an element that may already have display:block set by the user. And it's not supported by IE.

I think the better option is to make the .ng-hide CSS class less greedy:

.ng-hide:not(.ng-hide-remove):not(.ng-hide-add-active) {
   display:none!important;
}

This comment has been minimized.

Copy link
@ashclarke

ashclarke Jun 5, 2014

Will this cause issues if you are using display: flex?

I assume in cases like these you should write more specific styles to overwrite anyway?

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

The selector above should not interfere with flex since only none is being applied at the right time.

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

@ashclarke just tested flexbox with the following code and it works fine:

.ng-hide:not([class~="ng-hide-remove"]):not([class~="ng-hide-add-active"]) {
  display: none !important;
}

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

The reason why I'm using attribute selectors and not class selectors is so that the CSS .ng-hide code can be easily overridden by using a compound selector:

.my-animation.ng-hide {
  display:block!important; position:absolute; top:-9999px; left:-9999px;
}

This comment has been minimized.

Copy link
@ashclarke

ashclarke Jun 5, 2014

Thanks @matsko. Good to know!

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

Ahhh we can't use the attribute selectors since CSS weighs them the same as class selectors. The new CSS selector is like so:

.ng-hide:not(.ng-animate) {
  display: none !important;
}

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

Also, :not support for IE8 doesn't exist. So this fix won't be going into 1.2.

This comment has been minimized.

Copy link
@matsko

matsko Jun 5, 2014

Author Contributor

Merged: 1d90744

}
20 changes: 0 additions & 20 deletions src/ng/directive/ngShowHide.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@
* restating the styles for the .ng-hide class in CSS:
* ```css
* .ng-hide {
* /* Not to worry, this will override the AngularJS default...
* display:block!important;
*
* /* this is just another form of hiding an element */
* position:absolute;
* top:-9999px;
Expand Down Expand Up @@ -72,9 +69,6 @@
* /* this is required as of 1.3x to properly
* apply all styling in a show/hide animation */
* transition:0s linear all;
*
* /* this must be set as block so the animation is visible */
* display:block!important;
* }
*
* .my-element.ng-hide-add-active,
Expand Down Expand Up @@ -126,11 +120,6 @@
background:white;
}
.animate-show.ng-hide-add,
.animate-show.ng-hide-remove {
display:block!important;
}
.animate-show.ng-hide-add.ng-hide-add-active,
.animate-show.ng-hide-remove.ng-hide-remove-active {
-webkit-transition:all linear 0.5s;
Expand Down Expand Up @@ -214,9 +203,6 @@ var ngShowDirective = ['$animate', function($animate) {
* restating the styles for the .ng-hide class in CSS:
* ```css
* .ng-hide {
* //!annotate CSS Specificity|Not to worry, this will override the AngularJS default...
* display:block!important;
*
* //this is just another form of hiding an element
* position:absolute;
* top:-9999px;
Expand Down Expand Up @@ -244,7 +230,6 @@ var ngShowDirective = ['$animate', function($animate) {
* //
* .my-element.ng-hide-add, .my-element.ng-hide-remove {
* transition:0.5s linear all;
* display:block!important;
* }
*
* .my-element.ng-hide-add { ... }
Expand Down Expand Up @@ -292,11 +277,6 @@ var ngShowDirective = ['$animate', function($animate) {
background:white;
}
.animate-hide.ng-hide-add,
.animate-hide.ng-hide-remove {
display:block!important;
}
.animate-hide.ng-hide {
line-height:0;
opacity:0;
Expand Down

0 comments on commit 7c011e7

Please sign in to comment.