Closed amitava82 closed 2 years ago
Hey, @amitava82 . Weird, I can't replicate - but I have a theory. I believe that the search results show up immediately, but their images take more time to load since they were not preloaded. Does that seem like the case?
It would look somewhat like this: The search results actually do take up space (you see that the food & drink category is showing and has a height) but not all images loaded yet - even though they are hoverable.
If that's the case, I would recommend trying to preload all emojis so that won't happen on slow connections:
<Picker onEmojiClick={onEmojiClick} preload/>
No, it is not that. It is the keystroke that is laggy. On each keystroke it takes couple of seconds to update input because some reason thread is being blocked.
This module suffers of (extremly) poor performance because memoization issue. One possible fix :
First wrap the component into a memoized component :
const MemoEmojiPicker = React.memo(EmojiPicker);
Then use it like this :
// ...
const handleEmojiClickRef = useRef();
const handleEmojiClickFix = useCallback((...args) => handleEmojiClickRef.current(...args), []);
useEffect(() => { handleEmojiClickRef.current = handleEmojiClick; });
return (
// ...
<MemoEmojiPicker onEmojiClick={handleEmojiClickFix} />
// ...
);
This not fixes the inner searchbox but it prevent some unwanted rerendering.
Hope this helps
Hey, @hugomallet. Unfortunately I don't think it is that simple. After many spending quite a while investigating the render and performance issue I can confidently say that it is not related to memoization, but to the sheer amount of elements to render at once.
What would probably help solving this would be some kind of a virtualized list - a feature I am planning to add sometime in the future.
In 3.2 I touched a little on performance. Nothing huge, but it should slightly improve initial load time and also the full render after clearing the search term - these are the two main bottlenecks.
I will working on perf in a later version.
The demo app throws error after typing few characters. Error in /turbo_modules/emoji-picker-react@3.2.0/dist/index.js (2:155784) Cannot read property 'length' of undefined
Thanks, looking into it.
On Tue, Jun 2, 2020, 04:43 Amitava Saha notifications@github.com wrote:
The demo app throws error after typing few characters. Error in /turbo_modules/emoji-picker-react@3.2.0/dist/index.js (2:155784) Cannot read property 'length' of undefined
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ealush/emoji-picker-react/issues/133#issuecomment-637217452, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACV32P5BB26BP73YQJRKRW3RURKLLANCNFSM4J2HGUOQ .
This module suffers of (extremly) poor performance because memoization issue. One possible fix :
First wrap the component into a memoized component :
const MemoEmojiPicker = React.memo(EmojiPicker);
Then use it like this :
// ... const handleEmojiClickRef = useRef(); const handleEmojiClickFix = useCallback((...args) => handleEmojiClickRef.current(...args), []); useEffect(() => { handleEmojiClickRef.current = handleEmojiClick; }); return ( // ... <MemoEmojiPicker onEmojiClick={handleEmojiClickFix} /> // ... );
This not fixes the inner searchbox but it prevent some unwanted rerendering.
Hope this helps
handleEmojiClick is not defined? 🤷🏼♂️
@internationalhouseofdonald Hey, I believe that memoization won't help much here - in our case the problem is related to the sheer amount of elements we render at once.
am getting an error while click on the icon. it takes more time to load
To improve performance, instead of loading all the images from the cdn, can't the images be included with the package? @ealush
so we don't have to download all images on render but on install
I think its better to waste time on install than for all the user to have to suffer a laggy component
the whole package slows down performance of my site for every react update i make
Suggestion
To improve performance, instead of loading all the images from the cdn, can't the images be included with the package? @ealush
so we don't have to download all images on render but on install
I think its better to waste time on install than for all the user to have to suffer a laggy component
See my reply here: https://github.com/ealush/emoji-picker-react/issues/224#issuecomment-1054241800
To summarize - including the images as part of the package, would put the burden on you to find a decent way to store and serve the 1600+ image files, because simply having them in your node modules directory is obviously not enough.
Alternatively, if you're into serving the emojis from your own cdn rather the one publicly available, you can actually do that, and provide your own cdn url.
Maybe I am missing something, but would a sprite sheet help (hosted on the public cdn)? That cuts down the request count and it can be preloaded/cached similar to html5 games. The preload option doesn't seem to work for me either as is.
Maybe I am missing something, but would a sprite sheet help (hosted on the public cdn)? That cuts down the request count and it can be preloaded/cached similar to html5 games. The preload option doesn't seem to work for me either as is.
That's actually something I've tried before, but at least back then, the publicly available sprite wasn't good enough due to both low-res and subpixle positioning. https://github.com/ealush/emoji-picker-react/issues/224#issuecomment-1199947330
Along with that, the sprite is not ordered per-category, which means that until the whole sprite loads, categories would be partially visible.
I also believe that the lagginess in search is not due to the image loading - but mostly due to re-rendering of so many elements.
Regardless of all that, I am willing to accept a PR adding support for the sprite, if we see that it actually improves the performance of the picker.
Along with that, the sprite is not ordered per-category, which means that until the whole sprite loads, categories would be partially visible.
I was intending/hoping to load the full sheet near page load/component init, rather than picker open.
I won't be able to get to a PR atm, but I'll keep an eye on this. In the meantime I switched to native which seems to work way better, especially in searches. Is there a drawback to using native mode?
thanks!
Preloading all images is actually something we can do as a quick optimization regardless of the sprite. Might be worth exploring this too.
Native is indeed snappier, and looks better too. The main drawback would be that it looks different on every OS, and older OS's may miss some of the emojis. If that's not an issue for you, then you should be fine.
you don't ahve to use sprite maps. you can load each image per unicode, like /64/<unicode>.png
you don't ahve to use sprite maps. you can load each image per unicode, like
/64/<unicode>.png
That's how it works now
Interested to learn: I've recently (in the past few weeks) been working on a fourth major version of the picker. It is a complete re-write, and as such I've tried to tackle the performance issues the current version has, along with adding some new features.
Would love to hear what you think about the usability of this new version, here you can try out its pre-release:
https://codesandbox.io/s/floral-rgb-z2gkzp?file=/src/App.tsx
@ealush I gave that sandbox link a quick try and things look/feel good. I like the api and set "lazy load" to false and reloaded and things loaded very quickly. With lazy load to true it loaded very slowly though, so I would never use that option.
Search seemed to work very well. I'm not sure, but it seemed like certain terms had better results than before?
I didn't look much deeper than that, but the initial pass looks great.
Update - I went back and tried the "frequently used" feature, which works pretty well, but had 1 bug - get a few populated in that area, then click one of them repeatedly (I did the last). The order changes as the use counts change (I think), so you end up clicking different ones as they resort under your mouse. We have a usecase where we often like to hit the same emoji multiple times in a row, so this is an issue for us. If the recent/frequent use section only resorted on mounts, that would solve the issue for us.
Since v4 is out now, I hope it handles most cases. It definitely does not handle the loading speed of emoji images, but this has nothing to do with search speed, which has been improved greatly. Closing this.
Title. I've tried the demo and it takes a second or more on each keystroke to update UI.