Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Send and undo reaction events #2954

Merged
merged 14 commits into from
May 13, 2019
Merged

Send and undo reaction events #2954

merged 14 commits into from
May 13, 2019

Conversation

jryans
Copy link
Collaborator

@jryans jryans commented May 10, 2019

Reviewer: Each commit in this PR is self-contained and has some additional context in the commit message, so you may want to review commit by commit.

reactions-demo

Fixes element-hq/element-web#9572
Fixes element-hq/element-web#9574
Depends on matrix-org/matrix-js-sdk#911

jryans added 11 commits May 10, 2019 17:19
If reactions are enabled, we need to enable client-side aggregation in the
`MatrixClient` to access the data.
This displays existing reactions correctly in the action bar and reaction row,
but it doesn't yet update after a new reaction is sent.
This updates the reaction state in the reaction row and action bar when a
reaction is redacted.

Part of element-hq/element-web#9574
This simplifies `ReactionDimension` by using the emoji string everywhere instead
of keeping a separate text string as well. It should improve readability as
well, as the reaction events also have a field `key` which was the emoji
content, which was easy to confuse.
The `EventTile` for events without reactions now use `Event.relationsCreated` to
listen for a future time where they come in to being.
The various reaction UI bits will now listen for `Reactions.add` for new
reactions just like with redactions.

Part of element-hq/element-web#9572
This updates both the reaction row and action bar UIs to send and redact
reaction events as appropriate based on user interactions.

Fixes element-hq/element-web#9574
Fixes element-hq/element-web#9572
This changes to "did update" and also calls the reaction change handler to
ensure that we update the state of my reactions (to know which were sent by
you).
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall lgtm pending matrix-org/matrix-js-sdk#911 being approved.

src/settings/Settings.js Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
@Half-Shot
Copy link
Contributor

I've spent the last minute ogling the gif and trying to work out why it makes me slightly annoyed, and I think it's because the reactions pop in and move the button bar upwards. Is there any chance this could be fixed? Though I guess it's going to be a hard one :/

@jryans
Copy link
Collaborator Author

jryans commented May 13, 2019

I've spent the last minute ogling the gif and trying to work out why it makes me slightly annoyed, and I think it's because the reactions pop in and move the button bar upwards. Is there any chance this could be fixed? Though I guess it's going to be a hard one :/

Hmm, we may want to do something about it. We'll see what other people think as well when it lands.

jryans added 3 commits May 13, 2019 14:41
This changes to use component state instead of `forceUpdate`, so that it's more
obvious why an update is happening here.
@jryans jryans merged commit f5aa32b into develop May 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reactions: Undo a reaction Reactions: Send reaction event when user reacts
4 participants