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?
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.
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 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 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
one doesn’t find
1 finds only , but not
aus doesn’t find , but it finds - it finds when I search for austri
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”…
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.)
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.
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
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.
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.
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.
I am loving the new picker
.
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 
with “thumbs”… It works in the editor’s autocomplete.
, but it finds 
- it finds 
