From 7b1b8bb33e14bbf52dbf6a2ca3d414ecd89ade4b Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 2 Oct 2026 11:14:12 +0200 Subject: [PATCH] fix(widget): correctness fixes from code review - Group-by-list sort breaks ties on list id, so same-named lists no longer interleave into duplicate headers - Redundant single-section header suppression compares due kinds instead of translated label strings - AgendaTaskRow reuses floret-kit's WidgetGroupedRow instead of a hand-rolled tonal pill --- .../agendula/widget/TaskWidget.kt | 103 ++++++++++-------- .../agendula/widget/WidgetData.kt | 6 +- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/widget/TaskWidget.kt b/app/src/main/java/de/jeanlucmakiola/agendula/widget/TaskWidget.kt index a1e2eae..3b1822d 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/widget/TaskWidget.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/widget/TaskWidget.kt @@ -31,7 +31,6 @@ import androidx.glance.appwidget.lazy.items import androidx.glance.appwidget.provideContent import androidx.glance.appwidget.state.getAppWidgetState import androidx.glance.appwidget.updateAll -import androidx.glance.background import androidx.glance.color.ColorProvider import androidx.glance.currentState import androidx.glance.layout.Alignment @@ -52,14 +51,17 @@ import androidx.glance.text.TextStyle import androidx.glance.unit.ColorProvider as GlanceColorProvider import de.jeanlucmakiola.agendula.MainActivity import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.agendula.domain.SmartList import de.jeanlucmakiola.agendula.domain.TaskFilter import de.jeanlucmakiola.agendula.ui.theme.AgendulaDarkFallback import de.jeanlucmakiola.agendula.ui.theme.AgendulaLightFallback +import de.jeanlucmakiola.floret.components.Position import de.jeanlucmakiola.floret.components.pastelize import de.jeanlucmakiola.floret.glance.FloretGlanceTheme import de.jeanlucmakiola.floret.glance.WidgetActionButton import de.jeanlucmakiola.floret.glance.WidgetActionStyle import de.jeanlucmakiola.floret.glance.WidgetBackground +import de.jeanlucmakiola.floret.glance.WidgetGroupedRow import de.jeanlucmakiola.floret.glance.WidgetHeader import de.jeanlucmakiola.floret.glance.WidgetMessage import de.jeanlucmakiola.floret.glance.WidgetSurface @@ -180,13 +182,22 @@ private fun TaskWidgetBody( data.groupByList, data.listNames, ) { kind -> context.getString(dueBucketLabel(kind)) } - // A lone group whose label repeats the widget's own title verbatim + // A lone group whose bucket repeats the widget's own smart list // (the "Today" smart list showing one "Today" bucket) earns no - // place of its own. A lone group with a *different* label (a + // place of its own. A lone group for any other bucket (a // real-list widget whose only bucket is "Tomorrow") still has to // say so — it's the one thing telling the rows apart from today. + // Compared by due kind, not by rendered label text, since the two + // labels come from separately translated string resources and + // aren't guaranteed to read identically in every locale. val onlyHeader = allSections.singleOrNull { it is WidgetSection.Header } as? WidgetSection.Header - val redundantHeader = onlyHeader != null && onlyHeader.name.equals(title, ignoreCase = true) + val redundantDueKind = when ((filter as? TaskFilter.Smart)?.list) { + SmartList.TODAY -> DueKind.TODAY + SmartList.OVERDUE -> DueKind.PAST + else -> null + } + val redundantHeader = onlyHeader != null && redundantDueKind != null && + onlyHeader.key == "due:${redundantDueKind.name}" val sections = if (redundantHeader) allSections.filterNot { it is WidgetSection.Header } else allSections LazyColumn(modifier = GlanceModifier.fillMaxSize()) { items(sections.size) { index -> @@ -300,53 +311,51 @@ private fun AgendaTaskRow( val onColor = if (colored) GlanceColorProvider(Color.White) else GlanceTheme.colors.onSurface val onColorVariant = if (colored) GlanceColorProvider(Color.White.copy(alpha = 0.8f)) else GlanceTheme.colors.onSurfaceVariant val onColorError = if (colored) GlanceColorProvider(Color.White) else GlanceTheme.colors.error - Row( - modifier = GlanceModifier - .fillMaxWidth() - .cornerRadius(16.dp) - .background(color) - .padding(horizontal = 8.dp, vertical = 6.dp), - verticalAlignment = Alignment.CenterVertically, - ) { - Box( - modifier = GlanceModifier - .size(28.dp) - .cornerRadius(14.dp) - .clickable( - actionRunCallback( - actionParametersOf( - CompleteTaskAction.TaskIdKey to row.taskId, - CompleteTaskAction.OccurrenceKey to - (row.occurrenceStart?.toEpochMilliseconds() ?: CompleteTaskAction.NO_OCCURRENCE), + WidgetGroupedRow(position = Position.Alone, color = color) { + Row( + modifier = GlanceModifier.fillMaxWidth().padding(horizontal = 8.dp, vertical = 6.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Box( + modifier = GlanceModifier + .size(28.dp) + .cornerRadius(14.dp) + .clickable( + actionRunCallback( + actionParametersOf( + CompleteTaskAction.TaskIdKey to row.taskId, + CompleteTaskAction.OccurrenceKey to + (row.occurrenceStart?.toEpochMilliseconds() ?: CompleteTaskAction.NO_OCCURRENCE), + ), ), ), - ), - contentAlignment = Alignment.Center, - ) { - Image( - provider = ImageProvider(GlanceR.drawable.floret_ic_widget_check), - contentDescription = context.getString(R.string.widget_complete_task, title), - colorFilter = ColorFilter.tint(onColorVariant), - modifier = GlanceModifier.size(16.dp), - ) - } - Spacer(GlanceModifier.width(8.dp)) - Column(modifier = GlanceModifier.defaultWeight().clickable(open)) { - Text( - text = title, - maxLines = 1, - style = TextStyle(color = onColor, fontSize = 14.sp, fontWeight = FontWeight.Medium), - ) - dueText(context, row, use24Hour, suppressDayLabel)?.let { due -> - Text( - text = due, - maxLines = 1, - style = TextStyle( - color = if (row.isOverdue) onColorError else onColorVariant, - fontSize = 12.sp, - ), + contentAlignment = Alignment.Center, + ) { + Image( + provider = ImageProvider(GlanceR.drawable.floret_ic_widget_check), + contentDescription = context.getString(R.string.widget_complete_task, title), + colorFilter = ColorFilter.tint(onColorVariant), + modifier = GlanceModifier.size(16.dp), ) } + Spacer(GlanceModifier.width(8.dp)) + Column(modifier = GlanceModifier.defaultWeight().clickable(open)) { + Text( + text = title, + maxLines = 1, + style = TextStyle(color = onColor, fontSize = 14.sp, fontWeight = FontWeight.Medium), + ) + dueText(context, row, use24Hour, suppressDayLabel)?.let { due -> + Text( + text = due, + maxLines = 1, + style = TextStyle( + color = if (row.isOverdue) onColorError else onColorVariant, + fontSize = 12.sp, + ), + ) + } + } } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/widget/WidgetData.kt b/app/src/main/java/de/jeanlucmakiola/agendula/widget/WidgetData.kt index 8aa1dbd..d37ffe6 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/widget/WidgetData.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/widget/WidgetData.kt @@ -214,7 +214,11 @@ internal fun widgetRows( val sorted = open .filter { (it.parentId ?: 0L) <= 0L || it.parentId !in present } .sortedWith(comparator) - .let { if (groupByList) it.sortedBy { task -> listNames[task.listId].orEmpty() } else it } + .let { + if (groupByList) { + it.sortedWith(compareBy({ task -> listNames[task.listId].orEmpty() }, { task -> task.listId })) + } else it + } return sorted .take(MAX_WIDGET_ROWS) .map { task ->