Opened 13 years ago
Last modified 3 months ago
#19867 assigned enhancement
wp_dropdown_users() still not scalable
Reported by: | prettyboymp | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 3.3.1 |
Component: | Users | Keywords: | needs-patch needs-screenshots |
Focuses: | ui, accessibility, performance | Cc: |
Description
#14572 made huge improvements to the performance of wp_dropdown_users(), however, on certain sites that have an unusually large set of authors, wp_dropdown_users() still isn't usable. It either causes a memory error on the server, or potentially crashes the client browser due to content size.
Attachments (11)
Change History (122)
#2
@
13 years ago
attachment:ticket:19867:user_dropdown_filter.diff is alternative patch to at least allow a plugin to override the user dropdown before any queries are run.
#3
@
13 years ago
Just realized that my first patch didn't include the supporting js. Added attachment:ticket:19867:user_autocomplete.2.diff with it included.
#4
@
13 years ago
+1 on the filter.
Additional +1 on autocomplete after a certain number of users. Since we are doing autocomplete for sites/users in the network admin for 3.4, the bits can likely be shared quite nicely.
#13
@
12 years ago
What's the status on this? Says targeted to 3.3.1 but it doesn't look like these changes ever made it through. I can help test if there's an approved patch for testing.
#14
follow-up:
↓ 15
@
12 years ago
Version is when something was reported/requested or the earliest a bug can be reproduced. Milestone is for the target, as it were.
Could probably use a patch that implements autocomplete the way it's done now for the network admin after a certain number of users. There's wp_is_large_network( 'users' )
to check for a large number of users but the default of 10k is probably still way too many for a dropdown.
#15
in reply to:
↑ 14
@
12 years ago
- Cc kovshenin added
- Keywords needs-refresh added
Replying to helen: I think wp_is_large_network
is actually used to prevent autocomplete because such searches would be really slow on large databases. Here's site-users.php:
if ( ! wp_is_large_network( 'users' ) ) wp_enqueue_script( 'user-suggest' );
We can probably just reuse the whole thing as is, with a different ajax callback.
#16
@
12 years ago
- Cc mpvanwinkle77 added
So here's a new approach. creates a new function wp_suggest_users ... allows for several criterion in wp_dropdown_users to initiate a bail to wp_suggest_users() ... minor updates to js and to wp-admin/includes/post.php required to make this work. There's still a lot to do to make this production ready, but wanted to get some general feed back on the approach before continuing.
#20
@
11 years ago
Anyone ever get a chance to look at this? Is this the route we want to go? or is there a better strategy?
#24
follow-up:
↓ 25
@
11 years ago
- Component changed from Performance to Users
- Focuses performance added
- Milestone changed from Future Release to 3.9
Could we use the same user autocomplete we used in the network admin?
#25
in reply to:
↑ 24
@
11 years ago
Replying to nacin:
Could we use the same user autocomplete we used in the network admin?
I think so - two things occur to me that are probably different, though. It needs the ID as the value, as opposed to user_login in the current instances or user_email as in the patch on #25348, and the UI needs to show something more friendly than the ID and should prevent freeform entry.
Also, the patch on #25348 whitelists the field against an enumerated list (in_array) - should we keep expanding on that list, make it filterable, something else?
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
11 years ago
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
11 years ago
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
11 years ago
This ticket was mentioned in IRC in #wordpress-dev by DH-Shredder. View the logs.
11 years ago
#32
@
11 years ago
- Milestone changed from 3.9 to Future Release
I dove way down, and didn't come back up in time for 3.9. Here's where I landed (with help from gcorne). tl;dr: Autocomplete needs to enhance the right UI for the given data, whether that's a text input, a select element, or something else.
We have two instances of jQuery UI autocomplete in core right now, both in multisite. One is for adding existing users to a site (either from the network admin or the individual site admin) and the other, added this cycle, is for the new site admin email (#25348).
In exploring what it would take to allow wp_dropdown_users()
(and wp_dropdown_pages()
in #9864) to use this same method in order to scale, I realized that a freeform text input as the non-JS part of the UI was not actually ideal, and in fact, incorrect. If you test some of the patches I've been periodically putting onto the ticket for progress tracking, you'll see that I implemented a rough alternate UI, but too late in the cycle to do proper design and testing.
What I also realized is that the existing usages are actually two different things, and one of them uses a misleading UI. For a new site admin email, you can enter any email address and it will create a new user if that user doesn't already exist. The "autocomplete" in that case is more of a "prompt" or "suggest" - much like tags. However, when adding an existing user to a site, the UI really should only allow you to select an existing user, and not allow for arbitrary input. Essentially, this latter case should operate much as a select element would. Server-side catches it, of course, but we should have better UI.
I still want to push forward on approach and aim for early next release cycle. I'm thinking there are a few aspects:
- What we use as the JS helper for autocomplete. jQuery UI autocomplete with a custom set of skins, something else? Either way, we should definitely use our own JS wrapper so that switching libraries or maintaining shims is a little more future-proofed (see: Masonry).
- The UX and design of said helper.
- Being smart about offering up some initial options to speed things up (more of a nice-to-have enhancement than a necessity), e.g. 10 users with the most posts for author selection dropdowns.
#34
@
10 years ago
Yes, having initial options would be nice. Solution needs to work on large-author sites (we have a site with 30,000 authors, and some of the backend screens for admin are now unstable in Chrome, constantly crashing the browser). Thanks.
This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.
10 years ago
This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.
10 years ago
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
10 years ago
This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.
10 years ago
This ticket was mentioned in Slack in #core by drew. View the logs.
10 years ago
#43
@
10 years ago
Hi - I know this is well underway but thought I'd put in a quick request just in case.
Would it be possible to hook the wp_dropdown_users select and eventually the autosuggest to an action that we could remove or add to? It would be really helpful to be able to completely remove that action and add my own - not just filter the wp_dropdown_users query, although that would be cool, too.
What I've done for users who want to be deleted is assign their content to a dummy user creatively named "_deleted account" (the underscore was to pop it to the top of the dropdown list). However, now I can't use the list (85k users) although it still loads and only breaks if I touch it.
To work around this, I've put a checkbox on the new and edit users pages that allows me to include them in a list of users to whom it's ok to reassign content. I keep this list as an array in wp_options so I don't have to hunt through either of the users tables piecing it together. I'm currently using this array to replace the options in the wp_dropdown_users select using jQuery.
Having a small curated list really works for me. Putting users on it might take a tiny bit more time but then once I do, I never have to remember again who I am using for deleted user accounts. And I don't have to worry about me or anyone else accidentally assigning content to the wrong person (disastrous! or maybe hilarious, it would depend...)
This ticket was mentioned in Slack in #core by boone. View the logs.
10 years ago
This ticket was mentioned in Slack in #design by helen. View the logs.
9 years ago
#46
@
9 years ago
19867.patch is a solution that assigns the deleted user's content to user ID 0. It is a suitable workaround, but not widely tested, so not suitable as a full solution without more testing.
#48
@
9 years ago
I've just run up against this issue on a site where I've imported ~250,000 users. In general all is working fine except I cannot delete a user due to the "Attribute all content to:" wp_dropdown_users
running out of memory.
All these users have a custom role which only has the read
capability, so it seems to me that they shouldn't really be listed in the dropdown anyway?
#51
@
9 years ago
I have run into this issue on a site with approx. 210,000 users. Only 30 or so non-subscribers, but delete page is running out of memory and the dropdown and 'Confirm Deletion' button do not appear. Is this marked to be looked at (and hopefully fixed) in a future release?
This ticket was mentioned in Slack in #design by tharsheblows. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by drew. View the logs.
9 years ago
@
9 years ago
abstracts the args being passed to wp_dropdown_user
and filters them before being passed into the query
#55
@
9 years ago
As a stopgap / workaround, I've added a patch that will set the args outside of the query and filter before being passed into the wp_dropdown_users
function itself. this will allow site users to define things such as specific user roles or total counts (and other things, for that matter)
#56
follow-up:
↓ 57
@
9 years ago
Such filter would be best inside of wp_dropdown_users
. There is currently a filter called wp_dropdown_users
which filters the HTML output. A new one wp_dropdown_users_args
which filters the arguments would make sense.
#57
in reply to:
↑ 56
@
9 years ago
Replying to paulwilde:
Such filter would be best inside of
wp_dropdown_users
. There is currently a filter calledwp_dropdown_users
which filters the HTML output. A new onewp_dropdown_users_args
which filters the arguments would make sense.
I see both sides of it. I've added an additional patch that filters the args inside of wp_dropdown_users
instead, I'm fine with either being implemented.
@
9 years ago
filters the args inside of wp_dropdown_users
instead of just targeting the delete portion
#58
@
9 years ago
19867-04.diff would work brilliantly for me as it makes targeting just that instance of wp_dropdown_users easier and clearer. I use a different set of users in the reassign dropdown on the delete page than for eg the author select in posts.
But either would work, so please yes to one of them.
This ticket was mentioned in Slack in #core by drew. View the logs.
9 years ago
#61
@
9 years ago
- Keywords needs-refresh removed
Following [34692], it was pointed out that the 'name' argument isn't actually available for targeting specific instances of wp_dropdown_users()
. This is because we're currently only passing $query_args
to the hook. Access to 'name' means we also need to pass the actual default arguments.
19867.4.diff adds $r
(the default arguments) to the hook.
#62
@
9 years ago
Filtering the args for the query seems correct to me - it just needs the context of the original args that were passed to the function.
#64
follow-up:
↓ 65
@
9 years ago
- Milestone changed from 4.4 to Future Release
The addition of the wp_dropdown_users_args
filter is a good start, but I don't see us necessarily solving this in 4.4. Moving back to future release.
#65
in reply to:
↑ 64
;
follow-up:
↓ 66
@
9 years ago
Replying to DrewAPicture:
The addition of the
wp_dropdown_users_args
filter is a good start, but I don't see us necessarily solving this in 4.4. Moving back to future release.
The new filter solves the immediate problem that site owners face. While a deep dive into the query behind the scenes could be warranted, I still don't see how having this filter is a bad thing at all, especially given that we already filter the HTML output inside the same function.
#66
in reply to:
↑ 65
@
9 years ago
Replying to norcross:
Replying to DrewAPicture:
The addition of the
wp_dropdown_users_args
filter is a good start, but I don't see us necessarily solving this in 4.4. Moving back to future release.
The new filter solves the immediate problem that site owners face. While a deep dive into the query behind the scenes could be warranted, I still don't see how having this filter is a bad thing at all, especially given that we already filter the HTML output inside the same function.
It's not a bad thing. It's just not a complete fix :-)
This ticket was mentioned in Slack in #core by pento. View the logs.
9 years ago
#68
@
8 years ago
Any reason why @prettyboymp's fix with autocomplete wasn't included in any realease for 4 years? This issue is literally breaking large sites (30k+ users) and is present in non-network installs as well...
#69
@
8 years ago
- Type changed from defect (bug) to enhancement
This is still a problem as of 4.6 and will become more and more of a problem as WooCommerce proliferates and uses Users the same way for Customers. Sure, we might only have a dozen authors for marketing or whatever but it doesn't take long to rack up 10s of thousands of customers and crush that Select dropdown. It almost certainly needs switched to AJAX.
Thanks guys, you're heroes.
This ticket was mentioned in Slack in #bbpress by netweb. View the logs.
8 years ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
7 years ago
#74
@
6 years ago
Just wanted to point out that there's a fork of Select2 that fixes some of the a11y problems, in case anyone is interested in contributing to it, and then working to get those changes merged upstream.
#75
@
5 years ago
- Focuses accessibility added
I requested @prettyboymp post this ticket 7 years ago. we have > 275,000 users. still a problem. now we don't even bother and do direct SQL on the production DB cluster. not a great practice but this is functionally unusable for us.
#77
@
4 years ago
Re-reading the thread history in better detail that replied I posted made no sense.
Removed it, and apologies for the intromission.
Will watch the ticket in case an opportunity to help comes up.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
22 months ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
19 months ago
#83
@
19 months ago
- Keywords needs-patch added; has-patch removed
- Milestone changed from Future Release to 6.3
We're going to take another stab at this for 6.3.
#84
@
18 months ago
Now that #40613 is in core, I wonder if this ticket is valid. User Query queries now cache, so would help this a lot.
#85
follow-up:
↓ 86
@
18 months ago
It’s still valid. There are still major in-browser performance and usability problems when loading user select inputs with 10,000 values - especially in contexts like the importer, that could be loading each select dozens of times on the page.
#86
in reply to:
↑ 85
@
18 months ago
Replying to joedolson:
It’s still valid. There are still major in-browser performance and usability problems
Yea, in my opinion the browser/UI/UX problems have always been much larger than the PHP or DB problems, and much harder to fix. Seems the only possibilities would either be autocomplete or some sort of "advanced search" popup/modal. Any type of "drop-downs", even multi-row, seem impossibly hard to use when the entries are somewhere over 50-ish, no matter how you look at it.
#87
follow-up:
↓ 88
@
18 months ago
What do you think about using ComboboxControl as an autocomplete? It'd probably be easier to iterate on compared to Select[2|Woo], since it's first-party code.
IIRC, in the past we've used a single instance of an object when many are needed on the same page, and then swapped the data on the fly (maybe with TinyMCE)? Would that be helpful here?
#88
in reply to:
↑ 87
@
18 months ago
Replying to iandunn:
What do you think about using ComboboxControl as an autocomplete?
Yep, this looks really good imho, and meets the accessibility requirements I think.
...then swapped the data on the fly
Think one of the features of the ComboboxControl is that the (previously used) data is stored locally and can be reused by all instances. So perhaps the drop-down part of all instances can be updated with the new(er) entries after every search (still need to experiment with how that would "feel" but think it would be nicer than showing some default data everywhere).
#89
@
16 months ago
- Milestone changed from 6.3 to 6.4
This is an enhancement, and we are in 12 days until Beta 1 after which we will not add new enhancement to the release, so, because there is no patch so far, I am moving this ticket to 6.4.
By the look of the amount of watching ticket (they are not getting updates yet if they didn't participate in the discussion) this is a fascinating enhancement. And it seems that general solution is founded, so, if you want to work on this interesting ticket, plan it for the patch to be in the trunk until Beta 1 of the 6.4 release 🙌
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
15 months ago
#93
@
15 months ago
I'd support using ComboBoxControl for this. I agree that using a first-party component gives us a lot more flexibility on iteration.
#94
@
14 months ago
- Keywords needs-screenshots added
@joedolson if the decision is to use ComboBoxControl, what needs to be done next, do we need help from the Editor developers?
This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.
14 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
14 months ago
#97
@
14 months ago
Somebody comfortable with React probably needs to step in to move this forward. I don't really know what would need to happen for that component to be used elsewhere in core. Somebody from the Editor team would definitely be helpful.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
13 months ago
#99
@
13 months ago
- Milestone changed from 6.4 to 6.5
This is considered an enhancement, and we are currently in the week leading up to Beta 1. After this point, no new enhancements will be added to the release. Therefore, since there is no patch available at the moment, I am moving this ticket to milestone 6.5
.
If anyone believes it's ready for 6.4
, please feel free to move it back to that milestone.
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
13 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
12 months ago
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
12 months ago
#103
@
12 months ago
- Milestone changed from 6.5 to Future Release
- Owner joedolson deleted
- Status changed from accepted to assigned
During routine ticket refinement for the 6.5 release, we noticed that this ticket continues to get moved from release to release. Based on @joedolson's last comment, I'm going to move this to Future Release and un-assign, so someone else can pick this up. If an approach is decided, this can always come back into the milestone.
attachment:ticket:19867:user_autocomplete.diff is a suggested solution using an autocomplete handling to replace the select box after a threshold of results is hit.
As an alternative, I'm working on a second patch that will just be a tweak to add a filter that will a plugin to add this functionality before the dropdown is generated, in case a built in solution isn't accepted.