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

Paginated virtual scrolling menu #1010

Closed
Tracked by #780
tomdye opened this issue Jan 6, 2020 · 6 comments
Closed
Tracked by #780

Paginated virtual scrolling menu #1010

tomdye opened this issue Jan 6, 2020 · 6 comments
Assignees

Comments

@tomdye
Copy link
Member

tomdye commented Jan 6, 2020

Enhancement

Paginated virtual scrolling menu

We have recently added the ability to create a virtual scrolling menu which is used in select / menu / typahead (coming soon) etc...
To further enhance this we should make it paginated, for example if there are 100,000 items to display, the server may not be able to return all of these at once so we will need to request them once they are scrolled into view.

The first part of this has been done already by adding virtual scrolling and a total property.
This addition will make it easier to do server-side typeahead queries and to work with massive data sets.

Example

let options = [a,b,c];
<Menu 
   options={options}
   total={100}
   loadMore={() => {
      fetch().then(moreOptions => {
         options = [
            ...options,
            ...moreOptions
         ];
      })
   }}

Challenges

Ease of use

Using the menu in a paginated fashion should be simple and straightforward for our users. Furthermore it should be possible for all menu-using widgets, ie. select / typeahead etc to be be paginated.

Jumping to the end of the menu

Jumping to the end of the menu via the end key will skip the middle pages (if they exist) and load the final page. If using a single options array for this we may need to pad out the interim pages somehow.

Keeping the API consistent

In implementing this enhancement, we do not wish to change the existing APIs as much as possible. For example, we would ideally like to keep the options as a single array.
We will need to add a loadMore callback and pageSize property to the menu, these should both be required when either is used. This should be typed appropriately.

Loading indicators

Whilst data is loading we should render a placeholder to indicate to the user that data is loading.

Starting with no options

The menu should be capable of initialising with no options or total specified. When this occurs, the menu should fetch the first page automatically.

Requesting pages once

We should ensure that the menu only calls out to load missing pages once. This will stop multiple requests from firing and reduce unnecessary callbacks.

Options data changing completely

In the case of a typeahead for example, the options may change completely, ie. the search params could change. We must ensure that when this occurs, subsequent pages will be requested correctly, ie. any flags to indicate they have been fetched should be removed.

@tomdye tomdye added the dojo-7.0 label Jan 6, 2020
@tomdye tomdye self-assigned this Jan 6, 2020
@tomdye tomdye mentioned this issue Jan 6, 2020
47 tasks
@tomdye
Copy link
Member Author

tomdye commented Jan 9, 2020

Adding pagination to the virtual scrolling via middleware and new properties

I have put together a POC for this using a pagination middleware and some small changes to the underlying menu.

The new properties added to the menu are

  • loadMore?(page: number): void
    • This is called by the menu if it attempts to render an item and the array entry in options for that item is null.
    • This will also be called by the menu for page 1 if options is an empty array and a loadMore property has been passed
  • pageSize?: number
    • This is used to determine which page needs to be requests, ie. if option 31 is null and the pageSize is 10, page 4 will be requested.

This approach works with the existing options array but requires the user to pad the options array with null for unloaded pages. This can be made more straight forward by providing a paginatedMiddleware as shown here:

export const paginatedMiddleware = middlewareFactory(({ middleware: { icache } }) => {
	return ({ fetcher, pageSize }: { fetcher: (page: number, query?: string) => Promise<{ data: MenuOption[], total: number}>, pageSize: number }) => {

		async function loadMore(page: number) {
			const requestedPages = icache.getOrSet<number[]>('requestedPages', []);
			if (requestedPages.indexOf(page) < 0) {
				icache.set('requestedPages', [ ...requestedPages, page]);
				let currentOptions: MenuOption[] = [...icache.getOrSet('options', [])];

				const { data, total } = await fetcher(page);

				if (!requestedPages.length) {
					currentOptions = Array(total).fill(null);
				}
				const beforeSize = (page - 1) * pageSize;
				let before = currentOptions.splice(0, beforeSize);
				let after = currentOptions.splice(pageSize);
				icache.set('total', total);
				icache.set('options', [ ...before, ...data, ...after]);
			}
		}

		return {
			loadMore,
			options: icache.getOrSet('options', []),
			total: icache.getOrSet('total', 0)
		};
	};
});

This middleware is instantiated with a fetcher and a pageSize and it's functions then passed down to the menu widget

const factory = create({ icache, paginatedMiddleware });
export default factory(function LargeOptionSet({ middleware: { icache, paginatedMiddleware } }) {
	const pageSize = 20;

	async function fetcher(page: number) {
		const response = await fetch(`https://some.endpoint?page=${page}`);
		const json = await response.json();
		return {
			data: json.items,
			total: json.total
		}
	}

	const { loadMore, options, total } = paginatedMiddleware({fetcher, pageSize});
	return (
		<Menu
			options={options}
			onValue={(value) => {
				icache.set('value', value);
			}}
			total={total}
			itemsInView={10}
			loadMore={loadMore}
			pageSize={pageSize}
		/>
	);
});

This approach appears to work well with paginated endpoints / fetchers and the virtual scrolling menu

@tomdye
Copy link
Member Author

tomdye commented Jan 9, 2020

Making the pagination work with a query string

The above POC works well for pagination where the options object does not change, ie. the result set is static. The difficulty lies when a filter / query is added. This will be problematic when implementing a paginated typeahead widget.

I have worked to amend the above POC to work with a query string by adding a setQuery function to the middleware which then passes the query onto the fetcher.
When setting a query string, the requestedPages are cleared which ensures that the options object is reset and padded appropriately with the total returned from the first fetch.

This appears to work as expected with filterable, paginated APIs.

The adjusted middleware would look as follows:

export const paginatedMiddleware = middlewareFactory(({ middleware: { icache } }) => {
	return ({ fetcher, pageSize }: { fetcher: (page: number, query?: string) => Promise<{ data: MenuOption[], total: number}>, pageSize: number }) => {

		async function loadMore(page: number) {
			const requestedPages = icache.getOrSet<number[]>('requestedPages', []);
			if (requestedPages.indexOf(page) < 0) {
				icache.set('requestedPages', [ ...requestedPages, page]);
				const query = icache.get<string>('query');
				let currentOptions: MenuOption[] = [...icache.getOrSet('options', [])];

				const { data, total } = await fetcher(page, query);

				if (!requestedPages.length) {
					currentOptions = Array(total).fill(null);
				}
				const beforeSize = (page - 1) * pageSize;
				let before = currentOptions.splice(0, beforeSize);
				let after = currentOptions.splice(pageSize);
				icache.set('total', total);
				icache.set('options', [ ...before, ...data, ...after]);
			}
		}

		return {
			loadMore,
			setQuery(query?: string) {
				const currentQuery = icache.get('query');
				if (query !== currentQuery) {
					icache.set('query', query);
					icache.set('requestedPages', []);
					loadMore(1);
				}
			},
			options: icache.getOrSet('options', []),
			total: icache.getOrSet('total', 0)
		};
	};
});

When using this with the menu, the only change is that you need to call setQuery on the middleware when the query string changes, this in turn clears the existing pages and loads the first page.

const factory = create({ icache, paginatedMiddleware });
export default factory(function LargeOptionSet({ middleware: { icache, paginatedMiddleware } }) {
	const pageSize = 20;

	async function fetcher(page: number, query: string) {
		const response = await fetch(`https://some.endpoint?page=${page}&query=${query}`);
		const json = await response.json();
		return {
			data: json.items,
			total: json.total
		}
	}

	const { loadMore, options, total, setQuery } = paginatedMiddleware({fetcher, pageSize});
	return (
		<virtual>
			<Menu
				options={options}
				onValue={(value) => {
					icache.set('value', value);
				}}
				total={total}
				itemsInView={10}
				loadMore={loadMore}
				pageSize={pageSize}
			/>
			<TextInput onValue={(value) => {
				icache.set('textvalue', value);
				setQuery(value);
			}} value={icache.get('textvalue')}/>
		</virtual>
	);
});

@tomdye
Copy link
Member Author

tomdye commented Jan 9, 2020

Problems with this approach

When using paginated data, we do not have all of the items loaded into the options array, this causes alphabetical search via a-z key presses to fail so unloaded items. As a work around, we could ensure this works for all non-null items. Pressing home / end keys still work effectively to skip to the beginning / end of the results.

@tomdye
Copy link
Member Author

tomdye commented Jan 9, 2020

Follow up questions for discussion

  • If we decide to follow this approach, do we split menu into Menu / PaginatedMenu implementations
    • The required properties for either approach are different, granted, this can be dealt with using a union of the two appropriate property sets as the MenuProperties.
    • The difficulty will come when we get further down the line of Typeahead etc. For example, a non-paginated typeahead could offer a default filter functionality via an array filter and provide a property the user can use to pass their own filter function. However, a paginated typeahead would need to provide this functionality purely via it's loadMore / fetcher as the filtering would be part of the system providing both the options and the pagination.
    • We could probably get away with leaving the Menu as a single widget for now and look to split out the more complicated composite widgets when we implement them, ie. Typeahead / PaginatedTypeahead etc.

Update:

This should be possible without splitting the widgets into paginated / non-paginated versions. We can use a type union to specify the menu properties.
ie. interface TypeaheadProperties = PaginatedProperties | NormalProperties

@tomdye
Copy link
Member Author

tomdye commented Jan 21, 2020

Demo of paginated menu here:
https://dist-ioknyzbmlv.now.sh/#widget/menu/example/largeoptionset

Note: filtering via the text box only works right now when you're scrolled to the top

@tomdye
Copy link
Member Author

tomdye commented Mar 19, 2020

Closing this discussion issue, issue for changes pending implementation here: #1162

@tomdye tomdye closed this as completed Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant