-
Notifications
You must be signed in to change notification settings - Fork 95
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
Add support for vw/vh/vmin/vmax units #52
base: master
Are you sure you want to change the base?
Conversation
2b92863
to
c3ca7a6
Compare
} | ||
} | ||
|
||
return orientation === "vertical" |
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.
Orientation is resolved from block or inline to horizontal or vertical value within calculateScrollOffset without regard for the writing mode. It is not being used elsewhere in the function. Suggest we move the logical to physical orientation mapping to the top of this function (including the TODO unless fixed in the same PR). directionAwareScrollOffset has the rules for mapping block and inline.
Anything that can be done to help get this PR through? I just ran into this not being available so I'm managing window dimensions with a resize event and then passing them to CSSUnitValue for now. |
So I think with the new API (@kevers-google PR) the only parsed length is currently view-timeline-inset. It may be worth having support for viewport units there (when the polyfill supports this) and in possible future length additions, but I think we'll want to be careful to properly track invalidations. I.e. when something is based on viewport units it should trigger an invalidation on window The TLDR, we could add this, with TODO's for missing pieces (e.g. invalidations on viewport size changes). It currently isn't used by anything, but will likely be a benefit in the future to support these units. |
case 'vw': | ||
return Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0); | ||
case 'vh': | ||
return Math.max(document.documentElement.clientHeight || 0, window.innerHeight || 0); |
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 think this will return the size of the dynamic viewport on iOS Safari, and not the large viewport?
I wish there was an js api for reading these viewport relative units.
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.
The document.documentElement.client*
properties return the size of the ICB1. In practice – and soon in the spec too – this is the same value as reported by the the sv*
units, i.e. when the dynamic browser UI is expanded.
The Layout Viewport typically equals the values reported by window.innerWidth
/window.innerHeight
. When the dynamic browser UI gets retracted, these values grow as more space becomes available2.
So by theory you could just use use window.innerWidth
/window.innerHeight
. In practice there is a bug in iOS Safari where these values shrink when having pinch-zoomed in3. WebKit on mobile is the only engine that does this.
That should explain why there is a fallback to the document.documentElement.client*
values. It’s not 100% correct when having pinch-zoomed in, but at least it‘s not entirely wrong.
Footnotes
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.
The
document.documentElement.client*
properties return the size of the ICB1. In practice – and soon in the spec too – this is the same value as reported by the thesv*
units, i.e. when the dynamic browser UI is expanded.
Nice. I didn't catch that resolution. 👍
I appreciate the write-up 🙏
We have a way to resolve the small viewport size by evaluating document.documentElement.clientHeight
. Evaluating window.innerHeight
will give us the dynamic viewport size. (Apart from the bugs)
However, I don't see that we have a way to resolve the large viewport size though? Without injecting something into the dom to measure it?
Math.max(document.documentElement.clientHeight || 0, window.innerHeight || 0);
The expression above will resolve to the size of the dynamic viewport? (And at times be smaller than the large viewport size)
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.
However, I don't see that we have a way to resolve the large viewport size though? Without injecting something into the dom to measure it?
Correct.
The expression above will resolve to the size of the dynamic viewport? (And at times be smaller than the large viewport size)
Also correct (except for WebKit on Mobile when pinch-zoomed in due to that bug they have), so the code needs to be updated (for mobile).
Quickly worked out this that should get all values. Doesn’t include the fix for WebKit on Mobile though: https://codepen.io/bramus/pen/dyaJOza?editors=0010
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.
Note with that demo/code linked above: only seems to give correct results in MobileSafari (on iOS 17.0.3) itself. Chrome/Edge/Firefox give wrong results, most likely because they are a limited WebKit under the hood.
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.
Note with that demo/code linked above: only seems to give correct results in MobileSafari (on iOS 17.0.3) itself. Chrome/Edge/Firefox give wrong results, most likely because they are a limited WebKit under the hood.
Yeah.. Viewport-percentage lengths are still unreliable in many WebViews and In App Browsers 😞.
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.
Just came to the realization that Viewport Units in a WebKit WebView on iOS 17 are entirely broken. This wasn't the case on iOS 15.
Test Page: https://interop-2022-viewport.netlify.app/combined/viewport-units/
I tried to merge this with the changes on the main branch, but the modified I'm not quite sure if we have anything equivalent to this function afterwards, @kevers-google ? |
I’ll try to see where exactly this would fit in nowadays. |
We have implemented the scroll-timeline/src/scroll-timeline-base.js Lines 676 to 680 in 5a2639a
We could resolve the viewport units there: scroll-timeline/src/simplify-calculation.js Lines 75 to 83 in 5a2639a
I just pushed a draft PR where I use the procedure to support CSSUnitValues for ViewTimeline inset: scroll-timeline/src/scroll-timeline-base.js Lines 753 to 756 in ab45fa7
|
The CSS Polyfill does not support Viewport Units. This PR adds support for vw/vh/vmin/vmax units. It doesn’t cover vi/vb units.
One possible improvement could be to add a resize observer onto the window, which would calculage and cache the px values for the vw/vh units.
Demo which uses this version: https://codepen.io/bramus/pen/ExgOPRw