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

CSS fixes rollup: fixes #10796 and fixes #10639 #616

Closed
wants to merge 5 commits into from

Conversation

mikesherov
Copy link
Member

So I submitted a separate PR for #10796, but I needed the change from there to effectively fix #10639 So I created a PR with both changes.

If you take this one in, please ignore #610.
If you take #610, you can always take this one later!

style = elem.style;
width = style.width;
style.width = name === "fontSize" ? "1em" : ( ret || 0 );
ret = computedStyle.getPropertyValue("width") + "px";
Copy link
Member

Choose a reason for hiding this comment

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

Nice to see the new style guide changes folding in. As soon as the JSHint updates are rolled in I will have a full update of the change

@mikesherov
Copy link
Member Author

So, the perf is available here: http://jsperf.com/jquery-10693

It's VERY slow, but I don't see another way to get this to be faster. I tried using jQuery._data to store a cached copy of the pixel to percent ratio, but that to is very slow. I CAN hack it further to instead of using jQuery.data to store a cached copy of the ratio, store it in elem.cachedPixelRatio, but that seemed very hacky and was a lot of code.

At this point, it's a matter of speed vs. maintainability vs. size vs. correctness... I'd love some further comments and criticisms on this.

Thanks!

@rwaldron
Copy link
Member

expando turf is a dark road, I recommend exhausting your options

@mikesherov
Copy link
Member Author

I believe they're all exhausted unfortunately.

@@ -277,6 +278,11 @@ jQuery.support = (function() {
offsetSupport.subtractsBorderForOverflowNotVisible = ( inner.offsetTop === -5 );
offsetSupport.doesNotIncludeMarginInBodyOffset = ( body.offsetTop !== conMarginTop );

if( window.getComputedStyle ) {
Copy link
Member

Choose a reason for hiding this comment

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

if( => if (

@dmethvin
Copy link
Member

dmethvin commented Dec 7, 2011

Landed. 7f6a991

@dmethvin dmethvin closed this Dec 7, 2011
npmcomponent pushed a commit to npmcomponent/danzajdband-vanilla-masonry that referenced this pull request Jan 6, 2014
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 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.

4 participants