-
Notifications
You must be signed in to change notification settings - Fork 96
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,9 +114,43 @@ export function calculateMaxScrollOffset(source, orientation) { | |
return source.scrollWidth - source.clientWidth; | ||
} | ||
|
||
/** | ||
* Calculates the maximum pixel value to use | ||
* @param cssValue {CSSUnitValue} | ||
* @param source {DOMElement} | ||
* @param orientation {String} | ||
* @returns {number} | ||
*/ | ||
function calculateMaxValue(cssValue, source, orientation) { | ||
if (cssValue instanceof CSSUnitValue) { | ||
switch (cssValue.unit) { | ||
case 'vw': | ||
return Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0); | ||
case 'vh': | ||
return Math.max(document.documentElement.clientHeight || 0, window.innerHeight || 0); | ||
case 'vmin': | ||
return Math.min( | ||
Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0), | ||
Math.max(document.documentElement.clientHeight || 0, window.innerHeight || 0), | ||
); | ||
case 'vmax': | ||
return Math.max( | ||
Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0), | ||
Math.max(document.documentElement.clientHeight || 0, window.innerHeight || 0), | ||
); | ||
default: | ||
// NOOP | ||
} | ||
} | ||
|
||
return orientation === "vertical" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
? source.scrollHeight - source.clientHeight | ||
: source.scrollWidth - source.clientWidth; | ||
} | ||
|
||
function resolvePx(cssValue, resolvedLength) { | ||
if (cssValue instanceof CSSUnitValue) { | ||
if (cssValue.unit == "percent") | ||
if (['percent', 'vmin', 'vmax', 'vw', 'vh'].includes(cssValue.unit)) | ||
return cssValue.value * resolvedLength / 100; | ||
else if (cssValue.unit == "px") | ||
return cssValue.value; | ||
|
@@ -150,10 +184,7 @@ export function calculateScrollOffset( | |
if (orientation === "block") orientation = "vertical"; | ||
else if (orientation === "inline") orientation = "horizontal"; | ||
|
||
let maxValue = | ||
orientation === "vertical" | ||
? source.scrollHeight - source.clientHeight | ||
: source.scrollWidth - source.clientWidth; | ||
const maxValue = calculateMaxValue(offset, source, orientation); | ||
let parsed = parseLength(offset === AUTO ? autoValue : offset); | ||
return resolvePx(parsed, maxValue); | ||
} | ||
|
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 thesv*
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
Explainer: https://github.com/web-platform-tests/interop-2022-viewport/blob/main/explainers/icb.md ↩
Demo Page: https://interop-2022-viewport.netlify.app/individual/layout-viewport/ ↩
Bug: https://bugs.webkit.org/show_bug.cgi?id=174362 ↩
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.
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
. Evaluatingwindow.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?
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.
Correct.
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.
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/