-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: performance issues #163
Conversation
good job 👍 tested on iPhone 15 simulator and Pixel 3a emulator, you can feel the difference on both. Looks much smoother than it used to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! There is noticeably better performance. Unfortunately, it introduces an issue with the scroll indicator (screen recording below). Despite this, I think we could merge it and then think about a solution.
RPReplay_Final1705348348.MP4
I saw that too, but I didn't find a nice way to maintain a static size of scrollbar. However i thought that the overall performance is more important than scrollbar which is usually hidden in most applications. Anyway thanks for checks we can merge it (I'm not able to do so) |
In this PR I add some limitations to the number of rendered emojis to prevent rendering emojis out of view.
Previously when we open a keyboard and it was on first category even emojis from the last one were rendered. That was causing problems not only in the keyboard itself but also on the screen where it was used. There's still a small delay on changing category but it's significantly reduced. However it may be caused by development build so I decided to go with it for now and search for more improvements in the future if needed.
Edit: as you can see from commits I was forced to also fix them in this PR to not break workflows on main after merging this one
closes #148