Skip to content
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

New scrollable sticker picker #7064

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hackerbirds
Copy link
Contributor

@hackerbirds hackerbirds commented Oct 27, 2024

This new sticker picker allows users to scroll through their whole list of stickers, without having to click a single button.

First time contributor checklist:

Contributor checklist:

  • My contribution is not related to translations.
  • My commits are in nice logical chunks with good commit messages
  • My changes are rebased on the latest main branch
  • A npm run ready run passes successfully (more about tests here)
  • My changes are ready to be shipped to users

Description

This PR allows users to scroll through all the stickers on the same menu, without having to tediously look for the relevant sticker pack using the cumbersome buttons/pages at the top.

I used virtualization for efficiently rendering the stickers, so it is a non-trivial change: I had to move elements rendering into a rowRenderer, and find a way to calculate the heights of each row exactly (using rowHeight).

(Note: the scrollbar height issue in the video has since been fixed)

scrollable.picker.webm

Performance

Performance is just about the same as before, except initial loading is slightly slower. Scrolling through a large list incurs high CPU usage. I did consider using placeholders while scrolling, which makes it faster, but makes the real stickers load much slower when scrolling stops, so I decided it was not worth it.

In practice, on slow CPUs it works fine, and on a more modern computer it works great. Heap allocation is about the same as before as well. I've compared those things using Chromium's profiler (and I happen to have a slow laptop so I can tell).

This is outside the scope of this PR but I think in general performance would be much improved if tinier/compressed 68x68 sticker thumbnails were loaded rather than the full-quality original files as it is currently being done.

Design considerations

I made the sticker pack titles a bit bigger (<h3>) than what seems typical for other elements in the app--my thought process was that users may scroll through their stickers quickly, so bigger text means easier to read and more quickly.

AFAICT accessibility is not affected and should remain the same as it used to. Things are labelled correctly. I did change the alt text of individual stickers to include the emoji they're meant to represent (before that, it was just the sticker pack title). I thought that this was more appropriate so a screen reader can say what emoji the sticker is about.

  • Recents don't show up if there are no recents stickers or no packs, so some messages were simplified
  • Sticker packs can't be "empty" anymore. If there are no sticker packs at all, then the picker will just return a "no packs" message instead of an "empty" message. (Note that in practice this message won't really be seen--if there are no packs then the app sends the user to installing default stickers instead of opening the picker, but that can be bypassed with CTRL+SHIFT+G)

Testing

I did a lot of manual testing. I tested that scrolling behaves as expected, stickers send as expected, and that the sticker picker in the media editor works as expected. The featured time stickers in the media editor's sticker picker are OK. The picker updates correctly when a sticker is added/downloaded. Keyboard navigation works. RTL works (in fact there is a bug in the original sticker picker that I fixed here). Dark theme/light theme works. Accessibility labels/screen readers seem to be OK but I don't have experience with them. Lastly I made sure rare/legacy text/error messages display correctly.

Credit

@spazzylemons helped me fix a bug I had, so they're co-authored.

This new sticker picker allows users to scroll through their whole list of stickers, without having to click a single button.

Co-authored-by: spazzylemons <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants