-
Notifications
You must be signed in to change notification settings - Fork 4.6k
DataViews: add support for infinite scroll in table, grid, and list layouts #70955
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
Changes from all commits
b58dce5
a8042d9
5ef7214
8b6f5f4
61736d3
c8f47ab
83c5644
6269827
afc4ac7
c7b4fba
c86bb1c
47dec0d
ac60b59
9a7d7aa
bf8010e
55302da
fd50cfa
be34116
fd42a2c
b1ecc53
074c8ae
3d54442
1a9d90a
07b49dc
0cb6ad2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import { SORTING_DIRECTIONS, sortIcons, sortLabels } from '../../constants'; | |
| import { VIEW_LAYOUTS } from '../../dataviews-layouts'; | ||
| import type { NormalizedField, View } from '../../types'; | ||
| import DataViewsContext from '../dataviews-context'; | ||
| import InfiniteScrollToggle from './infinite-scroll-toggle'; | ||
| import { unlock } from '../../lock-unlock'; | ||
|
|
||
| const { Menu } = unlock( componentsPrivateApis ); | ||
|
|
@@ -102,12 +103,11 @@ export function ViewTypeMenu() { | |
| if ( 'layout' in viewWithoutLayout ) { | ||
| delete viewWithoutLayout.layout; | ||
| } | ||
| // @ts-expect-error | ||
| return onChangeView( { | ||
| ...viewWithoutLayout, | ||
| type: e.target.value, | ||
| ...defaultLayouts[ e.target.value ], | ||
| } ); | ||
| } as View ); | ||
| } | ||
| warning( 'Invalid dataview' ); | ||
| } } | ||
|
|
@@ -215,7 +215,12 @@ function SortDirectionControl() { | |
|
|
||
| function ItemsPerPageControl() { | ||
| const { view, perPageSizes, onChangeView } = useContext( DataViewsContext ); | ||
| if ( perPageSizes.length < 2 || perPageSizes.length > 6 ) { | ||
| const { infiniteScrollEnabled } = view; | ||
| if ( | ||
| perPageSizes.length < 2 || | ||
| perPageSizes.length > 6 || | ||
| infiniteScrollEnabled | ||
| ) { | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -801,6 +806,7 @@ export function DataviewsViewConfigDropdown() { | |
| { !! activeLayout?.viewConfigOptions && ( | ||
| <activeLayout.viewConfigOptions /> | ||
| ) } | ||
| <InfiniteScrollToggle /> | ||
| <ItemsPerPageControl /> | ||
|
Comment on lines
+809
to
810
Contributor
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. 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 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:
Contributor
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. 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.
Contributor
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. Ah, good point! Ignore me then 😄
Contributor
Author
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. Hmm I'm thinking that looks a bit squished 😄
Contributor
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. 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.
Contributor
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. 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.
Contributor
Author
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.
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?
Contributor
Author
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.
Contributor
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. 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’.
Contributor
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. Agree we can approach this separately and certainly systematically, and not necessarily urgently.
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. |
||
| </SettingsSection> | ||
| <SettingsSection title={ __( 'Properties' ) }> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { ToggleControl } from '@wordpress/components'; | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { useContext } from '@wordpress/element'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import DataViewsContext from '../dataviews-context'; | ||
|
|
||
| export default function InfiniteScrollToggle() { | ||
| const context = useContext( DataViewsContext ); | ||
| const { view, onChangeView } = context; | ||
| const infiniteScrollEnabled = view.infiniteScrollEnabled ?? false; | ||
|
|
||
| // Only render the toggle if an infinite scroll handler is available | ||
| if ( ! context.hasInfiniteScrollHandler ) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <ToggleControl | ||
| __nextHasNoMarginBottom | ||
| label={ __( 'Enable infinite scroll' ) } | ||
| help={ __( | ||
| 'Automatically load more content as you scroll, instead of showing pagination links.' | ||
| ) } | ||
| checked={ infiniteScrollEnabled } | ||
| onChange={ ( newValue ) => { | ||
| onChangeView( { | ||
|
Member
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. 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: Screen.Recording.2025-08-06.at.13.20.52.mov
Contributor
Author
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. That was an issue with the storybook implementation, not the toggle control 😅 |
||
| ...view, | ||
| infiniteScrollEnabled: newValue, | ||
| } ); | ||
| } } | ||
| /> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,12 +9,12 @@ import type { ReactNode, ComponentProps, ReactElement } from 'react'; | |
| import { __experimentalHStack as HStack } from '@wordpress/components'; | ||
| import { | ||
| useContext, | ||
| useEffect, | ||
| useMemo, | ||
| useRef, | ||
| useState, | ||
| useEffect, | ||
| } from '@wordpress/element'; | ||
| import { useResizeObserver } from '@wordpress/compose'; | ||
| import { useResizeObserver, throttle } from '@wordpress/compose'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
|
|
@@ -51,6 +51,7 @@ type DataViewsProps< Item > = { | |
| paginationInfo: { | ||
| totalItems: number; | ||
| totalPages: number; | ||
| infiniteScrollHandler?: () => void; | ||
| }; | ||
| defaultLayouts: SupportedLayouts; | ||
| selection?: string[]; | ||
|
|
@@ -143,6 +144,7 @@ function DataViews< Item >( { | |
| perPageSizes = [ 10, 20, 50, 100 ], | ||
| empty, | ||
| }: DataViewsProps< Item > ) { | ||
| const { infiniteScrollHandler } = paginationInfo; | ||
| const containerRef = useRef< HTMLDivElement | null >( null ); | ||
| const [ containerWidth, setContainerWidth ] = useState( 0 ); | ||
| const resizeObserverRef = useResizeObserver( | ||
|
|
@@ -193,6 +195,33 @@ function DataViews< Item >( { | |
| } | ||
| }, [ hasPrimaryOrLockedFilters, isShowingFilter ] ); | ||
|
|
||
| // Attach scroll event listener for infinite scroll | ||
| useEffect( () => { | ||
|
Contributor
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.
|
||
| if ( ! view.infiniteScrollEnabled || ! containerRef.current ) { | ||
| return; | ||
| } | ||
|
|
||
| const handleScroll = throttle( ( event: unknown ) => { | ||
| const target = ( event as Event ).target as HTMLElement; | ||
| const scrollTop = target.scrollTop; | ||
| const scrollHeight = target.scrollHeight; | ||
| const clientHeight = target.clientHeight; | ||
|
|
||
| // Check if user has scrolled near the bottom | ||
| if ( scrollTop + clientHeight >= scrollHeight - 100 ) { | ||
| infiniteScrollHandler?.(); | ||
| } | ||
| }, 100 ); // Throttle to 100ms | ||
|
|
||
| const container = containerRef.current; | ||
| container.addEventListener( 'scroll', handleScroll ); | ||
|
|
||
| return () => { | ||
| container.removeEventListener( 'scroll', handleScroll ); | ||
| handleScroll.cancel(); // Cancel any pending throttled calls | ||
| }; | ||
| }, [ infiniteScrollHandler, view.infiniteScrollEnabled ] ); | ||
|
|
||
| return ( | ||
| <DataViewsContext.Provider | ||
| value={ { | ||
|
|
@@ -221,6 +250,7 @@ function DataViews< Item >( { | |
| setIsShowingFilter, | ||
| perPageSizes, | ||
| empty, | ||
| hasInfiniteScrollHandler: !! infiniteScrollHandler, | ||
| } } | ||
| > | ||
| <div className="dataviews-wrapper" ref={ containerRef }> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,58 @@ export const data: SpaceObject[] = [ | |
| }, | ||
| { | ||
| id: 4, | ||
| title: 'Ganymede', | ||
| description: 'Largest moon of Jupiter', | ||
| image: 'https://live.staticflickr.com/7816/33436473218_a836235935_k.jpg', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Jupiter', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2022-01-04', | ||
| datetime: '2022-01-04T12:30:00Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 5, | ||
| title: 'Callisto', | ||
| description: 'Outermost Galilean moon of Jupiter', | ||
| image: 'https://live.staticflickr.com/804/27604150528_4512448a9c_c.jpg', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Jupiter', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2021-01-05', | ||
| datetime: '2021-01-05T14:15:30Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 6, | ||
| title: 'Amalthea', | ||
| description: 'Small irregular moon of Jupiter', | ||
| image: 'https://upload.wikimedia.org/wikipedia/commons/6/62/Amalthea.gif', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Jupiter', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2020-01-06', | ||
| datetime: '2020-01-06T10:45:15Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 7, | ||
| title: 'Himalia', | ||
| description: 'Largest irregular moon of Jupiter', | ||
| image: 'https://upload.wikimedia.org/wikipedia/commons/c/c2/Cassini-Huygens_Image_of_Himalia.png', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Jupiter', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2019-01-07', | ||
| datetime: '2019-01-07T16:20:45Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 8, | ||
| title: 'Neptune', | ||
| description: 'Ice giant in the Solar system', | ||
| image: 'https://live.staticflickr.com/65535/29523683990_000ff4720c_z.jpg', | ||
|
|
@@ -92,7 +144,46 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 5, | ||
| id: 9, | ||
| title: 'Triton', | ||
| description: 'Largest moon of Neptune', | ||
| image: 'https://live.staticflickr.com/65535/50728384241_02c5126c30_h.jpg', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Neptune', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2021-02-01', | ||
| datetime: '2021-02-01T11:30:00Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 10, | ||
| title: 'Nereid', | ||
| description: 'Irregular moon of Neptune', | ||
| image: 'https://upload.wikimedia.org/wikipedia/commons/b/b0/Nereid-Voyager2.jpg', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Neptune', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2020-02-02', | ||
| datetime: '2020-02-02T15:45:30Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 11, | ||
| title: 'Proteus', | ||
| description: 'Second-largest moon of Neptune', | ||
| image: 'https://live.staticflickr.com/65535/50727825808_bf427e007b_c.jpg', | ||
| type: 'Satellite', | ||
| isPlanet: false, | ||
| categories: [ 'Solar system', 'Satellite', 'Neptune', 'Moon' ], | ||
| satellites: 0, | ||
| date: '2019-02-03', | ||
| datetime: '2019-02-03T09:20:15Z', | ||
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 12, | ||
| title: 'Mercury', | ||
| description: 'Terrestrial planet in the Solar system', | ||
| image: 'https://live.staticflickr.com/813/40199101735_e5e92ffd11_z.jpg', | ||
|
|
@@ -105,7 +196,7 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 6, | ||
| id: 13, | ||
| title: 'Venus', | ||
| description: 'La planète Vénus', | ||
| image: 'https://live.staticflickr.com/8025/7544560662_900e717727_z.jpg', | ||
|
|
@@ -118,7 +209,7 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 7, | ||
| id: 14, | ||
| title: 'Earth', | ||
| description: 'Terrestrial planet in the Solar system', | ||
| image: 'https://live.staticflickr.com/3762/9460163562_964fe6af07_z.jpg', | ||
|
|
@@ -131,7 +222,7 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 8, | ||
| id: 15, | ||
| title: 'Mars', | ||
| description: 'Terrestrial planet in the Solar system', | ||
| image: 'https://live.staticflickr.com/8151/7651156426_e047f4d219_z.jpg', | ||
|
|
@@ -144,7 +235,7 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 9, | ||
| id: 16, | ||
| title: 'Jupiter', | ||
| description: 'Gas giant in the Solar system', | ||
| image: 'https://staging-jubilee.flickr.com/2853/9458010071_6e6fc41408_z.jpg', | ||
|
|
@@ -157,7 +248,7 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 10, | ||
| id: 17, | ||
| title: 'Saturn', | ||
| description: 'Gas giant in the Solar system', | ||
| image: 'https://live.staticflickr.com/5524/9464658509_fc2d83dff5_z.jpg', | ||
|
|
@@ -170,7 +261,7 @@ export const data: SpaceObject[] = [ | |
| email: '[email protected]', | ||
| }, | ||
| { | ||
| id: 11, | ||
| id: 18, | ||
| title: 'Uranus', | ||
| description: 'Ice giant in the Solar system', | ||
| image: 'https://live.staticflickr.com/65535/5553350875_3072df91e2_c.jpg', | ||
|
|
||






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.
Weird that this error disappeared when I added
infiniteScrollintolayoutin the several views, and reappeared when I moved it out. But I think this solution is nicer than the expect error comment.