-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
WIP Detect browser for real max element size #936
Conversation
Boilerplate for checking <div style="width: 1.0e+10px; height: 1.0e+10px;"></div>
<script>
const element = document.querySelector('div');
console.log(element.offsetWidth, element.offsetHeight, element.offsetWidth === element.offsetHeight);
</script> |
Hey, this is cool. 😄 Does it work? Is it solid enough to release? |
Not sure. Browser testers I just googled. Maybe we can use some vendors for this. |
Just curious if you'd smoke tested this and felt like it worked well enough to release. I like it. I've been meaning to do something like it for a while. I'd love to release it if it's stable enough (eg won't break SSR, has a fallback for IE+Edge, etc) |
Didn't try to write SSR test (without jsdom). ie/edge will have default which is already used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm feeling bold. 😄 Let's try it.
Testing more locally, it looks like the max safe value for Chrome (before borders start to blur) is actually closer to 1.67771e+07. This value renders well in Safari too, although drag-scrolling this many pixels in either Safari or Firefox is really bad for perf. Firefox doesn't render reliably anywhere near this high it seems. Borders are arbitrarily darker or thinner at random. 😦 I think I'm going to have to revert this change for now. Or maybe back it out to be only for Chrome. |
Hm.. at first it looked good. I added WIP to not merge this PR yet :) |
Haha, no worries at all. This was my own rushed fault 😄 |
Hi guys, I'm not sure if I'm writing in proper place, but seems this PR brokes SSR on latest version
|
Yikes. Yeah, that looks like. All |
Yep #970 |
@mpospelov Fixed in 9.17.3 |
Woo hoo! Thanks buddy. |
@TrySound Thanks a lot 👍 |
Ref #932
Added naive detectors for chrome, safari, opera and firefox. Can't test edge since I'm on mac, so use default on it. Also didn't test mobile devices. Sadly didn't find info about this with googling.