Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Improve Reaction UI #1490

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

refactor: Improve Reaction UI #1490

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8ccb9d6
Refactor: Improve Reaction UI in MessageList- Moves the ReactionRow b…
jamesarich Dec 13, 2024
fe44aed
Refactor: Update reaction button border color
jamesarich Dec 19, 2024
a4b1f1c
Merge branch 'master' into reaction_ui_improvement
jamesarich Dec 28, 2024
da5248b
Refactor MessageList and ReactionRow composables
jamesarich Jan 3, 2025
736bddf
Merge branch 'master' into reaction_ui_improvement
jamesarich Jan 3, 2025
875e94b
refactor: use calculated message height as padding
jamesarich Jan 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 39 additions & 30 deletions app/src/main/java/com/geeksville/mesh/ui/message/components/MessageList.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@

package com.geeksville.mesh.ui.message.components

import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.wrapContentSize
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.LazyListState
Expand All @@ -38,6 +41,7 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.hapticfeedback.HapticFeedbackType
import androidx.compose.ui.platform.LocalHapticFeedback
import androidx.compose.ui.unit.dp
import com.geeksville.mesh.DataPacket
import com.geeksville.mesh.database.entity.Reaction
import com.geeksville.mesh.model.Message
Expand All @@ -48,7 +52,7 @@ import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.debounce

@Suppress("LongMethod")
@Suppress("LongMethod", "MagicNumber")
@Composable
internal fun MessageList(
messages: List<Message>,
Expand Down Expand Up @@ -95,35 +99,40 @@ internal fun MessageList(
val fromLocal = msg.node.user.id == DataPacket.ID_LOCAL
val selected by remember { derivedStateOf { selectedIds.value.contains(msg.uuid) } }

ReactionRow(fromLocal, msg.emojis) { showReactionDialog = msg.emojis }
Box(Modifier.wrapContentSize(Alignment.TopStart)) {
var expandedNodeMenu by remember { mutableStateOf(false) }
MessageItem(
node = msg.node,
messageText = msg.text,
messageTime = msg.time,
messageStatus = msg.status,
selected = selected,
onClick = { if (inSelectionMode) selectedIds.toggle(msg.uuid) },
onLongClick = {
selectedIds.toggle(msg.uuid)
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
},
onChipClick = {
if (msg.node.num != 0) {
expandedNodeMenu = true
}
},
onStatusClick = { showStatusDialog = msg },
onSendReaction = { onSendReaction(it, msg.packetId) },
)
NodeMenu(
node = msg.node,
showFullMenu = true,
onDismissRequest = { expandedNodeMenu = false },
expanded = expandedNodeMenu,
onAction = onNodeMenuAction
)
Column(
modifier = Modifier.padding(vertical = 8.dp),
verticalArrangement = Arrangement.spacedBy((-12).dp)
Copy link
Member

Choose a reason for hiding this comment

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

Avoid adding Columns inside LazyColumns, use spacing modifiers like padding or offset directly in ReactionRow where it's needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find much documentation on why this should be avoided - but it was easy enough to swap out for offset and zindex.

) {
Box(Modifier.wrapContentSize(Alignment.TopStart)) {
var expandedNodeMenu by remember { mutableStateOf(false) }
MessageItem(
node = msg.node,
messageText = msg.text,
messageTime = msg.time,
messageStatus = msg.status,
selected = selected,
onClick = { if (inSelectionMode) selectedIds.toggle(msg.uuid) },
onLongClick = {
selectedIds.toggle(msg.uuid)
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
},
onChipClick = {
if (msg.node.num != 0) {
expandedNodeMenu = true
}
},
onStatusClick = { showStatusDialog = msg },
onSendReaction = { onSendReaction(it, msg.packetId) },
)
NodeMenu(
node = msg.node,
showFullMenu = true,
onDismissRequest = { expandedNodeMenu = false },
expanded = expandedNodeMenu,
onAction = onNodeMenuAction
)
}
ReactionRow(fromLocal, msg.emojis) { showReactionDialog = msg.emojis }
}
}
}
Expand Down
110 changes: 80 additions & 30 deletions app/src/main/java/com/geeksville/mesh/ui/message/components/Reaction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,30 @@

package com.geeksville.mesh.ui.message.components

import androidx.compose.animation.AnimatedContent
import androidx.compose.animation.slideInVertically
import androidx.compose.animation.slideOutVertically
import androidx.compose.animation.togetherWith
import androidx.compose.foundation.background
import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ExperimentalLayoutApi
import androidx.compose.foundation.layout.FlowRow
import androidx.compose.foundation.layout.FlowRowOverflow
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.LazyRow
import androidx.compose.foundation.lazy.items
import androidx.compose.foundation.shape.CircleShape
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.Badge
import androidx.compose.material.BadgedBox
import androidx.compose.material.ContentAlpha
import androidx.compose.material.Divider
import androidx.compose.material.Icon
Expand All @@ -43,7 +49,7 @@ import androidx.compose.material.MaterialTheme
import androidx.compose.material.Surface
import androidx.compose.material.Text
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.EmojiEmotions
import androidx.compose.material.icons.filled.AddReaction
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
Expand All @@ -53,10 +59,10 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.tooling.preview.PreviewLightDark
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import com.geeksville.mesh.MeshProtos
import com.geeksville.mesh.database.entity.Reaction
import com.geeksville.mesh.ui.components.BottomSheetDialog
Expand All @@ -80,7 +86,7 @@ fun ReactionButton(
}
IconButton(onClick = { showEmojiPickerDialog = true }) {
Icon(
imageVector = Icons.Default.EmojiEmotions,
imageVector = Icons.Default.AddReaction,
contentDescription = "emoji",
modifier = modifier.size(16.dp),
tint = MaterialTheme.colors.onSurface.copy(alpha = ContentAlpha.medium),
Expand All @@ -94,35 +100,51 @@ private fun ReactionItem(
emojiCount: Int = 1,
onClick: () -> Unit = {},
) {
BadgedBox(
modifier = Modifier.padding(start = 2.dp, top = 8.dp, end = 2.dp, bottom = 4.dp),
badge = {
if (emojiCount > 1) {
Badge(
backgroundColor = MaterialTheme.colors.onBackground,
contentColor = MaterialTheme.colors.background,
) {
Text(
fontWeight = FontWeight.Bold,
text = emojiCount.toString()
)
}
}
}

Surface(
modifier = Modifier
.padding(2.dp)
.border(
1.dp,
MaterialTheme.colors.secondaryVariant.copy(ContentAlpha.medium),
RoundedCornerShape(8.dp)
)
.clickable { onClick() },
color = MaterialTheme.colors.surface.copy(alpha = ContentAlpha.medium),
contentColor = MaterialTheme.colors.onSurface.copy(alpha = ContentAlpha.medium),
shape = RoundedCornerShape(8.dp),
elevation = 4.dp,
) {
Surface(
Row(
modifier = Modifier
.clickable { onClick() },
color = MaterialTheme.colors.surface,
shape = RoundedCornerShape(32.dp),
elevation = 4.dp,
.background(MaterialTheme.colors.surface)
.padding(4.dp),
verticalAlignment = Alignment.CenterVertically
) {
Text(
text = emoji,
modifier = Modifier
.padding(8.dp)
.clip(CircleShape),
style = MaterialTheme.typography.h6,
text = emoji
)
if (emojiCount > 0) {
Spacer(
modifier = Modifier.width(2.dp)
)
AnimatedContent(
targetState = emojiCount,
transitionSpec = {
if (targetState > initialState) {
slideInVertically { -it } togetherWith slideOutVertically { it }
} else {
slideInVertically { it } togetherWith slideOutVertically { -it }
}
}
) {
Text(
text = "$it",
style = MaterialTheme.typography.body2,
)
}
}
}
}
}
Expand All @@ -146,11 +168,21 @@ fun ReactionRow(
)
}

var maxLines by remember { mutableStateOf(1) }
FlowRow(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 16.dp),
horizontalArrangement = if (fromLocal) Arrangement.End else Arrangement.Start
horizontalArrangement = if (fromLocal) Arrangement.End else Arrangement.Start,
maxLines = maxLines,
overflow = FlowRowOverflow.expandIndicator {
ReactionItem(
emoji = "...",
emojiCount = 0
) {
maxLines += 1
}
}
) {
emojiList.forEach { entry ->
ReactionItem(
Expand All @@ -164,6 +196,23 @@ fun ReactionRow(
}
}

@Composable
internal fun Ellipsis(text: String, onClick: () -> Unit) {
Surface(
color = MaterialTheme.colors.surface,
contentColor = MaterialTheme.colors.onSurface,
modifier = Modifier
.clickable(onClick = onClick)
) {
Text(
modifier = Modifier
.padding(3.dp),
text = text,
fontSize = 18.sp
)
}
}

fun reduceEmojis(emojis: List<String>): Map<String, Int> = emojis.groupingBy { it }.eachCount()

@Composable
Expand Down Expand Up @@ -231,6 +280,7 @@ fun ReactionItemPreview() {
) {
ReactionItem(emoji = "\uD83D\uDE42")
ReactionItem(emoji = "\uD83D\uDE42", emojiCount = 2)
ReactionItem(emoji = "\uD83D\uDE42", emojiCount = 222)
ReactionButton()
}
}
Expand Down
Loading