Permalink
Browse files

CSS: Toggle detached elements as visible unless they have display: none

Fixes gh-2863
Closes gh-3037
  • Loading branch information...
1 parent ce6c83f commit 755e7ccf018eb150eddefe78063a9ec58b3229e3 @gibson042 gibson042 committed Apr 4, 2016
Showing with 61 additions and 14 deletions.
  1. +1 −6 src/css/showHide.js
  2. +19 −4 src/css/var/isHiddenWithinTree.js
  3. +24 −0 test/unit/css.js
  4. +17 −4 test/unit/effects.js
View
@@ -54,12 +54,7 @@ function showHide( elements, show ) {
elem.style.display = "";
}
}
- if ( elem.style.display === "" && jQuery.css( elem, "display" ) === "none" &&
-
- // Support: Firefox 43+
- // Don't set inline display on disconnected elements with computed display: none
- jQuery.contains( elem.ownerDocument, elem ) ) {
-
+ if ( elem.style.display === "" && isHiddenWithinTree( elem ) ) {
values[ index ] = getDefaultDisplay( elem );
}
} else {
@@ -5,14 +5,29 @@ define( [
// css is assumed
], function( jQuery ) {
- // This function differs from the :hidden selector
- // in that it intentionally ignores hidden ancestors (gh-2404)
+ // isHiddenWithinTree reports if an element has a non-"none" display style (inline and/or
+ // through the CSS cascade), which is useful in deciding whether or not to make it visible.
+ // It differs from the :hidden selector (jQuery.expr.pseudos.hidden) in two important ways:
+ // * A hidden ancestor does not force an element to be classified as hidden.
+ // * Being disconnected from the document does not force an element to be classified as hidden.
+ // These differences improve the behavior of .toggle() et al. when applied to elements that are
+ // detached or contained within hidden ancestors (gh-2404, gh-2863).
return function( elem, el ) {
// isHiddenWithinTree might be called from jQuery#filter function;
// in that case, element will be second argument
elem = el || elem;
- return jQuery.css( elem, "display" ) === "none" ||
- !jQuery.contains( elem.ownerDocument, elem );
+
+ // Inline style trumps all
+ return elem.style.display === "none" ||
+ elem.style.display === "" &&
+
+ // Otherwise, check computed style
+ // Support: Firefox <=43 - 45
+ // Disconnected elements can have computed display: none, so first confirm that elem is
+ // in the document.
+ jQuery.contains( elem.ownerDocument, elem ) &&
+
+ jQuery.css( elem, "display" ) === "none";
};
} );
View
@@ -926,6 +926,30 @@ QUnit[ jQuery.find.compile && jQuery.fn.toggle ? "test" : "skip" ]( "toggle()",
jQuery.fn.hide = oldHide;
} );
+QUnit[ jQuery.find.compile && jQuery.fn.toggle ? "test" : "skip" ]( "detached toggle()", function( assert ) {
+ assert.expect( 6 );
+ var detached = jQuery( "<p><a/><p>" ).find( "*" ).addBack(),
+ hiddenDetached = jQuery( "<p><a/></p>" ).find( "*" ).addBack().css( "display", "none" ),
+ cascadeHiddenDetached = jQuery( "<p><a/></p>" ).find( "*" ).addBack().addClass( "hidden" );
+
+ detached.toggle();
+ detached.appendTo( "#qunit-fixture" );
+ assert.equal( detached[ 0 ].style.display, "none", "detached element" );
+ assert.equal( detached[ 1 ].style.display, "none", "element in detached tree" );
+
+ hiddenDetached.toggle();
+ hiddenDetached.appendTo( "#qunit-fixture" );
+ assert.equal( hiddenDetached[ 0 ].style.display, "", "detached, hidden element" );
+ assert.equal( hiddenDetached[ 1 ].style.display, "", "hidden element in detached tree" );
+
+ cascadeHiddenDetached.toggle();
+ cascadeHiddenDetached.appendTo( "#qunit-fixture" );
+ assert.equal( cascadeHiddenDetached[ 0 ].style.display, "none",
+ "detached, cascade-hidden element" );
+ assert.equal( cascadeHiddenDetached[ 1 ].style.display, "none",
+ "cascade-hidden element in detached tree" );
+} );
+
QUnit.test( "jQuery.css(elem, 'height') doesn't clear radio buttons (bug #1095)", function( assert ) {
assert.expect( 4 );
View
@@ -1547,15 +1547,17 @@ QUnit.test( "animate should set display for disconnected nodes", function( asser
assert.expect( 20 );
var env = this,
- methods = {
- toggle: [ 1 ],
- slideToggle: [],
+ showMethods = {
fadeIn: [],
fadeTo: [ "fast", 0.5 ],
slideDown: [ "fast" ],
show: [ 1 ],
animate: [ { width: "show" } ]
},
+ toggleMethods = {
+ toggle: [ 1 ],
+ slideToggle: []
+ },
$divEmpty = jQuery( "<div/>" ),
$divTest = jQuery( "<div>test</div>" ),
$divNone = jQuery( "<div style='display: none;'/>" ),
@@ -1578,7 +1580,7 @@ QUnit.test( "animate should set display for disconnected nodes", function( asser
assert.expectJqData( env, $divNone[ 0 ], "olddisplay" );
- jQuery.each( methods, function( name, opt ) {
+ jQuery.each( showMethods, function( name, opt ) {
jQuery.fn[ name ].apply( jQuery( "<div/>" ), opt.concat( [ function() {
assert.strictEqual( jQuery( this ).css( "display" ), nullParentDisplay,
"." + name + " block with null parentNode" );
@@ -1589,6 +1591,17 @@ QUnit.test( "animate should set display for disconnected nodes", function( asser
"." + name + " block under fragment" );
} ] ) );
} );
+ jQuery.each( toggleMethods, function( name, opt ) {
+ jQuery.fn[ name ].apply( jQuery( "<div/>" ), opt.concat( [ function() {
+ assert.strictEqual( jQuery( this ).css( "display" ), "none",
+ "." + name + " block with null parentNode" );
+ } ] ) );
+
+ jQuery.fn[ name ].apply( jQuery( "<div>test</div>" ), opt.concat( [ function() {
+ assert.strictEqual( jQuery( this ).css( "display" ), "none",
+ "." + name + " block under fragment" );
+ } ] ) );
+ } );
clock.tick( 400 );
} );

0 comments on commit 755e7cc

Please sign in to comment.