Closed danielbachhuber closed 6 years ago
@WordPress/gutenberg-core Could I get some direction on how you'd like to handle this? It appears hierarchical-term-selector.js
isn't even using withAPIData
to fetch its data.
I see editor/components/post-author/check.js
is beginning to use withSelect
but not (yet?) for author data itself: https://github.com/WordPress/gutenberg/pull/6167/files#diff-4305c6ef4b86dbe9d7b1db481b038b98R35
Because it's a valid expectation to bring all items into the DOM (and not depend on autocomplete or similar)
Playing devil's advocate: What if there are 1000 users? 10,000 users? Is making dozens of API requests when the sidebar opens acceptable?
I'd prefer that we build e.g. a generic component that replaces<select>
and supports paginating through large datasets.
Playing devil's advocate: What if there are 1000 users? 10,000 users? Is making dozens of API requests when the sidebar opens acceptable?
Two points:
We are slowly deprecating withApiData
in favor of wp.data
and select/withSelect
in particular. I thing @aduth and @youknowriad should help to give directions to this one. I think @aduth was looking into something similar last week with tags, where it wasn't possible to pick a tag on sites where there were more than 500 tags.
I don't think we want to continuously load in the background for resources which may or may not ever be needed for how the user plans to interact with the current editor session.
For something like the author selector, I see it working as a component which loads an initial set of options, then more only in response to:
There are a few side considerations here:
/wp/v2/users
)X-WP-Total
response header) the total number of resources. We can use this to simulate a more accurate scrollable sizeYou can see some of these ideas in action here: https://bvaughn.github.io/react-virtualized/#/components/InfiniteLoader
Other prior art:
As far as generalization, I agree concepts of pagination and querying should be built-in to core-data
. There's some related discussion here in #6085. Ultimately I think we could create some higher-order reducers to encapsulate common pagination / querying behaviors (related article).
Other prior art:
For something like the author selector, I see it working as a component which loads an initial set of options, then more only in response to:
What accessibility concerns are there with this approach? cc @afercia
You can see some of these ideas in action here: https://bvaughn.github.io/react-virtualized/#/components/InfiniteLoader
If this proves to be significantly greater level of effort than simply loading all of the user and term data into scope, would you be open to doing the simple implementation first?
It will be a significantly greater level of effort 😄 As for iterations, only my standing hesitation of contentment with mediocrity.
As for iterations, only my standing hesitation of contentment with mediocrity.
I understand. As it exists now though, the current implementation is a blocking bug. Furthermore, I'm concerned there are unknown accessibility issues with the lazy-load approach. I don't know the full history of why Select2 has been so difficult to introduce.
I might imagine if it's considered an in-place substitute for <select>
, there are issues in that pressing e.g. "T" to navigate to an author option "Thomas" would not work as expected if we haven't yet loaded these resources. My thinking would be that there is substitute behavior for this by incorporating the search field, where such an interaction would shift focus back to the search field and trigger the search†.
†Also makes me wonder if we should have such behavior when starting to enter text while navigating listed block options in the inserter.
As for iterations, only my standing hesitation of contentment with mediocrity.
I understand. As it exists now though, the current implementation is a blocking bug.
Sure, this is precisely how mediocre happens. There's a sense of urgency when a thing is completely unusable, as exists here today. This urgency is lost when a "not good, but tolerable" solution is implemented. This is a trade-off, of course; the simplest solution is often more immediately achievable. But it leaves a good probability that it will remain this way forever. I am not saying one way or the other is "right", and is dependent on the circumstances.
I think this is the time where someone chimes in with a favorite mantra of lazy development "perfect is the enemy of good". 😉
Sure, this is precisely how mediocre happens.
Just so it's stated, I'm not going to enter this debate.
Obviously, paginating through the output of these endpoints is a less than an ideal solution.
I'd love to see a lazy loader solution for this, but there have been significant challenges in the past. As @danielbachhuber Select2 was investigated, but it had significant accessibility issues.
Since then, the Woo team have created SelectWoo, which has a focus on accessibility, so may be worth exploring.
That said, investigating either of these will require a significant amount of work to ensure they're a good long term option, I would place this firmly as a stretch goal for 5.0, at best.
In the mean time, I have no problem solving this by automatically iterating over paged endpoints. Particularly with the preloading support added in #6076, I wouldn't expect this to add any significant load to the server, beyond what the Classic Editor already does.
Reiterating my own points from today's discussion in core-editor Slack:
There are multiple things going on in parallel here. I’m not trying to solve them all at once. Things I see off the top of my head:
- We have queryable data, which should be implemented in core-data with a generalized solution across the various resources available. This is what I’m working on.
- We have a number of components which have their own ad-hoc ways of fetching data, such as the author selector. These would need to be ported to take advantage of core-data.
- We have a UI which needs to be present all options in an accessible/usable and bandwidth-efficient manner So updating the author UI to infinitely load options is not strictly in conflict with what I am working on, though may become redundant as a decision can be reached on an ideal UI (or not). Generally speaking, I’m personally motivated to not be loading dozens of pages of data in the background for a UI which may or may not ever be touched by the user in their session. This is not within my immediate scope, but could benefit from it.
I’m also wondering if the infinite loading is as simple as it appears on the surface; how do you inform the user that options are still loading when you’ve only loaded 5 of the 35 API pages of data? Is it prone to the same accessibility issues described in https://github.com/WordPress/gutenberg/pull/5921#issuecomment-383290782 with infinite scroll and recalculating the number of options?
Since then, the Woo team have created SelectWoo, which has a focus on accessibility, so may be worth exploring.
The accessibility team already tested it a few months ago, we were pinged by the developer working on it. Although there are some improvements, it's still not sufficient from an accessibility perspective, especially when it comes to dynamically adding new sets of results.
I’m also wondering if the infinite loading is as simple as it appears on the surface; how do you inform the user that options are still loading when you’ve only loaded 5 of the 35 API pages of data?
I won't repeat the a11y issues already described on https://github.com/WordPress/gutenberg/pull/5921#issuecomment-383290782 but yeah, infinite scrolling is also an usability issue. It gives the illusion to solve a problem, but are we really sure it really does it? A good UI should just give users the necessary information to operate on the UI, not lead to a sense of loss of control, and not cause frustration.
I see infinite scrolling as something that developers like more than users 🙂 It's a pattern that was vastly experimented in the past but today a quick search for infinite scrolling usability
gives tons of articles highlighting its weak points. The fact is, infinite scrolling is not for all users, some will love it others will hate it. There are alternatives. Pagination is the most obvious, and there's an example pattern in WordPress (though it would need a better design):
Thanks for the extra feedback, @afercia!
The variations of this problem are fairly obviously quite difficult, I don't believe that an infinite scroll solution is viable (or necessary) for merge.
After chatting to @danielbachhuber earlier, I believe that a combination of things will provide a reasonable fix:
per_page=-1
, so all of the data is retrieved in one request._fields
parameter, to reduce processing on fields that aren't being returned, improving server response time._fields
. In the case of the /users
request, it retrieves the entire user object, when we really just need the id
and name
. This will significantly reduce bandwidth requirements.I'll take a swing at what it would take to support -1
between now and the weekend, and post with whatever I find then. In the REST API meeting today @TimothyBJacobs raised concerns about the behavior of per_page=-1
on very, very large sites, but this approach still feels the most pragmatic and we can cross the scale bridge once we come to it.
I see infinite scrolling as something that developers like more than users
Would an adequate search/input like an autocomplete not be a more developer friendly solution than paging over data? Doesn't React have an autocomplete frontend component? If there is a component for autocomplete, then is it not a case of defining a custom render & fetch method? (I clearly need to find more time to play with it)
Iterating would take orders of magnitude more time on sites where for example customer
roles are tied to users (WooCommerce). The success of the site would almost become it's downfall if the only pattern for access, means scanning the entire database (even if only in small chunks). It's equivalent to a full-table-scan in database vs an index-lookup.
I Have a customer that has quite a small e-commerce site. They still have > 2500 users (most are customers), which even with a pagination size of 100 entries is 25 scroll interactions, it's 250 at a page size of 10.
I've just tested using the following bookmarklet / console scripts
setInterval(function() { wp.apiRequest({method:'POST',path:'/wp/v2/pages',credentials:true,data:{'title':'REST-gen-'+Date.now(),'content':'empty','status':'publish'}}) }, 1000)
visiting /wp-admin/edit.php?post_status=publish&post_type=page I can see it creates 500+ pages, all published
visiting /wp-admin/post-new.php?post_type=page and running the following content script
[].slice.call(document.querySelectorAll('#inspector-select-control-0 option')).length
shows 101 (no matter how many pages I add.
Fundamentally I'd expect the UI patterns and user-experience for a site with 2500 entries for a dropdown to be different to that of a smaller database with dozens, or hundreds of records to avoid lots of visual scanning over what would be from this image quite busy.
Sites that are even larger, I'd imagine would hit a point, where there were too many pages or posts.
WordPress already lists 1000 authors if you have them, so it wouldn't be a departure from existing behavior.
I was talking about the existing behaviour with @mikelittle at WordCamp London RE: sites with a lot of pages and the pages dropdown in admin (similar issue as getting all users with > 1000) is the page sidebar, where you can select a parent page, which at present from above tops-out at 101 in gutenberg for top-level pages.
It's a problem I've encountered in other software projects. I'll admit I'm lazy up-front, so I always reach for an auto-complete, with select semantics (double-click searches and needs a special case). I'm not sure how it would work with React or WP_Query server-side (mine uses jQuery UI Autocomplete), but I've never had a complaint (scanning is moved to the DB).
As an alternative to only infinite loading, what about skip to specific page using prev-next and a ~random-~ direct paging access input box? (please excuse the fonts, and probably imagine an "of {totalPages})
It doesn't rule-out an infinite loader as well as manual prev-next and paging controls, but the numerical selection could become a browser native number box with minimum, maximum.
@Lewiscowles1986 whilst a possibility I think we need to avoid such complicated visual interfaces as you suggest. That's a lot of things going on in one place. This is where combining autocomplete and infinite scroll, when done well (there are numerous bad examples) can hit the right spot.
It's worth noting autocomplete and suggestions have accessibility benefits, for example those with dyslexia. Where we can limiting the actions you need to do and suggesting ultimately helps everyone.
@karmatosed wondering what you think of the above direct paging access?
Infinite scroll sounds like it's got legs here, I'm trying to understand why besides being able to provide access to all records.
Autocomplete / suggestions and infinite scroll are two very different things. While the first can be made accessible and there are already examples in WordPress and Gutenberg, infinite scroll can't be made accessible for the reasons already explained in https://github.com/WordPress/gutenberg/pull/5921#issuecomment-383290782
@Lewiscowles1986 I think what I am seeing above is confusing as an interface. There's a lot going on there. For example hierarchy, is paging a higher priority than the content itself? How would I also know what page information would be on to add a page number?
Autocomplete / suggestions and infinite scroll are two very different things.
They are but combined they can resolve issues of usability with infinite scroll for example. I am talking not about accessibility in saying that as that obviously is another strong factor to iterate here.
Indeed my efforts were mostly as an alternative to the paging via links above.
How would I also know what page information would be on to add a page number?
I didn't quite understand that. It's not direct access based upon knowledge of page numbers to resources, but a way to bypass iterating over a result set.
Say I click next.
Im not 100% behind it myself, but I use it
I didn't quite understand that. It's not direct access based upon knowledge of page numbers to resources, but a way to bypass iterating over a result set.
In the graphic above the number is an input field, that's why I asked how would you know what input to add. I get there is pagination but if it's an input field that indicates that someone would/could add a number.
The original aim of this issue is no longer valid:
Support for automatically iterating paginated resources in core-data
The more immediate problems have been resolved with #6627 #6657 #6722
We'll bring per_page=-1
to core with https://core.trac.wordpress.org/ticket/43998
I'm open to discussing UI/UX improvements in a new issue if someone cares to forward a proposal.
In cases like #4622, #4022, #5820 and others, UI dependent on
withAPIData
can break when all available items aren't loaded in the UI.For example,
editor/components/post-author/check.js
will only fetch up to 100 users:If there are more than 100 users, the UI currently only displays the first 100.
Because it's a valid expectation to bring all items into the DOM (and not depend on autocomplete or similar), it would be helpful if
core-data
supported automatically iterating paginated resources to bring the entire dataset within scope.