Skip to content

Attributes: Warn and fill .toggleClass( boolean ) #181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/attributes.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

var oldRemoveAttr = jQuery.fn.removeAttr,
oldToggleClass = jQuery.fn.toggleClass,
rmatchNonSpace = /\S+/g;

jQuery.fn.removeAttr = function( name ) {
Expand All @@ -14,3 +15,34 @@ jQuery.fn.removeAttr = function( name ) {

return oldRemoveAttr.apply( this, arguments );
};

jQuery.fn.toggleClass = function( state ) {

// Only deprecating no-args or single boolean arg
if ( state !== undefined && typeof state !== "boolean" ) {
return oldToggleClass.apply( this, arguments );
}

migrateWarn( "jQuery.fn.toggleClass( boolean ) is deprecated" );

// Toggle entire class name of each element
return this.each( function() {
var className = this.getAttribute && this.getAttribute( "class" ) || "";

if ( className ) {
jQuery.data( this, "__className__", className );
}

// If the element has a class name or if we're passed `false`,
// then remove the whole classname (if there was one, the above saved it).
// Otherwise bring back whatever was previously saved (if anything),
// falling back to the empty string if nothing was stored.
if ( this.setAttribute ) {
this.setAttribute( "class",
className || state === false ?
"" :
jQuery.data( this, "__className__" ) || ""
);
}
} );
};
44 changes: 44 additions & 0 deletions test/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,47 @@ QUnit.test( ".removeAttr( boolean attribute )", function( assert ) {
} );

} );

QUnit.test( ".toggleClass( boolean )", function( assert ) {
assert.expect( 14 );

var e = jQuery( "<div />" ).appendTo( "#qunit-fixture" );

expectWarning( "toggling initially empty class", function() {
e.toggleClass( true );
assert.equal( e[ 0 ].className, "", "Assert class is empty (data was empty)" );
} );

expectNoWarning( ".toggleClass( string ) not full className", function() {
e.attr( "class", "" );
e.toggleClass( "classy" );
assert.equal( e.attr( "class" ), "classy", "class was toggle-set" );
e.toggleClass( "classy", false );
assert.equal( e.attr( "class" ), "", "class was toggle-removed" );
} );

expectWarning( ".toggleClass() save and clear", 1, function() {
e.addClass( "testD testE" );
assert.ok( e.is( ".testD.testE" ), "Assert class present" );
e.toggleClass();
assert.ok( !e.is( ".testD.testE" ), "Assert class not present" );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe test that e has none of those classes and not that it just doesn't have both at the same time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests were copied from core pretty much as-is so I hesitate to add more assertions. Whatever it was doing before was lightly documented and we're deprecating it now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK then!


// N.B.: Store should have "testD testE" now, next test will assert that
} );

expectWarning( ".toggleClass() restore", 1, function() {
e.toggleClass();
assert.ok( e.is( ".testD.testE" ), "Assert class present (restored from data)" );
} );

expectWarning( ".toggleClass( boolean )", 1, function() {
e.toggleClass( false );
assert.ok( !e.is( ".testD.testE" ), "Assert class not present" );
e.toggleClass( true );
assert.ok( e.is( ".testD.testE" ), "Assert class present (restored from data)" );
e.toggleClass();
e.toggleClass( false );
e.toggleClass();
assert.ok( e.is( ".testD.testE" ), "Assert class present (restored from data)" );
} );
} );
6 changes: 6 additions & 0 deletions warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,9 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58
**Cause:** The standard way to add new custom selectors through jQuery is `jQuery.expr.pseudos`. These two other aliases are deprecated, although they still work as of jQuery 3.0.

**Solution:** Rename any of the older usage to `jQuery.expr.pseudos`. The functionality is identical.

### JQMIGRATE: jQuery.fn.toggleClass( [ boolean ] ) is deprecated

**Cause:** Calling `.toggleClass()` with no arguments, or with a single Boolean `true` or `false` argument, has been deprecated. Its behavior was poorly documented, but essentially the method saved away the current `class` value in a data item when the class was removed and restored the saved value when it was toggled back. If you do not believe you are specificially trying to use this form of the method, it is possible you are accidentally doing so via an inadvertent undefined value, as `.toggleClass( undefined )` toggles all classes.

**Solution:** If this functionality is still needed, save the current full `.attr( "class" )` value in a data item and restore it when required.