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
This commit is contained in:
@@ -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<CompleteTaskAction>(
|
||||
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<CompleteTaskAction>(
|
||||
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,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 ->
|
||||
|
||||
Reference in New Issue
Block a user