New Emoji picker feedback


(Sam Saffron) #1

@joffreyjaffeux just merged in the brand new emoji picker: :sparkler::fireworks::ribbon::rice_scene::heavy_heart_exclamation:

This topic is for tracking feedback:

Open issues:

  • Clicks are be re-bound causing a single click to add multiple emojis (filter to a word, clear filter, filter again, click emoji)

  • Clicking anywhere outside the picker should close the picker.

  • Some of the CSS is impacting non-emoji stuff … like having a round hover on search results

  • Recently used emojis logic is too fancy, is should just push recently used to the first in the list

  • It would be nice if the picker moved with the composer at the same time when resizing

  • This visual issue on composer is new

- Use SASS variables for styling so it works on the Dark Theme (and all color only themes)

  • Picker moves to modal based on width alone, sometimes there is plenty of height for it, modal should be last resort.

  • Clicking inside picker but not on an emoji closes the picker

  • Debouncing image load is not quite right, I still see square blanks even when images are on disk, so its replacing the “loading” too late.


Hosted Discourse Version?
(Sam Saffron) #2

Also a quick note to say :star_struck: I am loving the new picker

:balloon::tanabata_tree::revolving_hearts::love_letter::dizzy:


(Robin Ward) #3

It’s looking rad! I reviewed the code locally and here are some notes:

d-editor.js.es6

  • You should declare emojiPickerIsActive: false at the top of the component. Javascript is a lot faster when it knows the shape of objects in advance.

emoji-picker.js.es6

  • resolveTemplate calls should not happen in the main scope of the file. This might lead to confusing results when the resolver/app has not booted properly, or if a plugin wants to change something after you’ve already looked it up. I’d recommend instead looking up the template when you need it.

  • There are a bunch of constants that seem to be exported just so the acceptance test can clear them out. Instead, why not export a method called resetCache that will do the clearing? That way the test doesn’t need to know how the component is implemented. Also, exporting less things is generally better.

  • We discussed this briefly, but is there any way the @observes can be replaced with callbacks from the ember component lifecycle? If not that’s cool, I just want to know if you looked into it.

  • Why does _later exist and avoid doing async work in testing? Also, I suspect later is the wrong callback here. Perhaps you want Ember.run.scheduleOnce('afterRender', ...)?

  • unbindEvents seems to be doing a lot less than bindEvents – are we sure every event has been removed?

emoji-picker-raw.hbs.erb

  • If you use Emoji.url_for in the template, doesn’t that bake the current emoji set into the template? What happens if the emoji set changes and the template has already been baked like in production mode? I guess what I’m asking here is are we sure that changing the emoji set still works in production mode with this code?

(Rafael dos Santos Silva) #4

The new picker is amazing, great job @joffreyjaffeux :tada:.

What do you guys think about removing some borders on the search area?

Before | After


(Rafael dos Santos Silva) #5

Yes, this would be great.

Also:

  • If the user is using the recent emoji area to insert an emoji, don’t update it, so the hovered emoji don’t change on click.

  • If the user has no recent emoji, the first emoji use makes the entire list scroll down, so I lose my position and hovered emoji. If possible, we want to keep the scroll position.


(Gerhard Schlager) #6

The new emoji picker looks fantastic. Great work @joffreyjaffeux!

Here’s a list of things I noticed:

  • After reloading the page and clicking on the emoji picker toolbar icon the picker appears below the editor before it moves to the top. I think this is related to the “visual issue” @sam mentioned.
  • Clicking on the skin tones doesn’t seem to have any effect after clicking. I know that it’s reloading all the emojis in the background, but I think there should be some kind of visual feedback that something is actually happening.
  • Maybe even hide those skin tones behind a :gear: icon and add a label so that everyone knows that those are skin tones. I think this is a set-and-forget option that shouldn’t always be visible. It uses valuable space - some emoji names like :woman_factory_worker: can’t be displayed completely.
  • How does search work? Is it searching only for emoji names? Does it include aliases? I’d like to be able to search for :+1: with “thumbs”… It works in the editor’s autocomplete.
  • Search doesn’t always find the emoji I’m looking for. Examples:
    • + doesn’t find anything. I would expect :+1:
    • one doesn’t find :one:
    • 1 finds only :+1:, but not :-1:
    • aus doesn’t find :austria:, but it finds :australia: - it finds :austria: when I search for austri

(Joffrey Jaffeux) #7

Thx for the feedback, much appreciated.

Just a quick note on search, it will indeed be improved in the future, at the moment it’s a very basic regex on the main emoji list. Ideally we would also want to have some kind of keywords, like “joy”, “face”…


(Sam Saffron) #8

Not a huge fan of that, I think people will get used to it being there. Forcing people to one shade of brown runs counter to my philosophy :older_man:t5::older_man:t2:


(Stefano Costa) #9

Absolutely fantastic :mage:


(Eli the Bearded) #10

:broken_heart:

I want to like it, but I’m finding it painfully slow. On the old picker, I’d see blank white space while the icons loaded. Here I’m seeing broken image icons until they finish loading.

I’m guessing that what you are doing is loading all of the emoji images. Pages that make lots of simultaneous requests always cause me problems when I’m on wifi. What constitutes “lots” can vary, but all of the icons would do it.

The previous picker loaded them in batches, which was slow but not very bad.

(If it matters, Firefox 54 on Chrome-book quality laptop running Linux, namely Ubuntu 16.04.)


(Joffrey Jaffeux) #11

Yes I’ve seen this since it’s on meta, I think there’s actually another issue. As a point of comparison, twitter emoji picker is loading 1500 emojis from the start, while I’m only loading emojis, section by section. So we are loading it in batches.

You can actually launch the network inspector, and you will see that we are not loading all of them.


(Eli the Bearded) #12

Maybe it is just that the batches are bigger now? Old one had a paged interface, and loaded page at a time, you are loading group at time?

(The twitter one sounds terrible as you describe it. I’ve been read-only on Twitter for a few years now and have not had the pleasure of using it.)


(Joffrey Jaffeux) #13

if it wasn’t clear from my first answer, I do appreciate this feedback. And please trust me, making it blazing fast is a major concern for me, so we will get there if we are not yet :wink:


(Joffrey Jaffeux) #14

The thing is, it’s a tradeoff between preloading before people scroll on fast connections, or wait more before preloading for slower connections. In one case we make fast connections wait on the other we overflow slow connections.


(Joffrey Jaffeux) #15

I think we get the issue with @sam there’s an nginx config issue, once solved, it should be faster than flash. :running_woman:


(Joffrey Jaffeux) #16

I applied a fix which will reset src before applying new src, so you will see this when you change diversity level:

I think it should be enough to emphasise that we are loading new emojis.


(Sam Saffron) #17

I fixed the caching!

Some of the debouncing is not quite right, I still see square boxes even once all the images are cached on disk. It should be a lot more aggressive when stuff is on-screen.


(Joffrey Jaffeux) #18

I deployed multiple fixes which should greatly improve the search experience. @elijah could you give it a new try?


(Eli the Bearded) #20

Still slow, but not as ugly while it fills in. I cropped these, but copied overbits of my system toolbar.

The last used emoji was there immediately. The left hand column took 10 seconds or so to fill in, and more than 30 for the whole pane here to fill in.

Yellow bar is battery, next box is network, this didn’t show noticeable strain on the wifi, next chart is CPU, which also stayed flatish. Then timestamp.


(Joffrey Jaffeux) #21

did you scroll during that time ?