Selecting another theme feature filter requires to wait until a previous request has finished

[ocean90]

Steps to reproduce:

• Go to the new themes panel in the customizer ([38813])
• Open the network tab in your browser dev tools
• Select multiple feature filters at once
• Inspect the Ajax request
• You'll notice that it only includes the first tag.

I think there should be a button which inits the Ajax request.

[celloexpressions]

I wonder if debouncing the requests (more - it should be already debounced some) might help? The buttons were removed for a more direct flow I believe but maybe we need to bring one back, cc @folletto.

[folletto]

I wonder if debouncing the requests (more - it should be already debounced some) might help?

For that UI, that's the right approach. I would try to avoid to have the button back because it would break the UX in yet another exception.

[ocean90]

I'm not sure if debouncing is going help us here. Let's say I want to filter for "Grid Layout" (first item) and "Portfolio" (last item), this would require a debounce time of 3+ seconds.

[afercia]

Debouncing always implies an assumption about the delay amount that should be introduced. Users use the mouse or the keyboard (or touch) at different speed. Some users have impairments that force them to use such devices very slowly. So what should be the delay exactly? In any case, it would be an arbitrary choice.
On the other hand, blocking the UI when a request is running would be a bit tedious. A more traditional interaction (with a button) would solve the issue but I understand the concern about visual and UX. Not sure about the best option here. It's something that happens in other parts of the admin too and maybe would need a more general discussion and approach.

[helen]

I'm inclined to agree with a button for applying a set of filters - that's generally the practice around core, and I find it to be both a more performant experience in terms of data going back and forth and less jumpy.

[celloexpressions]

If we're going to go back to having an apply button, we need a design for the button and also whether we're providing a summary of the filters as the original UI did. The button could maybe go next to the feature filter section header. I have no ideas for summarizing the selected filters to apply given the current structure of this.

This is a consequence of the redesign to diverge from the admin theme installer UI; accordingly we'll need a new UI pattern for this.

[folletto]

I think I'm missing something here: why are we discussing about delays and apply buttons?

The issue above is that any filter request done while the admin-ajax.php is running gets ignored. That's a bug, and has to be fixed.

The simple fix is to just queue requests. Debounce is a queue with an additional delay to avoid quick bursts of multipl calls, but it's still a queue (well, it should, I'm aware that implementations can differ here), as such, will still solve the issue on multiple calls.

I'd note that this fix is still very relevant even if we chose to roll back having an Apply button (which I think we shoulnd't), because it would also handle multiple "Apply" triggers that happen if the API call is still happening.

Can we implement a proper debounced queue so all the calls are handled properly?

[afercia]

Replying to folletto:

Can we implement a proper debounced queue so all the calls are handled properly?

Basically, there's no "proper" way :) An arbitrary delay would fit for some users, and wouldn't for others. See also discussion on #36202 and #35273.

[folletto]

Thanks for the tickets, which confirm what I was mentioning about a button not being a solution to the issue.

Also, the issue I see on #36202 seems similar to what I'm saying here: there's a discussion on debounce, but not on queuing. Debounce is an optimization on top of queuing, not in substitution of it, because as you said without queuing just the debounce will still cause errors on one side or the other of the delay.

Basically, there's no "proper" way :) An arbitrary delay would fit for some users, and wouldn't for others.

IMHO there's a proper way, which is both queue + debounce. The queue addresses the issue of multiple requests (and we should discuss the logic about it) and debounce optimizes the requests the queue is going to do.

Let's put the "delay" point on the side for a moment, since it's not the primary issue here. The issue is that the Ajax call is blocking, thus: we need to fix the queuing first.

---

So, here's Draft 1 of the logic:

• Filter toggled → the request is done with a debounce to 500ms (we can tweak this once we test it live)
• If the user toggles another filter within the threshold (500ms), then debounce takes care of it.
• If the request is sent, and the user toggles another filter during the request, the new call is queued (1).
• If the request is sent, and the user toggles another filter during the request, the new call is queued (2).
• Once the request is returned, the UI updates, and the next request in the queue gets processed (1)

This is probably not ideal on slow connections due to the implied delay of multiple checks stacking up, so we could get around it with a "wait slot" (or "queue of one", there's probably a better name but my memory is failing me) that holds always the last available request. Draft 2:

• Filter toggled → the request is done with a debounce to 500ms (we can tweak this once we test it live)
• If the user toggles another filter within the threshold (500ms), then debounce takes care of it.
• If the request is sent, and the user toggles another filter during the request, the new request is added to the wait slot.
• If the request is sent, and the user toggles another filter during the request, the old wait slot request is discarded, and the new request is placed in the wait slot.
• Once the request is returned, the UI updates, and whichever is in the wait slot gets sent.

Thinking about it, always reflecting on potential slow connections, the logic could maybe be even simpler if we can cancel the running request. This optimizes for responsiveness at the risk of more API requests. However, the debounce should avoid a too high ratio of these, so it might be good. Draft 3:

• Filter toggled → the request is done with a debounce to 500ms (we can tweak this once we test it live)
• If the user toggles another filter within the threshold (500ms), then debounce takes care of it.
• If the request is sent, and the user toggles another filter during the request, the request is cancelled and the new request is sent.
• If the request is sent, and the user toggles another filter during the request, the request is cancelled and the new request is sent.
• Once the request is returned, the UI updates.

Whichever we decide here, should be useful also on the other ticket, so maybe we can think of it of a more general pattern that can be reused.

I hope it helps. :)

[celloexpressions]

It makes sense to me to add a way to queue up a "next request" that fires as soon as the current Ajax call is returned, and contains the most recent set of filters at any given time (maybe call the variable pendingRequest, that part isn't too important). The current theme-loading logic is built primarily for infinite-scroll, with things like the feature filter added later on top of that, so there wasn't a need for a queue concept initially (and we also had the apply button before the redesign).

Canceling the current request would be the best approach, but I'm not sure whether that's feasible here from a technical perspective. It would presumably happen on the JS side. We should also keep in mind that these queries go to WordPress.org, so we can anticipate them taking a bit longer than a regular Ajax call that goes to the site's server and right back without that added step.

Assigning to myself to work on at the end of the week; if anyone else is able to put together a patch before that please go ahead, and please also confirm that everyone's okay with keeping the existing UI as-is and queuing the latest request to update after a current call is returned to fix the bug here.

[celloexpressions]

If there's a more recent term than the one in the results, re-run the query with the new term.

[celloexpressions]

38365.diff​ ensures that results are always shown for the most recent term. An easy way to test is by entering an invalid username for the favorites section, then correcting it to a user with favorites and pressing enter again before the initial query stops loading - it should seem to continue loading until it shows the results for the corrected username.

This approach allows the existing UI to function as intended.

[helen]

Have to close this as invalid since the feature isn't in core. Referenced back on #37661.