search

signalapp / Signal-Desktop

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

Emoji drawer: incorrect category button highlighted #4749

Open samuel-lucas6 opened 3 years ago

samuel-lucas6 commented 3 years ago

Bug Description

Clicking on a category button in the emoji drawer highlights the wrong category button.

Steps to Reproduce

  1. Open a chat.
  2. Click on the emoji smiley face.
  3. Click on different emoji categories.

Actual Result: The wrong emoji category button is highlighted.

Expected Result: The emoji category you clicked on should be highlighted.

Screenshots

Signal Emoji Drawer Categories

Platform Info

Signal Version: 1.39.4

Operating System: Ubuntu 20.10

Linked Device Version: 5.0.8

Link to Debug Log

https://debuglogs.org/aa95ab1f2ebb8d9918adebec9401c3ef44e58fb7234028367092bb54449e33ed

hiqua commented 3 years ago

Fixed as of current beta.

EvanHahn-Signal commented 3 years ago

I'm still able to reproduce this if I adjust Signal Desktop's zoom level, so I don't think this has been fixed.

hiqua commented 3 years ago

I'm still able to reproduce this if I adjust Signal Desktop's zoom level, so I don't think this has been fixed.

I stand corrected, didn't think of adjusting the zoom level!

EvanHahn-Signal commented 3 years ago

I haven't investigated why this is happening, but I've labeled this as a good starter task. If someone wants to try to debug it, we'd appreciate it!

jonatcln commented 3 years ago

I have been looking into this and it seems to be an issue with react-virtualized.

Scrolling through the emoji grid causes the Grid component (from react-virtualized) to fire an onSectionRendered event. The callback is defined here: https://github.com/signalapp/Signal-Desktop/blob/d1040c01df715d00bc644e040b0bd7f38a9a4b87/ts/components/emoji/EmojiPicker.tsx#L288-L298

This callback sets the selected category to the category of the first (i.e. the top-most) visual row of emojis. It determines the relevant row based on the rowStartIndex parameter passed into the callback. The problem here being that the Grid component passes a wrong rowStartIndex (always 1 less, but never below zero) to the callback when the window is at a different zoom level than 100%.

A similar issue has already been reported at the react-virtualized repo.

hhofner commented 1 year ago

Not the most useful info but, I can't seem to reproduce this with version 6.2.0 (Apple silicon)

jonatcln commented 1 year ago

Not the most useful info but, I can't seem to reproduce this with version 6.2.0 (Apple silicon)

Can still reproduce it with 6.2.0 (Ubuntu 22.04). Though only after adjusting the zoom level.