Skip to content

Conversation

@aKn1ghtOut
Copy link
Contributor

@aKn1ghtOut aKn1ghtOut commented Jan 11, 2021

Checklist

  • I have read the Contributing Guide
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Proposed changes

A few methods concerning Emojis are bound multiple times to the DOM using the Template events() call, once in the reactions init.js and the other time after they get exported from app/ui/client/views/app/lib/getCommonRoomEvents.js to whatever page binds all the functions. The getCommonRoomEvents methods are always bound, hence negating a need to bind in a lower-level component.

Issue(s)

Closes #20151

Steps to test or reproduce

  1. go to a message
  2. mouse over the message
  3. click on ' add a reaction' (the smiley with the ' +' icon)
  4. click on any smiley
  5. the emoji appear below the message with a counter
  6. click on the emoji
  7. => the emoji isn't removed nor the counter decremented.

Further comments

On inspecting the network requests, we note that duplicate requests were sent, toggling the reaction twice, hence not actually changing the state while also creating the blinking sensation due to it actually toggling twice. This happens because of the double binding on EmojiEvents.

@aKn1ghtOut
Copy link
Contributor Author

@ggazzo @gabriellsh @dougfabris Please review the changes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Several users can't increase or reduce counter of emoji at the bottom of the message [BUG] Cannot remove emoji reaction

2 participants