-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Suggest a rootMargin strategy for lazy-loaded elements #5917
Conversation
the following suggestions to consider:</p> | ||
|
||
<ul> | ||
<li><p>The value is greater than or equal to 1250px.</p></li> |
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.
What is the rationale behind the value 1250px?
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.
It's the smallest value that Chromium uses. They experimented with 750px but found it was too low. See #5408 (comment)
We could specify a different value, or not suggest any particular value. However, what I'd like to avoid is having some browsers use a very small value (like 1px or 50px) which makes the feature annoying for users and causes web developers to avoid using native lazy loading.
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.
Maybe this could be goal-based instead, like saying there should be a minimum value that most often results in images being loaded before they intersect the viewport under normal usage patterns for the given device. This could be quite different for a watch compared to a smartphone, for example (because smaller screen and maybe slower scrolling).
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.
Right, I was thinking this should be carefully worded as we can have many different form factors (watch is a good "extreme" case).
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.
c1c7848
to
a9f9c1d
Compare
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.
LGTM - I like the wording here. I agree that having a goal-based root margin description is probably best here.
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.
Maybe this could be goal-based instead, like saying there should be a minimum value that most often results in images being loaded before they intersect the viewport under normal usage patterns for the given device.
I concur with this approach of being goal-based in thinking about how to approach the rootMargin strategy too. LGTM.
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.
Editorially LGTM. I'm not sure whether we should wait for more review (e.g. from other implementers), or go ahead and merge, since it's just adding suggestions. I'll leave that up to you, @zcorpan.
Thanks, @scott-little and @addyosmani! @domenic, let's give this a day or two for review from others. I can merge on Thursday if there are no objections or new feedback. |
a9f9c1d
to
6e8abfe
Compare
See #5408 (comment)
Fixes #5408.
(See WHATWG Working Mode: Changes for more details.)
/urls-and-fetching.html ( diff )