Skip to content

Commit 33fe249

Browse files
nbradburynbradbury
authored
Add date subheaders to the RS comment list (#23139)
* Add date subheaders to the RS comment list Interleave date-group headers into the rs comment list, matching the legacy list. A new withDateHeaders() groups consecutive comments by their existing relativeDate label (the same javaDateToTimeSpan value legacy groups by) and inserts a header row on each change; the LazyColumn renders headers and comments as distinct row types. * Fix review findings in the date subheaders - Key each date header by its label instead of the group's first comment id, so a header stays stable (no remove+add re-animation) when a newer comment is prepended into an existing date group. Labels are unique per contiguous group given the date sort. Drops the now-unneeded keyId field. - Uppercase the header label with the default locale, matching the legacy subheader's locale-aware textAllCaps (Kotlin's no-arg uppercase() is Locale.ROOT and mis-cases e.g. Turkish month names). * Read the subheader locale observably to satisfy lint Reading Locale.getDefault() in a composable trips lint's NonObservableLocale (the header wouldn't recompose on a locale change). Read the locale from LocalConfiguration via ConfigurationCompat instead, matching the existing stats-card pattern, keeping the locale-aware uppercase. * Make date-header keys collision-proof The header LazyColumn key relied on the invariant that date-sorted comments make each label one contiguous group. If the list ever arrived out of date order a label could repeat non-contiguously, producing a duplicate key -> LazyColumn IllegalArgumentException (a crash, where the legacy list only showed a redundant header). withDateHeaders now mints a guaranteed-unique key per header (still just the label in the normal case, disambiguated only on a non-contiguous repeat), so correctness no longer depends on the sort order.
1 parent e05eaf0 commit 33fe249

4 files changed

Lines changed: 190 additions & 10 deletions

File tree

‎WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListRow.kt‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package org.wordpress.android.ui.commentsrs
2+
3+
/** A rendered row in the rs comment list: either a date-group header or a comment. */
4+
sealed interface CommentsRsListRow {
5+
/** [key] is a guaranteed-unique LazyColumn key; [label] is the (date) text shown to the user. */
6+
data class DateHeader(val label: String, val key: String) : CommentsRsListRow
7+
data class Item(val comment: CommentRsUiModel) : CommentsRsListRow
8+
}
9+
10+
/**
11+
* Interleaves date subheaders into [comments] (already in display order), mirroring the legacy
12+
* list: a header before the first comment and before every comment whose date label differs from
13+
* the previous one. The label is the row's own [CommentRsUiModel.relativeDate] — the same
14+
* javaDateToTimeSpan value the legacy list groups by — so no extra date handling is needed here.
15+
*
16+
* Each header carries a unique [CommentsRsListRow.DateHeader.key] for the LazyColumn. Comments are
17+
* date-sorted, so a label normally maps to one contiguous group and the key is just the label —
18+
* which keeps the header stable when a newer comment is prepended into an existing group. Should
19+
* the list ever arrive out of date order, a repeated label is disambiguated rather than emitting a
20+
* duplicate key (which LazyColumn rejects with a hard crash, unlike the legacy RecyclerView).
21+
*/
22+
fun withDateHeaders(comments: List<CommentRsUiModel>): List<CommentsRsListRow> {
23+
val rows = ArrayList<CommentsRsListRow>(comments.size + 1)
24+
val usedKeys = HashSet<String>()
25+
var lastLabel: String? = null
26+
for (comment in comments) {
27+
if (comment.relativeDate != lastLabel) {
28+
rows.add(CommentsRsListRow.DateHeader(comment.relativeDate, uniqueKey(comment.relativeDate, usedKeys)))
29+
lastLabel = comment.relativeDate
30+
}
31+
rows.add(CommentsRsListRow.Item(comment))
32+
}
33+
return rows
34+
}
35+
36+
/** A stable per-label key, suffixed only if the same label recurs non-contiguously (see above). */
37+
private fun uniqueKey(label: String, used: MutableSet<String>): String {
38+
val base = "header_$label"
39+
if (used.add(base)) return base
40+
var n = 1
41+
while (!used.add("$base#$n")) n++
42+
return "$base#$n"
43+
}

‎WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,19 @@ import androidx.compose.ui.draw.clip
2929
import androidx.compose.ui.graphics.Color
3030
import androidx.compose.ui.graphics.vector.rememberVectorPainter
3131
import androidx.compose.ui.layout.ContentScale
32+
import androidx.compose.ui.platform.LocalConfiguration
3233
import androidx.compose.ui.res.stringResource
3334
import androidx.compose.ui.text.AnnotatedString
3435
import androidx.compose.ui.text.SpanStyle
3536
import androidx.compose.ui.text.buildAnnotatedString
3637
import androidx.compose.ui.text.font.FontWeight
3738
import androidx.compose.ui.text.style.TextOverflow
3839
import androidx.compose.ui.unit.dp
40+
import androidx.core.os.ConfigurationCompat
3941
import coil.compose.AsyncImage
4042
import org.wordpress.android.R
4143
import org.wordpress.android.ui.commentsrs.CommentRsUiModel
44+
import java.util.Locale
4245

4346
private val AVATAR_SIZE = 40.dp
4447
private val PENDING_INDICATOR_WIDTH = 4.dp
@@ -173,3 +176,23 @@ private fun AnnotatedString.Builder.boldRange(formatted: String, part: String) {
173176
addStyle(SpanStyle(fontWeight = FontWeight.Bold), start, start + part.length)
174177
}
175178
}
179+
180+
/** A date-group subheader row, matching the legacy list's all-caps overline separators. */
181+
@Composable
182+
fun CommentsRsDateHeader(label: String, modifier: Modifier = Modifier) {
183+
// Locale-aware uppercase, like the legacy subheader's android:textAllCaps (Kotlin's no-arg
184+
// uppercase() is Locale.ROOT and would mis-case e.g. Turkish month names). Read the locale
185+
// observably from the composition so it tracks locale changes.
186+
val locale = ConfigurationCompat.getLocales(LocalConfiguration.current)[0] ?: Locale.ROOT
187+
Text(
188+
text = label.uppercase(locale),
189+
style = MaterialTheme.typography.labelMedium,
190+
color = MaterialTheme.colorScheme.onSurfaceVariant,
191+
maxLines = 1,
192+
overflow = TextOverflow.Ellipsis,
193+
modifier = modifier
194+
.fillMaxWidth()
195+
.background(MaterialTheme.colorScheme.surface)
196+
.padding(start = 16.dp, end = 16.dp, top = 16.dp, bottom = 8.dp)
197+
)
198+
}

‎WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import androidx.compose.material3.pulltorefresh.PullToRefreshDefaults
2626
import androidx.compose.material3.pulltorefresh.rememberPullToRefreshState
2727
import androidx.compose.runtime.Composable
2828
import androidx.compose.runtime.LaunchedEffect
29+
import androidx.compose.runtime.remember
2930
import androidx.compose.runtime.snapshotFlow
3031
import androidx.compose.ui.Alignment
3132
import androidx.compose.ui.Modifier
@@ -36,7 +37,9 @@ import androidx.compose.ui.unit.dp
3637
import kotlinx.coroutines.flow.distinctUntilChanged
3738
import org.wordpress.android.R
3839
import org.wordpress.android.ui.commentsrs.CommentRsUiModel
40+
import org.wordpress.android.ui.commentsrs.CommentsRsListRow
3941
import org.wordpress.android.ui.commentsrs.CommentsTabUiState
42+
import org.wordpress.android.ui.commentsrs.withDateHeaders
4043
import org.wordpress.android.ui.compose.components.ShimmerBox
4144

4245
@OptIn(ExperimentalMaterial3Api::class)
@@ -135,21 +138,35 @@ private fun CommentListContent(
135138
}
136139
}
137140

141+
// Interleave date subheaders once per comment-list change, like the legacy list.
142+
val rows = remember(comments) { withDateHeaders(comments) }
138143
LazyColumn(
139144
state = listState,
140145
modifier = Modifier.fillMaxSize()
141146
) {
142147
items(
143-
items = comments,
144-
key = { it.remoteCommentId }
145-
) { comment ->
146-
CommentsRsListItem(
147-
comment = comment,
148-
isSelected = comment.remoteCommentId in selectedIds,
149-
onClick = { onCommentClick(comment.remoteCommentId) },
150-
onLongClick = { onCommentLongClick(comment.remoteCommentId) },
151-
modifier = Modifier.animateItem()
152-
)
148+
items = rows,
149+
key = { row ->
150+
when (row) {
151+
is CommentsRsListRow.DateHeader -> row.key
152+
is CommentsRsListRow.Item -> row.comment.remoteCommentId
153+
}
154+
},
155+
contentType = { it::class }
156+
) { row ->
157+
when (row) {
158+
is CommentsRsListRow.DateHeader -> CommentsRsDateHeader(
159+
label = row.label,
160+
modifier = Modifier.animateItem()
161+
)
162+
is CommentsRsListRow.Item -> CommentsRsListItem(
163+
comment = row.comment,
164+
isSelected = row.comment.remoteCommentId in selectedIds,
165+
onClick = { onCommentClick(row.comment.remoteCommentId) },
166+
onLongClick = { onCommentLongClick(row.comment.remoteCommentId) },
167+
modifier = Modifier.animateItem()
168+
)
169+
}
153170
}
154171

155172
if (isLoadingMore) {

‎WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListRowTest.kt‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package org.wordpress.android.ui.commentsrs
2+
3+
import org.assertj.core.api.Assertions.assertThat
4+
import org.junit.Test
5+
import org.wordpress.android.fluxc.model.CommentStatus
6+
import org.wordpress.android.ui.commentsrs.CommentsRsListRow.DateHeader
7+
import org.wordpress.android.ui.commentsrs.CommentsRsListRow.Item
8+
9+
class CommentsRsListRowTest {
10+
@Test
11+
fun `empty list produces no rows`() {
12+
assertThat(withDateHeaders(emptyList())).isEmpty()
13+
}
14+
15+
@Test
16+
fun `a single comment gets a leading date header`() {
17+
val comment = comment(id = 1, date = "Today")
18+
19+
assertThat(withDateHeaders(listOf(comment))).containsExactly(
20+
header("Today"),
21+
Item(comment)
22+
)
23+
}
24+
25+
@Test
26+
fun `consecutive comments with the same date share one header`() {
27+
val a = comment(id = 1, date = "Today")
28+
val b = comment(id = 2, date = "Today")
29+
30+
assertThat(withDateHeaders(listOf(a, b))).containsExactly(
31+
header("Today"),
32+
Item(a),
33+
Item(b)
34+
)
35+
}
36+
37+
@Test
38+
fun `a new header is inserted whenever the date label changes`() {
39+
val a = comment(id = 1, date = "Today")
40+
val b = comment(id = 2, date = "Today")
41+
val c = comment(id = 3, date = "Yesterday")
42+
val d = comment(id = 4, date = "January 8")
43+
44+
assertThat(withDateHeaders(listOf(a, b, c, d))).containsExactly(
45+
header("Today"),
46+
Item(a),
47+
Item(b),
48+
header("Yesterday"),
49+
Item(c),
50+
header("January 8"),
51+
Item(d)
52+
)
53+
}
54+
55+
@Test
56+
fun `a header stays identical when a newer comment is prepended into its group`() {
57+
// The header is keyed by its label, so adding a same-day comment at the top of the group
58+
// must not change the header's identity (which would make it re-animate on refresh).
59+
val before = withDateHeaders(listOf(comment(id = 1, date = "Today")))
60+
val after = withDateHeaders(listOf(comment(id = 2, date = "Today"), comment(id = 1, date = "Today")))
61+
62+
val beforeHeader = before.filterIsInstance<DateHeader>().single()
63+
val afterHeader = after.filterIsInstance<DateHeader>().single()
64+
assertThat(afterHeader).isEqualTo(beforeHeader)
65+
}
66+
67+
@Test
68+
fun `a label that recurs non-contiguously gets distinct header keys instead of crashing`() {
69+
// Defensive: comments are normally date-sorted so a label is one contiguous group, but if
70+
// the list ever arrives out of order two groups can share a label. LazyColumn rejects
71+
// duplicate keys with a crash, so each header must still get a unique key.
72+
val rows = withDateHeaders(
73+
listOf(
74+
comment(id = 1, date = "Today"),
75+
comment(id = 2, date = "Yesterday"),
76+
comment(id = 3, date = "Today")
77+
)
78+
)
79+
80+
val headers = rows.filterIsInstance<DateHeader>()
81+
assertThat(headers.map { it.label }).containsExactly("Today", "Yesterday", "Today")
82+
assertThat(headers.map { it.key }).doesNotHaveDuplicates()
83+
}
84+
85+
/** A header as it appears in the normal (contiguous) case: key derived directly from the label. */
86+
private fun header(label: String) = DateHeader(label, "header_$label")
87+
88+
private fun comment(id: Long, date: String) = CommentRsUiModel(
89+
remoteCommentId = id,
90+
authorName = "Jane",
91+
avatarUrl = "",
92+
snippet = "hello",
93+
relativeDate = date,
94+
status = CommentStatus.APPROVED,
95+
postId = 99L
96+
)
97+
}

0 commit comments

Comments
 (0)