I'm really not sure if the semantics here are correct: when groupByField is enabled, this means there will be multiple feed role groups on the page. Should we just disable infinite scroll when grouping by field?

Good question. I suppose semantically it'd be the containing VStack that would get the "feed" role if there should just be the one?

Should we just disable infinite scroll when grouping by field?

From my perspective it would make sense to start out not supporting "group by" for now, and return to adding it further down the track if it's needed? Aside from the semantics of the feed role, it does feel like the group by case is a slightly different UX from scrolling through a single list 🤔

When switching it off, we also need to set the page we are in and make sure we only list the perPage number of items (not all of them)? Otherwise, it's like nothing happened:

That was an issue with the storybook implementation, not the toggle control 😅
It should be fixed in 8804ac9.

I wanted to try the "loading" experience. Layouts need to combine it with "has data":

• when there is no data, display a spinner instead like they do (e.g., table)
• when there is actually data but is still loading: what do we do? display a "loading item" at the end of the list?

Additionally, what do we do about the existing pagination? The current state is that we display the last page that has been fetched. And interacting with it doesn't do anything. Perhaps we hide it if infinite scroll is on?

And interacting with it doesn't do anything. Perhaps we hide it if infinite scroll is on?

Interacting does something, but I don't think it's expected. The page jumps when you select pages after all have loaded, and the n items changes.

I wonder if infinite scroll needs some sort of custom footer, e.g., how many have been loaded and/or selected.

Otherwise it's working nicely for me.

Additionally, what do we do about the existing pagination? The current state is that we display the last page that has been fetched. And interacting with it doesn't do anything. Perhaps we hide it if infinite scroll is on?

Yeah I think we just hide the pagination bit.

how many have been loaded and/or selected

This bit is working fine so we can leave it as is for now.

when there is no data, display a spinner instead like they do (e.g., table)

When would we have no data? If the server doesn't respond, shouldn't we display an error message in that case?

Ok I think I get it. When the data is slow to load on the page, the spinner displays. That'll happen for the initial data whether infinite scroll is enabled or not.

If we're loading additional data with infinite scroll, that's where we might display a "loading items...". I can have a play with that and test it on the Pages screen. It might need some design input though.

I think you could remove the useMemo here, and instead have the boolean calculated inline.

We can if we assume that view.layout?.infiniteScroll will only exist if the infiniteScrollHandler also exists. Which it should! I might have been overly cautious here.

useRefEffect could be an option here instead of useEffect, but tbh, it's a very minor difference. Only advantage is you wouldn't need to check containerRef.current is set.

I wonder if the value should only be passed value when isInfiniteScroll is true, because when regular pagination is active it'll be providing the incorrect value (e.g. on the second page it'll start from 1 again).

Hmm if you're planning to use posinset when infinite scroll is disabled, then yes that would make sense to do. Otherwise in this PR posinset is only rendered when infinite scroll is on.

Defo a nitpick. To me, invalid data is passed to GridItem, and then we're trusting that the component does the right thing with that data. It's not an issue with the current code, but code changes, and some future person might miss the subtlety.

I've found a bit of a problem with the aria attributes on the list layout: they don't play well at all with the Composite component we're using. I've removed them from list for now, though I haven't disabled infinite scroll on it.

Composit is built on ariakit and the error occurs on that side. Basically any posinset value passed to Composite.Row makes it explode 😅

😬 What's the error?

Might be worth checking the issues for ariakit to see if anyone has had the same - https://github.com/ariakit/ariakit/issues. I searched for posinset and saw nothing though.

Not sure if you tried it, but you could try setting the prop on the <div /> that the Composite.Row renders.

ReferenceError: Cannot access 'rowId' before initialization which is coming from the ariakit component. I haven't really looked into it.

Adding it to the div works 🎉 I also had to add aria-setsize to the div otherwise it ends up attached to the button inside.

I also had to update the CSS selectors directly targeting [role="row"] to target [role="article"] too.

Interesting. I think I'll need a different role for the data picker changes. I'm not sure whether article supports selection. Will need to figure out if infinite scroll can work for a picker or if that's an invalid combination.

I'll also need posinset so it's useful to establish it here.

That's good question. Aside from whether the roles will work or not, personally I think I'd prefer to infinite scroll through a large picker than to navigate pagination, but it's not a great solution for accessibility so it shouldn't be the default mode. Does it makes sense to have view config options in a picker scenario?

Does it makes sense to have view config options in a picker scenario?

Hard to say, possibly a smaller subset of options. But maybe it's not worth differentiating in that case and instead providing all the options.

Edit: Dan raised a good point that my suggestion below should not be followed!

Just thinking out loud, but the toggle control looks quite wide in its current implementation. Would it be worth conditionally wrapping these two controls in an HStack (like the sort and direction controls) when infinite scroll is present? (I imagine we'd need to add a context.hasInfiniteScrollHandler check here if that's the case?)

Here's how it's looking now:

But I was wondering if it'd be a bit more efficient use of space if we did something like this:

IMO that makes Items Per Page look too confined. It can also have up to 6 items specified, so while it has 4 in that screenshot it's not a true reflection of the worst case.

Ah, good point! Ignore me then 😄

Hmm I'm thinking that looks a bit squished 😄
@jameskoster @jasmussen do you have opinions?

The appearance dropdown works decently as is, and yet it can benefit from a revisit given how it's organically grown. But part of that is also reflecting on: what should be here? IMO, and I feel this somewhat strongly, this dropdown should not hold a toggle for infinite scroll.

Infinite scroll has pros and cons. If implemented right, for extremely large amounts of data, such as your entire media library archive of tens of thousands, it can be an excellent way to provide you access to the entire silo of content in one view. When paired with other date-scrolling or letter-jumping tools, it can be one of the best ways to find exactly what you're looking for, in a way that maintains performance.

It's also got plenty of downsides. You can't do an in-page search for something that hasn't loaded yet, and there are some assistive tech challenges as well. I recognize those are largely mitigated by the presence of the pagination control, as well as the "Items per page" choice. In most cases, however, those controls are mutually exclusive: either you have infinite scroll, or you have n items per page.

I'm not saying infinite scroll can't be combined with an items per page choice, but it is unusual. And unlike posts per page which is commonly a per-view relevant choice, infinite scroll feels more like a set it and forget it choice; presumably you either want it everywhere, or nowhere. To frame it as an exercise, if we presumed all of WordPress used dataviews for list views, would you set infinite scroll for some, but not other screens?

For that reason, I'd put infinite scroll as a choice in a global preferences screen, and have it apply to all. Such as Settings > General.

I recognize that's non-trivial as far as progressing the dataviews component, so I could see it temporarily living in this dropdown, if we were to agree to eventually move it to a global place.

Although I feel somewhat strongly about this, it's also not a hill I'll do battle on, and if there's consensus otherwise that infinite scroll should be a per-view setting that's tied to or adjacent to posts per page, I would approach it as a toggle button not quite unlike how Figma does it for their auto-layout wrap button:

That is, keep the segmented control (or dropdown if need be) for posts per pages, and attach a toggle button to the right of it with a tooltip. This is in part to reduce the prominence of the button.

The two problems I have with the above is that 1) it gives huge prominence to something I'm not sure should have prominence, and secondarily that the panel as a whole is already huge:

The design can probably work if it moves to a Settings > General page eventually. There's space to be verbose and contextualize there.

I don't want to block this work from moving forward, but the two pieces of followups to look into are just that: moving this to a settings page at some point, and overall looking at whether we can make this list appearance control more dense/compact/glancable. Those two tasks may in fact be the same, I have this vague feeling that most of the "set and forget" settings can live elsewhere.

The design can probably work if it moves to a Settings > General page eventually.

One thing we need to be aware of regarding infinite scroll as a global setting is that it has to be implemented on a per-screen basis. Dataviews only provides an event listener that the screen using it can attach a handler to. Even if we add infinite scroll handlers on all the core screens using dataviews, it's up to plugin developers to do it on their screens, so the global setting may only work for a subset of admin screens. Is that acceptable?

OK I've updated the PR to try out what Jay suggested above:

I've also centered the "loading more results" text:

(that text should only appear over a slow connection)

Does this look OK for now?

For me it’s an acceptable starting point from which to refine.

I agree the view options dialog is beginning to feel cumbersome in some circumstances, but I see that as a separate issue that we should approach systematically.

A global setting could makes sense, but ultimately this is a property of each individual dataview so the local option would likely need to remain regardless. I could see a global setting working similarly to ‘Default post category’.

Agree we can approach this separately and certainly systematically, and not necessarily urgently.

A global setting could makes sense, but ultimately this is a property of each individual dataview so the local option would likely need to remain regardless. I could see a global setting working similarly to ‘Default post category’.

Honestly this depends for me. In both Proton Drive, Dropbox, and Google Drive, there's not a choice to turn off or on infinite scroll, or even choose items per page. And it feels plenty, honestly. I recognize that doesn't map 1:1 with what DataViews is doing, because it's also handling column visibility properties. So I'm not suggesting we remove these. Perhaps it's just that the cog feels very prominent, and the dropdown it opens is substantial, so perhaps when we get to it changing the cog to an ellipsis or something ligther, and working on the density of the controls inside, is plenty.

Good question. I suppose semantically it'd be the containing VStack that would get the "feed" role if there should just be the one?

Should we just disable infinite scroll when grouping by field?

From my perspective it would make sense to start out not supporting "group by" for now, and return to adding it further down the track if it's needed? Aside from the semantics of the feed role, it does feel like the group by case is a slightly different UX from scrolling through a single list 🤔

Weird that this error disappeared when I added infiniteScroll into layout in the several views, and reappeared when I moved it out. But I think this solution is nicer than the expect error comment.