Emoji picker search performance

When using the emoji picker on mobile devices, it can be quite sluggish when typing into the search box. Waiting a bit of time for results isn’t a problem, but something seems to be impacting the performance of the <input> field itself.

The easiest way to repro is using the CPU slowdown options in the chrome dev tools. It seems to be worse when pressing the backspace key. Here’s a video - I pressed backspace 5 times in quick succession, and you can see that the text in the input field disappears sporadically. (6x CPU slowdown in chrome dev tools, to exaggerate the problem)

It looks like there is already some debounce logic here, so I’m not sure if there’s an easy solution…

https://github.com/discourse/discourse/blob/5a715b7d55f443e9783e34609f0a39c2a0de86fb/app/assets/javascripts/discourse/components/emoji-picker.js.es6#L89-L93

5 Likes

Maybe @joffreyjaffeux can take a peek.

1 Like

never experienced this, will try to reproduce and concatenate the string in a better order. :grin:

1 Like

I gave that a try out of curiosity and, indeed, I can reproduce the issue in Chrome with CPU throttling as @david mentioned. For me, replacing this line:

https://github.com/jorgemanrubia/discourse/blob/5a715b7d55f443e9783e34609f0a39c2a0de86fb/app/assets/javascripts/discourse/components/emoji-picker.js.es6#L220

With:

this.$list.css('visibility', 'hidden');

That fixed the problem for me. I know that visibility: hidden is more performant than display: none because it won’t cause a reflow, but no idea about why that is having such impact here (maybe this old chrome bug is still around?). Also, I don’t know if the visibility: hidden will introduce other unwanted side effects, but seems to work fine.

It was not a JQuery’s hide() thing. this.$list.css('display', 'none') was as slow as this.$list.hide().

10 Likes

seems like a right path, I will investigate monday if we can’t do anything better and if it has implications. thx.

5 Likes

I just merged a fix (still awaiting deploy on meta):

https://github.com/discourse/discourse/commit/52fbf9d3aea16b1ff05c9b4c29bb253084ee427c

Thx @david and @jorge_manrubia.

Now more details on this:

  • I follow your advice and went for visibility. I applied it explicitly when making it visible too.
  • I was also thinking of the initial thought of david: “there’s already debouncing…” So I bumped the debouncing timeout on mobile a little bit, my thought behind this being there’s probably more delay when you backspace on mobile than on desktop, so the chances you trigger multiple rendering when you just want to delete multiple chars were higher on mobile.
  • I also took this opportunity for a small refactoring and changing the modal positioning on mobile, I actually made it look like it’s a panel at the top, I think it’s a good improvement and should avoid it from going under keyboard, let me know what you think:

7 Likes

This topic was automatically closed after 30 hours. New replies are no longer allowed.