Customize: Themes search debounce is not debounced properly

[afercia]

Debouncing a search, i.e. not triggering a search while users are still typing the search term, has a few advantages and WordPress is already using this pattern in a few places.

The new Customize Theme search, especially for the .org themes, tries to debounce the search but as far as I see the current code fails to debounce properly. See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/js/customize-controls.js?rev=42025&marks=1837-1846#L1837

This code always calls checkTerm(), the first time is a debounced call and the second time is a direct call.

To reproduce:

• go in the customizer > change theme > WordPress.org themes
• type something in the search field at a normal speed
• observe the search gets triggered for any entered character

The second call to checkTerm() runs always, for each entered character. Also, when typing slowly, checkTerm() is always called twice.

To reproduce:

• dump some console.log() inside checkTerm()
• type 10 characters in the search field
• observe your console dumping your console.log() 10 times + 1 time after 500ms

or

• enter 1 character and wait for at least 500ms
• enter a second character
• observe for each typed character, checkTerm() runs twice

Observing the network panel in Chrome, multiple AJAX requests happen while typing. Seems to me loadThemes() helps a bit reducing the actual number of requests. However, requests should not be sent at all while typing.

Ideally, nothing should happen while typing. The search should be triggered only after users have finished typing and 500ms have passed. As I see it, this is a bug in trunk and should be fixed for 4.9.

Additionally, these kind of searches should be standardized across che admin. This was discussed a few times in the past, see for example https://core.trac.wordpress.org/ticket/37233#comment:21

Ideally:

• a search should be triggered after at least 2 chars have been entered (technically: 2 ASCII chars or one high UTF-8 char)
• entering a space should not trigger a search

Right now, entering a space in the search field triggers a search and a re-rendering of the whole themes list, which seems not so ideal to me even from an usability and user experience perspective.

[adamsilverstein]

Hey @subrataemfluence, would you like to take a look at this?

[westonruter]

I switched from debounce to throttle in [41797] because hitting back from Customizer to themes screen needed to throttle instead of debounce to ensure that the search results are updated immediately instead to prevent a flash of unsearched themes. My intention was that it should do a search with the first keystroke and then will do a subsequent search when the user stops typing. Per Underscore.js ​docs:

By default, throttle will execute the function as soon as you call it for the first time, and, if you call it again any number of times during the wait period, as soon as that period is over. If you'd like to disable the leading-edge call, pass {leading: false}, and if you'd like to disable the execution on the trailing-edge, pass {trailing: false}

But it isn't working as I expected.

See conversation at ​https://wordpress.slack.com/archives/C0381N237/p1509028368000714

[subrataemfluence]

@adamsilverstein Thank you for giving me the opportunity! I would surely take a look tomorrow.

Replying to adamsilverstein:

Hey @subrataemfluence, would you like to take a look at this?

[westonruter]

Sorry, my comment above should have been left on #42348 which is a similar issue.

[adamsilverstein]

I think it should still be _.debounce, with { trailing: false }?

[westonruter]

@adamsilverstein wouldn't that mean the last keystrokes then wouldn't be sent in a request?

[adamsilverstein]

Replying to westonruter:

@adamsilverstein wouldn't that mean the last keystrokes then wouldn't be sent in a request?

Mmmm, I guess you are right. I'll have to test again to see the exact behavior.

[westonruter]

@celloexpressions Can you provide support here?

[celloexpressions]

Remove extraneous call to checkTerm() and trim search term in term change check.

[celloexpressions]

It seems that the second call to checkTerm() was extraneous; I'm not sure why that was ever added. Additionally, we can trim spaces from the input when checking if the term changed to avoid unnecessary reloading. 42343.diff​ should fix this with relatively minimal changes.

As an aside, checkTerm() should probably be renamed before release. maybeInitializeRemoteQuery() or something along those lines would be more appropriate given that it is now only used for searching on remote section instances (it formerly also handled tags and other checks).

[westonruter]

@celloexpressions Searching in the WordPress.org themes section is working properly with debouncing in your patch. However, the installed themes section is not getting debounced searches yet. I think that needs to be done as well, as the installed themes admin screen also debounces searching. The issue is made more apparent when using the ​a11y-speech-synthesis plugin as the browser is continually trying to communicate the number of themes found, but it is doing so too frequently with each keystroke.

[afercia]

Worth considering one more reason to make the local and remote searches behave the same: consistency in the user experience.

While I understand the local search is more a "filtering" of a local collection, producing results at each keystroke would introduce an interaction model that's different from the one needed on the remote search.

#31818 tries to address this issue holistically across the admin, taking into considerations all the (many) different search behaviors in the admin. No great progress there so far, unfortunately. It would be great to revive a bit that ticket and consider further improvements there.

[celloexpressions]

Debounce calls to updateCount when it may be called multiple times quickly.

[celloexpressions]

I don't think that we should change the interaction intent at this point. Instead, the installed filter-search needs to debounce calls to announce the updated count. The bug is with the way the information is presented to screen readers, but it works well for other users without artificially slowing down the experience here.We can also improve the visual experience by not animating the count change until the search is completed. See 42343.2.diff​.

There are fundamental technical differences in these types of sections, and also between this experience and the wp-admin version. The user experience is going to be significantly different between something that needs to reload from Ajax every time a parameter changes and something that filters information that's already displayed - trying to unify those probably wouldn't result in a better experience overall.

[westonruter]

In 42040:

Customize: Debounce requests for theme searches and the updating of the resulting filter count.

Props celloexpressions.
See #37661.
Fixes #42343.