Skip to content

Conversation

@AurelioDeRosa
Copy link
Member

Fixes gh-312

@AurelioDeRosa
Copy link
Member Author

@agcolom @kswedberg Fixed! Thank you for the feedback.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it makes sense to also mention which property we're actually talking about, instead of just "the property"

Copy link
Member

Choose a reason for hiding this comment

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

@arthurvr, @AurelioDeRosa: 👍 to specifying the property. Maybe something like this?…

The .addClass() method manipulates the className property of the selected elements, not the class attribute. Once the property is changed, it's the browser that updates the attribute accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

Note also that behavior will change in jQuery 3.0, where we will set the class attribute instead of the className property for better SVG support: jquery/jquery#2199

@AurelioDeRosa
Copy link
Member Author

PR updated. @gibson042 I'll open a separate issue for that and create a different PR if you agree. This way we can merge it into the relevant branch.

@gibson042
Copy link
Member

👍

@AurelioDeRosa
Copy link
Member Author

Anyone keen for a final review of this?

@dmethvin
Copy link
Member

LGTM, and we will revisit it when 3.0 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

7 participants