Feature/fga/reactions UI improvements #5204
Conversation
|
Looks good! TY! The designs were made with the assumption that the text in each reaction is black in colour. Can we match the colour and style of '3 more' and 'show less' to match the text style used for reactions? i.e. grey and bold text? |
Matrix SDKIntegration Tests Results:
|
|
will this also fix #5149 ? |
Oh yeah probably! |
| @@ -83,23 +113,36 @@ abstract class AbsBaseMessageItem<H : AbsBaseMessageItem.Holder> : BaseEventItem | |||
| reactionButton.isEnabled = reaction.synced | |||
| holder.reactionsContainer.addView(reactionButton) | |||
| } | |||
| if (reactions.count() > MAX_REACTIONS_TO_SHOW) { | |||
| val showReactionsTextView = createReactionTextView(holder, 6) | |||
There was a problem hiding this comment.
nit: could be handy to have some named arguments for the hardcoded values for some extra context
There was a problem hiding this comment.
Actually, I'm changing this to a Style as it's shared with ReactionButton
| val showAll: Boolean = false, | ||
| val onShowMoreClicked: () -> Unit, | ||
| val onShowLessClicked: () -> Unit, | ||
| val onAddMoreClicked: () -> Unit |
There was a problem hiding this comment.
Weird to see some lambda in data class
There was a problem hiding this comment.
not sure what will happen when we attempt to parcelize 🤔
| @@ -3779,4 +3779,7 @@ | |||
| <string name="tooltip_attachment_poll">Create poll</string> | |||
| <string name="tooltip_attachment_location">Share location</string> | |||
|
|
|||
| <string name="message_reaction_show_less">Show less</string> | |||
| <string name="message_reaction_show_more">"%1$d more"</string> | |||
There was a problem hiding this comment.
You should have used a plurals (?)
This PR improves the UI of reactions by handling #3344 #4674 and #5149
It also fixes reactions order on Bubble and UI echo.