Add 'more reactions' button to message #756
Conversation
...tlin/io/element/android/features/messages/impl/timeline/components/MessagesReactionButton.kt
Outdated
Show resolved
Hide resolved
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #756 +/- ##
===========================================
- Coverage 56.81% 56.80% -0.01%
===========================================
Files 912 913 +1
Lines 23071 23116 +45
Branches 4674 4681 +7
===========================================
+ Hits 13108 13132 +24
- Misses 7886 7903 +17
- Partials 2077 2081 +4
☔ View full report in Codecov by Sentry. |
jonnyandrew
requested review from
julioromano and
bmarty
and removed request for
a team
| @@ -103,6 +104,9 @@ fun TimelineItemEventRow( | |||
| fun onReactionClicked(emoji: String) = | |||
| onReactionClick(emoji, event) | |||
|
|
|||
| fun onMoreReactionsClicked() = | |||
There was a problem hiding this comment.
This function might pass an outdated version of event to onMoreReactionsClick, but I guess it's not really and issue since we're just using the event id. Still, using a lambda here would make this less error-prone (see #771 (comment)).
There was a problem hiding this comment.
Good point, I actually ran into this exact issue just now while working on something else.
I think the local function captures the outer function parameter the first time it's called but doesn't get updated on recomposition.
Maybe we should ensure local functions do not reference arguments to the parent composable to avoid subtle bugs like this?
… jonny/reaction-buttons
|
Kudos, SonarCloud Quality Gate passed! |
- Add 'more reactions' button to message - Fix display of existing emoji reactions to match designs - Refactor emoji reactions to reduce nesting of composables --------- Co-authored-by: ElementBot <[email protected]>
Changes
Context
Part of
Screenshots