signalapp / Signal-Desktop

A private messenger for Windows, macOS, and Linux.
https://signal.org/download
GNU Affero General Public License v3.0
14.68k stars 2.68k forks source link

New scrollable sticker picker #7064

Open hackerbirds opened 3 weeks ago

hackerbirds commented 3 weeks ago

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:

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.

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.