-
Notifications
You must be signed in to change notification settings - Fork 83
Conversation
@graynorton @domenic PTAL |
So, I thought the plan was that we were going to make
In all cases the virtual-list always has overflow: auto; you never need to set that yourself. And you're not able to scroll within the main document with From the implementation side, I thought the plan was to basically not change anything about the existing implementation, but rename it to Implementation isn't as important as public API for ALso, this implementation strategy can avoid the limitations section mentioned, which seems pretty important, as those are not great limitations. |
Defaulting these values should be quite straightforward, as we'd need to add these to the :host {
display: block;
/* ... */
width: 300px;
height: 150px;
overflow: auto;
} Re default width/height:
We cannot do this, as we position all the children via position: absolute, and we're not rendering all the elements - we cannot compute the height of the container w/o rendering them all. The user will have to provide this info via style.
The suggested configuration has issues regarding distribution, as from the
<lazy-rendering-viewport>
#shadowRoot
<style> /*...*/ </style>
<slot></slot>
#/shadowRoot
<!-- stamped in the light dom -->
<div>item 0</div>
<div>item 1</div>
...
</lazy-rendering-viewport> Now, if <style> div { color: red } </style>
<virtual-list>
#shadowRoot
<lazy-rendering-viewport>
<slot></slot>
<!-- these are not red! -->
<div>item 0</div>
<div>item 1</div>
...
</lazy-rendering-viewport>
#/shadowRoot
</virtual-list> If
Alternatively, we could add logic in Also, we cannot avoid exposing
The example I mentioned in the description is specifically when the user wants to switch from "lazy-rendering-viewport" mode to "virtual-list" mode, which is very edge case and would be better supported by exposing a property. For more common cases like updating the viewport height (e.g. |
I don't understand the issue about not being able to auto-determine the height or width from the contents. Here is an example of a virtual-list that auto-sizes to its contents width: http://jsbin.com/nagukajega/3/edit?html,output. You can do the same thing flipped for auto-sizing to height.
Again, please take a look at the jsbin for how this is possible. It sounds like a lot of your issues are with the current architecture, e.g. RepeatsAndScrolls, VirtualList, container, etc. Maybe it's time to abandon that architecture if it doesn't help us achieve our goals. I've given a very simple example of the desired result in that jsbin; perhaps we should use that as the basis, instead of the current internals?
The problem is actually that the list has two modes at all. It should not. The lazy-rendering-viewport mode should not be exposed to developers in the same API surface as virtual-list. We can consider building a separate component in v2, but the idea is that virtual-list is only one thing, and lazy-rendering-viewport is a separate thing. |
We cannot auto-determine width/height because we're positioning the children with The only difference between the 2 modes is in the default style, one has an explicit sizing and overflow: auto (virtual-list mode), the other sizes itself and makes a scrolling parent scrollable (lazy-rendering-viewport mode). The drawback of requiring an explicitly sized With the current defaults, the developer would be able to just replace the |
Regarding the implementation, we currently support both modes with the minimal number of DOM nodes, so I'd keep it that way. The only difference between the 2 modes would be these styles: :host {
display: block;
position: relative;
contain: strict;
}
:host(.explicitly-sized-mode) {
overflow: auto;
width: 300px;
height: 150px;
} |
I'd like to see some benchmarks here? Contain in particular makes things slower today. But, we can start with explicitly requiring width/height for now.
But, as we discussed previously, this is an explicit non-goal for the virtual-list project. It is instead a goal for the lazy-rendering-viewport project, which is separate. We should not conflate the two into a single element just because it's easy inside your implementation. It makes for a confusing developer experience and bad API.
That's not the whole story though. The fact that these modes are incompatible, and that you're using styles as a mechanism for switching between them, causes problems of the sort listed in your OP's "limitations", where now the style attribute behaves differently on this element than on any other element in the platform. This two modes need to be separated into two elements, which you can not even attempt to switch between. Otherwise the behavior for the developer is too confusing. It's like |
I don't have any handy benchmark, but in general doing positioning ourselves allows for zero paint/layout cost for items not being recycled during scroll, or when other items are being recycled. In any static/flow/flex layout we'd invalidate the container box and cause unnecessary layout/paint. The two modes can be compatible, we'd have to just expose a With In
In this PR, I was trying to avoid exposing |
Having a default size fixes that. I continue to believe we shouldn't have two modes in a single element. |
Opened #42 which makes |
Closing this in favor of #42 |
Fixes #23.
Introduce
scrollTarget
property inRepeatsAndScrolls
, defaults towindow
:scrollTarget === container
, we size the container via a sizer element, which is inserted in the container and positioned to give the container the correct size and make it scrollable (see example, inspired from https://developers.google.com/web/updates/2016/07/infinite-scroller). For it to work, it requires the container to have its own stacking context (that's already the case since the stamped children need to be positioned absolutely according to the container)container
is sized via min-width/height (as it was done before).This allows
<virtual-list>
to be the scrolling element, or to be contained in a scrolling element other than the main document.<virtual-list>
searches for the scroll target when attached. A scroll target is any element that hasoverflow: auto|scroll
.Added demo:
Limitations
<virtual-list>
will search for the scrolling element only when attached. Stuff like this will not work:The user can still detach/attach the element. Not ideal, but gets the job done.