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-contain-1] Make It easier to use contain: size in cases where the size is unknown #1043

Closed
bmaurer opened this issue Feb 16, 2017 · 13 comments

Comments

@bmaurer
Copy link

bmaurer commented Feb 16, 2017

At Facebook we've been investigating the extent to which we could use CSS containment. Ideally we'd like to provide as much containment as possible on major elements -- like newsfeed stories -- so that we can enable browser optimizations.

However, we've found that one major challenge is that it is often difficult to provide size containment.

For example, let's say there's a newsfeed story that is well above the viewport. If we could provide contain: size on that story we could enable a number of browser optimizations. Specifically the spec mentions that when size is contained browsers can potentially:

When the style or contents of a descendant of the containing element is changed, calculating what part of the DOM tree is "dirtied" and might need to be re-laid out can stop at the containing element.
When laying out the page, if the containing element is off-screen or obscured, the layout of its contents can be delayed or done at a lower priority.

However, feed stories can change height. For example, we have a longpoll that updates comments and could insert new comments in an existing feed story. A user might also interact with a feed story (eg by writing their own comment, thus changing the size). It can be very difficult to track down all the code paths that can modify the story. In addition, if the story is out of the viewport we don't really care what it's height is.

Ideally we'd want behavior along the lines of:

(1) If an element is out of the viewport, cache the size and set the height and width to the cached size, then add contain: size
(2) when the element comes in the viewport, remove the size cache and recompute the size. If the size has changed and the element is above the scrollTop, adjust scrollTop to remove the jank of scrolling.

I've put together a jsfiddle simulating this idea:

https://jsfiddle.net/zLr7o6bt/

This page has a set of feed-like stories. Every 100 ms, 1% of the stories have their content changed (and their text made red for debugability). There is an intersection observer that watches for content going in and out of the viewport. When content goes out it sets contain:size on the element using the existing height, when it comes in that is dropped.

On most browsers scrolling will cause the page to shift around as the height of elements above the viewport changes. But if you use chrome canary which has scroll anchoring it's relatively smooth -- when the size of an element above the viewport is changed the browser changes the scrolltop to compensate.

Why do this in the platform?

  1. The browser has a better sense of how far outside of the viewport layout should be done to enable smooth scrolling.
  2. Scroll anchoring is hard to get right from userspace.
  3. With the polyfill if you scroll too fast you'll see the browser paint elements which used the cached size and will overflow the container. With a browser implementation you would get the same behavior as if you hadn't been using contain

What would the spec look like

I'm imagining you'd say something like:

.card {
  contain: strict;
  width: 500 px;
  height: auto;
  contain: content height-auto;
  height-auto-default: 200px;
}

By adding height-auto to contain it says to the browser that if the card is outside of the viewport, it's height can be cached from a previous layout, or if it was never laid out a default height of 200px can be used.

At any time of the browser's choosing -- but at minimum if any part of the element is in the viewport (using the cached or default size) or if javascript specifically queries the height of the element -- the browser may choose to make the normal height calculation. If it does so, and the bottom of the element was below the scrollTop of its scroll parent then the parent element is adjusted by the change in height.

@flackr
Copy link
Contributor

flackr commented Feb 16, 2017

To simplify the behavior and so that we don't have hysteresis bugs based on what has been previously visible, I would suggest that we always use the automatic default size when elements are outside of the viewport and return that size even if Javascript specifically queries it. As for a spec recommendation, might I recommend

.card {
  contain: paint layout-default; /* contains layout when .card is not visible */
  width: 500px;
  height: auto;
  default-height: 200px;
}

I am worried that this is only useful when combined with scroll anchoring.

@bmaurer
Copy link
Author

bmaurer commented Feb 16, 2017

I see a few potential issues with that:

  1. From the perspective of the layout engine I think we want to avoid resizing the element every time it moves out of the viewport. For example, that would often force the browser to scroll the page firing a scroll event. It'd also change the size of the scrollbars which would be confusing to the user. And you'd have to run resize observer.
  2. It's pretty common to have JS code that queries the height of an element. EG imagine that for logging somebody wants to see the height of feed stories with videos in them. triggering resize on the querying of height makes the element mostly feel like it did before "layout-default"

@bvaughn
Copy link

bvaughn commented Feb 17, 2017

This sounds exciting! When I read the following parts...

when the element comes in the viewport, remove the size cache and recompute the size. If the size has changed and the element is above the scrollTop, adjust scrollTop to remove the jank of scrolling.

when the size of an element above the viewport is changed the browser changes the scrolltop to compensate.

...I think there's some potential for weird scroll UX if scrollTop is adjusted by any significant amount. eg the track thumb could move out from under a user's mouse cursor. Also would want to make sure adjustments don't interfere with momentum scrolling. (Probably goes without saying though.)

@bmaurer
Copy link
Author

bmaurer commented Feb 17, 2017

Any type of infinite scroller is going to jank the track thumb a little bit unless it knows with absolute certainty the size of the elements being loaded.

This will be reduced if the site can come up with a relatively accurate default-height. Also, I could imagine browsers choosing to do things like animate changes to the track thumb.

@frivoal
Copy link
Collaborator

frivoal commented Feb 21, 2017

I'm a bit torn on this.

On the plus side:

1d size containment has been proposed separately for a different reason, but maybe that will work out here as well.

I'm wondering: without explicitly adding to containment the concept of caching sizes when you're off screen, maybe browser engines could still be smart enough to know that if you have: 1d size containment + layout containment + scroll anchoring, then you don't need to recompute the height when you're off screen, as it would not affect the apparent scroll position anyway.

On the negative side:
Regardless of whether we do it as I say above, or with an explicit value, this only seems to make sense if you're assuming 1 column layout. I you have 2 or more columns of content in parallel, the relative height of previous things matters in each column. As the height of things above the viewport change in one column and not in the other, things in the viewport that used to be side by side no longer are. With traditional layout computation, that may be bad UX, but it isn't inherently hard. When you add this ability to not update the height when outside the viewport though, you get some kind of weird stateful layout, where depending on which path you've scrolled through at what time, you may have updated the layout of different elements, resulting in a different global state.

putting it back together:
Thinking of it some more, the problems that happen with multiple columns of content also happen with a single column, if you've got non uniform backgrounds or borders, so we're stuck with this kind of statefull layout problem. I suspect that's not going to be popular with implementers.

It's starting to look as if the polifyl you're using is a pretty good approach after all. Looking back at the downsides:

  • With the polyfill if you scroll too fast you'll see the browser paint elements which used the cached size and will overflow the container. With a browser implementation you would get the same behavior as if you hadn't been using contain.

This would actually be a problem with browsers too, because painting / compositing / scrolling happens on a different thread than layout, so you could still get in situations where you've scrolled faster than you were able to relayout. Granted, that would probably less often since we'd get JS out of the loop, but it could still happen. So maybe from the UX point of view, it could be preferable (for example) to do it in JS, and in addition to size containment, also apply a filter: grayscale(100%) blur(5px) to off screen elements, that you remove only when you're done setting them back to auto size. That way, if the user scrolls so fast that they get some none-updated elements into the viewport they'll look non-updated rather than overflowing and broken. And with contain:paint, the browser would optimize that away for off screen things anyway.

  • On most browsers scrolling will cause the page to shift around as the height of elements above the viewport changes. But if you use chrome canary which has scroll anchoring it's relatively smooth

    &

    Scroll anchoring is hard to get right from userspace.

Hopefully, in due time, all browsers will have scroll anchoring, so this downside will go away.

  • The browser has a better sense of how far outside of the viewport layout should be done to enable smooth scrolling.

Due to the problem of multiple columns or non uniform backgrounds or borders, and because layout and scrolling typically aren't synchronized, I don't think that's actually true, and the browser will struggle as well.

@frivoal
Copy link
Collaborator

frivoal commented Feb 21, 2017

@tabatkins @dbaron What do you think?

@bmaurer
Copy link
Author

bmaurer commented Feb 21, 2017

You make a good point about multicolumn and backgrounds. From a practical standpoint I'm not sure if it's that common to do a multi-column infinite scroll. On facebook we do scroll the left and right columns along with feed but the size of those columns is small in comparison to the feed (which is infinite) and once you hit the bottom of the left and right column we keep it pinned so that the bottom of the column never goes above the bottom of the viewport (essentially we apply position: sticky once you've scrolled to the bottom of that column).

For backgrounds I think this is actually a case where the author likely does not want to make the background jump when above-the-viewport content jumps -- since the sensical use case for a background here is some sort of repeating pattern.

This would actually be a problem with browsers too, because painting / compositing / scrolling happens on a different thread than layout, so you could still get in situations where you've scrolled faster than you were able to relayout. Granted, that would probably less often since we'd get JS out of the loop, but it could still happen. So maybe from the UX point of view, it could be preferable (for example) to do it in JS, and in addition to size containment, also apply a filter: grayscale(100%) blur(5px) to off screen elements, that you remove only when you're done setting them back to auto size. That way, if the user scrolls so fast that they get some none-updated elements into the viewport they'll look non-updated rather than overflowing and broken

This doesn't quite make sense to me. If feed story X, which is off screen, currently has contain:strict and an filter/blur applied to it the browser can choose not to layout X while it's off screen. But once it goes on-screen the browser won't show X until the style inside X is valid, right? Since the browser could choose not to lay out X at all it'd have no way of figuring out what elements were inside without doing the layout. If it does the layout, we might as well show a valid version of X.

My hope here was that through one of the houdini apis that we'd be able to do some sort of experience where elements like X were replaced with a tombstone (IE the placeholder cards we have today) if the user scrolled so fast that the element couldn't be painted -- Similar to the experience in http://googlechrome.github.io/ui-element-samples/infinite-scroller/, but with the houdini apis we could ensure that even if the main thread wasn't responsive that we were able to insert paint the placeholders. But I think that this is an idea unrelated to the idea of css containment.

@frivoal
Copy link
Collaborator

frivoal commented Feb 22, 2017

From a practical standpoint I'm not sure if it's that common to do a multi-column infinite scroll.

I don't think it is common, but as long as it is possible, any in-browser solution has to know how to deal with it. Site-specific JavaScript solutions can work from the assumption that this particular site doesn't have this problems, but browser engines have to deal with the general case.

For backgrounds I think this is actually a case where the author likely does not want to make the background jump when above-the-viewport content jumps -- since the sensical use case for a background here is some sort of repeating pattern.

That seems fairly likely, but here too it's not the only possibility in the general case, and it is tricky to infer intent.

This doesn't quite make sense to me. If feed story X, which is off screen, currently has contain:strict and an filter/blur applied to it the browser can choose not to layout X while it's off screen.

My assumption was that this had been laid out once, frozen in place with a hand-calculated height, and you would gray/blur it out until you've had the chance to update it (or to verify that no update is needed). If you assume it has never been laid out at all, then yes, that doesn't quite work. my assumption was that there would not be such a thing in browser engines as elements that have not been laid out. All elements are laid out, so you can always show them if you want to, but as your js does, we'd turn on strict containment with a manual size to turn off updating the size of the element and relaying out its surrounding when off screen, and flip that when getting in / near the viewport. The costlier thing isn't really (in most cases) to relayout the element itself, but its surroundings when its size changes.

So if you sroll too fast before JS has had a chance to catch up and unfreeze an element, you'd still have something to paint (and if you didn't have an up to date layout, the engine can quickly do one within the existing size), and I was suggesting graying/bluring it out (plus overflow: hidden or overflow:clip) to make it clear to a user who is accidentally seeing it that it's expected for this element to look weird, and that it will get back to normal real soon.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed , and agreed to the following resolutions:

RESOLVED: WONTFIX this level, explain why
RESOLVED: Update WD of css-contain with edits for issues we resolved today
The full IRC log of that discussion
<Florian> topic:
<Florian> https://github.com/w3c/csswg-drafts/issues/1043
<Florian> github topic: https://github.com/w3c/csswg-drafts/issues/1043
<fantasai> Florian: Probably better in 1D context, but still applies to 2D
<fantasai> Florian: Imagine you have long FB page, infinite scroller, 37 screns down
<fantasai> Florian: because everything is dynamic, ppl will update stories above where you are, which will change your size, which will make you scroll
<fantasai> Florian: Things that are off-screen should have their size frozen, and when they enter the screen you should unfreeze them
<fantasai> Florian: So want a variant of size containment that only applies offscreen
<fantasai> Florian: And then when you scroll in it solves
<fantasai> plinss: Sounds like a giant hack that should be solved by scroll anchroing
<fantasai> iank_: FB can also use intersection observer to do this
<fantasai> Florian: That ends up being just slow enough that they do resizing on-screen (rather than just off-screen)
<fantasai> Florian: My take is that once we get scroll anchoring a large part will be solved, combine with fact that you can do manual freezing if you want
<fantasai> Florian: But I think this is not something to solve with magic freezing and unfreezing
<fantasai> fantasai: I agree with the last statement
<fantasai> flackr: If you were to drag the scrollbar, we will rpobably put up a frame before we run any script in response to the scroll position, so you will see unresized content
<fantasai> Florian: which is why I'd suggest there was some UI indication (like graing out) that indicates out-of-date-ness
<fantasai> Florian: Not as good as magic, but dunno how to make that happen
<fantasai> Florian: So proposed wontfix
<fantasai> astearns: Objections?
<fantasai> iank_: We should also said that we'll keep use case in mind for future stuff
<fantasai> TabAtkins: But right now solution is use JS intersection observer
<fantasai> Florian: And various upcoming feaures will make it better (though not perfect) soon
<fantasai> astearns: Can someone add a comment summarizing these conclusions?
<fantasai> RESOLVED: WONTFIX this level, explain why
<fantasai> Florian: That takes us to zero open issues on css-contain
<fantasai> Florian: Modulo DoC, go to CR?
<fantasai> ChrisL: Changes section?
<fantasai> Florian: No, should I do that first?
<fantasai> ChrisL: If you can do it in a rasonable time. Just want to avoid "CR pending edits" which takes 9 months
<fantasai> tantek: Could update a WD now, and the resolve on CR once you have DoC
<fantasai> Florian: Question about DoC...
<fantasai> Florian: Not much happened between FPWD and now, do we need a DoC covering comments from earlier
<fantasai> ChrisL: Assumption in the Process is that FPWD is actually the first *public* working draft
<fantasai> Florian: okay, I'll do DoC over old issues
<fantasai> tantek: Is that needed?
<fantasai> Florian: need to demonstrate wide review
<fantasai> ChrisL: We also need to show usual sec/a11y/i18n reviews
<fantasai> Florian: Will show non-tivial DoC, not necessarily comprehensive to first ED
<fantasai> RESOLVED: Update WD of css-contain with edits for issues we resolved today

@frivoal
Copy link
Collaborator

frivoal commented Apr 19, 2017

@bmaurer : The WG has discussed this, and decided not to add anything to the current specification to address this.

We felt that this addressing this use case properly through the spec would required significant complexity. Among other things, it introduces in dependencies between different parts of the platform (scrolling system, layout system...) that are very unnatural. As we are wrapping up the specification, this isn't something we're willing to dive into, at least for now.

Moreover, scroll anchoring should relieve some of the pain, and implementing this will probably become somewhat simpler when #1031 is solved.

For now, you'll unfortunately need to keep doing this in Javascript.

Closing as WONTFIX, as per the above resolution.

@bmaurer: I realise this is not what you hoped for, but as you raised this issue, can you confirm you're willing to live with this conclusion?

@svgeesus
Copy link
Contributor

svgeesus commented May 8, 2017

Pinging @bmaurer again for confirmation.

@bmaurer
Copy link
Author

bmaurer commented May 9, 2017

I totally get that this doesn't make sense for the current level of the spec -- I do wonder if this bug is an appropriate place (or if there's a better place) to track the issue long term. Containment does seem like it'd be an important part of any platform-level attempt to solve the infinite scrolling problem, since an element must be contained in order for the platform to ignore it during scrolling.

@frivoal
Copy link
Collaborator

frivoal commented May 10, 2017

I totally get that this doesn't make sense for the current level of the spec

Thanks for confirming.

I do wonder if this bug is an appropriate place (or if there's a better place) to track the issue long term.

Totally personal take on this question: while it does seem that there's a genuine use case to solve here, I think there was enough skepticism in the WG (including from myself) of how it's supposed to work that a mere feature request is unlikely to lead anywhere soon, and that a fleshed out proposal would have better chances either to convince the WG that it's doable, or to explore the problem enough to uncover a different way to solve it.

My suggestion would be that you start a draft specification somewhere you control (similarly to WIP proposals from Tab or myself), and keep this group in the loop (either via github or via the [email protected] mailinglist). You could do that in a github issue as well, but I don't find that's a very good place to maintain a specification.

This is not an attempt to make you go away, nor a suggestion that you go incubate it somewhere else. The hard part about this proposal is that it needs to work well with the rest of the layout system, so this is the right group to talk to, but you're going to need to be more specific, and a draft spec could be a good way to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants