Skip to content
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

support css value !important #2016

Closed
wants to merge 2 commits into from
Closed

Conversation

bluelovers
Copy link

$('div').css('blue', '!important');

@dmethvin
Copy link
Member

That example doesn't even make sense, there is no property named blue. That probably would have been a bit more obvious if the PR had added unit tests. See #1385 for a previous failed attempt, and check out our Contributing site for more help.

@mgol
Copy link
Member

mgol commented Jan 18, 2015

That example doesn't even make sense, there is no property named blue.

I guess the OP meant:

$('div').css('color', 'blue !important');

That said, the PR introduces the nativeCSSStyleDeclaration support test but doesn't use it and doesn't test it (see test/unit/support.js; btw, I don't think the support test is needed on master) and, as @dmethvin pointed out, there are no unit tests attached.

Also, this is a pretty critical code path and the !important use case is not terribly common; I'd like to see perf tests to make sure it doesn't make it worse for the 90% use cases.

@bluelovers
Copy link
Author

xd sorry type wrong demo code

$('div').css('color', 'blue !important');

@timmywil
Copy link
Member

Thank you for taking the time to submit a PR, but this will not make it in.

@timmywil timmywil closed this Feb 18, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants