Skip to content

#11938 - css() and similar functions should accept Array parameter #834

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 1 commit into from
Closed

Conversation

visiongeist
Copy link

added multiple gets handling in jQuery.access() through array parameter

added multiple gets handling in jQuery.access() through array parameter
@mikesherov
Copy link
Member

Hmm, this looks like its just a convenience method and doesn't give any perf boost. I would expect that this pull request would attempt getting computedStyle just once. This way, it could be used internally to speed up situations in which we need multiple style attributes, i.e. determining offset or dimensions.

@visiongeist
Copy link
Author

True. I didn't want to restructure too much to avoid any side effects.

However I could also change the css() to make it aware about the Array parameter and handle the return object creation in curCSS. The downside of this is that other functions like attr() would need to do that too (if the array parameter would be used..)

Another solution I could image: I could fetch the computed style in css() (css.js line 100) and pass it down to curCSS as an additional parameter.

Any other suggestions how to solve that?

@mikesherov
Copy link
Member

I am not personally looking to bring array getters to anything other than CSS. But that's just me. I have a selfish need for it. I agree there is a fair size rewrite there, but perf and clarity are opposite sides of the same coin.

In terms of suggestions, the only advice I have is that the most performant approach gets computed styles once. That's the only way I'd be +1 on this pull.

@dmethvin
Copy link
Member

Since we're on the verge of beta for 1.8, I think it will be worthwhile to discuss how we want this to work for 1.9. A few perf tests would be nice to find out if we really save a lot by making only one call to gCS. I like the feature if it's not too big.

In the meantime I'll close this since it will be a while before we land anything.

@dmethvin dmethvin closed this Jun 21, 2012
@mikesherov
Copy link
Member

sounds good.

@visiongeist
Copy link
Author

sry even the request is close just to let you know: I quickly implemented this with a single gCS call. They Array still works on all functions which call the access() though in css() the gCS will be cached to increase perf

https://github.com/dantipa/jquery/commit/f7490d4088540803a3eecb33725ced9f6c1214bc

@mikesherov
Copy link
Member

@dantipa, that's a great rough draft! We'll revisit it when 1.9 is in discussion!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 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.

3 participants