fix(widget): correctness fixes from code review
- CompleteTaskAction only ever redrew TaskWidget by glanceId; since the callback is shared with GlanceTaskWidget's rows too, and a GlanceId doesn't say which provider it belongs to, that could push the wrong widget's RemoteViews into the other's slot. Redraws both via updateAll instead. - A single-group header is now dropped only when its label would repeat the widget's own title verbatim (e.g. the "Today" smart list showing one "Today" bucket) rather than whenever there's just one group — a real-list widget whose only bucket is "Tomorrow" needs that label, since nothing else on screen says it. - A widget's preset list, like the app's own default list, is checked against the writable/existing list set before seeding a new task — it can otherwise have been deleted or turned read-only since the widget was configured. - The "group by list" toggle is cleared on save once the selection narrows to where its row no longer shows, instead of staying stuck on with no way left to see or undo it. - GlanceTaskWidget's next-task row now shares TaskWidget's due-text logic (bare time when the day's already said), instead of always showing the full date for a same-day due. - Dropped Position/positionOf from WidgetSection now that rows are standalone pills rather than WidgetGroupedRow cards — it was dead weight computed on every redraw, and the doc comment describing the card rendering no longer matched what's there. - Collapsed Ready.singleListName into a lookup on singleListId rather than a second parallel filter switch.
This commit is contained in:
@@ -123,10 +123,11 @@ class TaskEditViewModel @Inject constructor(
|
||||
val settings = settingsPrefs.settings.first()
|
||||
defaultFields = settings.defaultEditFields
|
||||
val lists = runCatching { repository.taskLists().first() }.getOrElse { emptyList() }
|
||||
// A read-only or vanished default would give the task a list it
|
||||
// A read-only or vanished default — or a widget's preset list, if it was
|
||||
// since deleted or turned read-only — would give the task a list it
|
||||
// cannot be saved to.
|
||||
val writable = lists.filter { it.acceptsWrites }
|
||||
val defaultList = presetListId
|
||||
val defaultList = presetListId?.takeIf { id -> writable.any { it.id == id } }
|
||||
?: settings.defaultListId?.takeIf { id -> writable.any { it.id == id } }
|
||||
?: writable.firstOrNull { !it.isLocal }?.id
|
||||
?: writable.firstOrNull()?.id
|
||||
|
||||
@@ -51,8 +51,6 @@ import de.jeanlucmakiola.floret.glance.WidgetActionStyle
|
||||
import de.jeanlucmakiola.floret.glance.WidgetBackground
|
||||
import de.jeanlucmakiola.floret.glance.WidgetSurface
|
||||
import de.jeanlucmakiola.floret.glance.WidgetThemeMode
|
||||
import de.jeanlucmakiola.floret.time.formatDateTimeCompact
|
||||
import de.jeanlucmakiola.floret.time.formatTime
|
||||
import de.jeanlucmakiola.floret.glance.R as GlanceR
|
||||
import kotlinx.coroutines.flow.first
|
||||
|
||||
@@ -199,9 +197,12 @@ private fun GlanceNextTaskRow(row: WidgetTaskRow, use24Hour: Boolean) {
|
||||
maxLines = 2,
|
||||
style = TextStyle(color = GlanceTheme.colors.onSurface, fontSize = 13.sp),
|
||||
)
|
||||
row.due?.let { due ->
|
||||
// This widget is always Today + overdue (see the class doc), and
|
||||
// GlanceSummary already labels the group "Today" — so, like the main
|
||||
// Tasks widget's own rows, a same-day due only needs its bare time.
|
||||
dueText(context, row, use24Hour, suppressDayLabel = true)?.let { due ->
|
||||
Text(
|
||||
text = due.formatDateTimeCompact(row.isAllDay, use24Hour).ifBlank { due.formatTime(use24Hour) },
|
||||
text = due,
|
||||
maxLines = 1,
|
||||
style = TextStyle(
|
||||
color = if (row.isOverdue) GlanceTheme.colors.error else GlanceTheme.colors.onSurfaceVariant,
|
||||
|
||||
@@ -30,6 +30,7 @@ import androidx.glance.appwidget.lazy.LazyColumn
|
||||
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
|
||||
@@ -128,7 +129,16 @@ class CompleteTaskAction : ActionCallback {
|
||||
runCatching {
|
||||
context.widgetEntryPoint().tasksRepository().setCompleted(taskId, occurrence, true)
|
||||
}
|
||||
TaskWidget().update(context.applicationContext, glanceId)
|
||||
// This callback is shared by both TaskWidget's and GlanceTaskWidget's
|
||||
// rows, and glanceId only tells us the tapped widget's instance, not
|
||||
// which of the two provider classes it belongs to — updating the wrong
|
||||
// one by id would push this class's RemoteViews into the other's
|
||||
// AppWidget slot. updateAll is class-scoped, so redrawing both is the
|
||||
// immediate, correct option; it also covers every other open instance
|
||||
// of either widget showing this task.
|
||||
val appContext = context.applicationContext
|
||||
TaskWidget().updateAll(appContext)
|
||||
GlanceTaskWidget().updateAll(appContext)
|
||||
}
|
||||
|
||||
companion object {
|
||||
@@ -170,12 +180,14 @@ private fun TaskWidgetBody(
|
||||
data.groupByList,
|
||||
data.listNames,
|
||||
) { kind -> context.getString(dueBucketLabel(kind)) }
|
||||
// A single group repeats what the widget's own title already
|
||||
// says (e.g. the "Today" smart list showing one "Today"
|
||||
// bucket) — the label only earns its place once there's more
|
||||
// than one group to tell apart.
|
||||
val singleGroup = allSections.count { it is WidgetSection.Header } <= 1
|
||||
val sections = if (singleGroup) allSections.filterNot { it is WidgetSection.Header } else allSections
|
||||
// A lone group whose label repeats the widget's own title verbatim
|
||||
// (the "Today" smart list showing one "Today" bucket) earns no
|
||||
// place of its own. A lone group with a *different* label (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.
|
||||
val onlyHeader = allSections.singleOrNull { it is WidgetSection.Header } as? WidgetSection.Header
|
||||
val redundantHeader = onlyHeader != null && onlyHeader.name.equals(title, ignoreCase = true)
|
||||
val sections = if (redundantHeader) allSections.filterNot { it is WidgetSection.Header } else allSections
|
||||
LazyColumn(modifier = GlanceModifier.fillMaxSize()) {
|
||||
items(sections.size) { index ->
|
||||
when (val section = sections[index]) {
|
||||
@@ -199,8 +211,9 @@ private fun TaskWidgetBody(
|
||||
color = container,
|
||||
colored = rowColor == WidgetRowColor.BY_LIST,
|
||||
// The due-bucket header above already names the day
|
||||
// (Today / Tomorrow); grouped by list, there's no
|
||||
// such header, so the row needs to say it itself.
|
||||
// (Today / Tomorrow) whenever one is shown — and when
|
||||
// it isn't, that's only because it would have repeated
|
||||
// the title, which names the same day just as well.
|
||||
suppressDayLabel = !data.groupByList,
|
||||
)
|
||||
Spacer(GlanceModifier.height(8.dp))
|
||||
@@ -345,7 +358,7 @@ private fun AgendaTaskRow(
|
||||
* "Tomorrow", or the compact date for anything else. `null` without a due date,
|
||||
* or for an all-day task once its day is already said by the header.
|
||||
*/
|
||||
private fun dueText(context: Context, row: WidgetTaskRow, use24Hour: Boolean, suppressDayLabel: Boolean): String? {
|
||||
internal fun dueText(context: Context, row: WidgetTaskRow, use24Hour: Boolean, suppressDayLabel: Boolean): String? {
|
||||
val due = row.due ?: return null
|
||||
val relative = when (row.dueKind) {
|
||||
DueKind.TODAY -> context.getString(R.string.home_due_today)
|
||||
|
||||
@@ -186,6 +186,11 @@ private fun WidgetFilterPicker(
|
||||
TextButton(
|
||||
enabled = canSave,
|
||||
onClick = {
|
||||
// The toggle row only shows (and so can only be turned off) with
|
||||
// more than one list selected; narrowing back down to one, or to
|
||||
// a smart list, has to clear it here instead; otherwise it's
|
||||
// stuck on with no way left to see or undo it.
|
||||
if (selectedLists.size <= 1 && groupByList) onGroupByListChange(false)
|
||||
selectedSmart?.let { onSave(TaskFilter.Smart(it)) }
|
||||
?: onSave(TaskFilter.OfLists(selectedLists))
|
||||
},
|
||||
|
||||
@@ -11,8 +11,6 @@ import de.jeanlucmakiola.agendula.domain.Task
|
||||
import de.jeanlucmakiola.agendula.domain.TaskFilter
|
||||
import de.jeanlucmakiola.agendula.domain.calendarDate
|
||||
import de.jeanlucmakiola.agendula.domain.comparator
|
||||
import de.jeanlucmakiola.floret.components.Position
|
||||
import de.jeanlucmakiola.floret.components.positionOf
|
||||
import de.jeanlucmakiola.floret.glance.WidgetBackground
|
||||
import de.jeanlucmakiola.floret.glance.WidgetThemeMode
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
@@ -137,13 +135,6 @@ sealed interface TaskWidgetData {
|
||||
val themeMode: ThemeMode,
|
||||
val use24Hour: Boolean,
|
||||
) : TaskWidgetData {
|
||||
/** The one list's name, when [filter] names exactly one real list. */
|
||||
val singleListName: String? = when (filter) {
|
||||
is TaskFilter.OfList -> listNames[filter.listId]
|
||||
is TaskFilter.OfLists -> filter.listIds.singleOrNull()?.let(listNames::get)
|
||||
is TaskFilter.Smart -> null
|
||||
}
|
||||
|
||||
/** The one list's id, when [filter] names exactly one real list — seeds a widget's add button. */
|
||||
val singleListId: Long? = when (filter) {
|
||||
is TaskFilter.OfList -> filter.listId
|
||||
@@ -151,32 +142,32 @@ sealed interface TaskWidgetData {
|
||||
is TaskFilter.Smart -> null
|
||||
}
|
||||
|
||||
/** [singleListId]'s name. */
|
||||
val singleListName: String? = singleListId?.let(listNames::get)
|
||||
|
||||
val overdueCount: Int = rows.count { it.isOverdue }
|
||||
}
|
||||
}
|
||||
|
||||
/** A run of rows in the widget's list: a task (with its [Position] within the run's card), or a run's header. */
|
||||
/** A run of rows in the widget's list: one task, or a run's header. */
|
||||
internal sealed interface WidgetSection {
|
||||
data class Header(val key: String, val name: String) : WidgetSection
|
||||
data class TaskItem(val row: WidgetTaskRow, val position: Position) : WidgetSection
|
||||
data class TaskItem(val row: WidgetTaskRow) : WidgetSection
|
||||
}
|
||||
|
||||
/** One headed run: its header's stable [key] and display [name], and the rows under it. */
|
||||
private data class WidgetRun(val key: String, val name: String, val rows: List<WidgetTaskRow>)
|
||||
|
||||
/** Each [WidgetRun] as its own [WidgetSection.Header] followed by its rows, positioned within that run's card. */
|
||||
/** Each [WidgetRun] as its own [WidgetSection.Header] followed by its rows. */
|
||||
private fun headedRuns(runs: List<WidgetRun>): List<WidgetSection> =
|
||||
runs.flatMap { run ->
|
||||
listOf(WidgetSection.Header(run.key, run.name)) +
|
||||
run.rows.mapIndexed { index, row -> WidgetSection.TaskItem(row, positionOf(index, run.rows.size)) }
|
||||
}
|
||||
runs.flatMap { run -> listOf(WidgetSection.Header(run.key, run.name)) + run.rows.map(WidgetSection::TaskItem) }
|
||||
|
||||
/**
|
||||
* [rows] split into headed runs: one per list when [groupByList] is on
|
||||
* (already sorted that way by [widgetRows]), otherwise one per [DueKind]
|
||||
* bucket — "Overdue" / "Today" / "Tomorrow" / "Later" / "No date" — in that
|
||||
* fixed order, since [rows] isn't due-sorted. Each run becomes one
|
||||
* [WidgetGroupedRow][de.jeanlucmakiola.floret.glance.WidgetGroupedRow] card.
|
||||
* fixed order, since [rows] isn't due-sorted. Each run becomes one header
|
||||
* followed by its own standalone, independently coloured task pills.
|
||||
*/
|
||||
internal fun widgetSections(
|
||||
rows: List<WidgetTaskRow>,
|
||||
|
||||
@@ -8,7 +8,6 @@ import de.jeanlucmakiola.agendula.domain.TaskSorting
|
||||
import de.jeanlucmakiola.agendula.domain.TaskStatus
|
||||
import de.jeanlucmakiola.agendula.domain.allDayInstantOf
|
||||
import de.jeanlucmakiola.agendula.domain.testTask
|
||||
import de.jeanlucmakiola.floret.components.Position
|
||||
import de.jeanlucmakiola.floret.glance.WidgetBackground
|
||||
import de.jeanlucmakiola.floret.glance.WidgetThemeMode
|
||||
import org.junit.jupiter.api.Test
|
||||
@@ -164,11 +163,11 @@ class WidgetDataTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a bucket of one row is Alone, a bucket of several has Top Middle Bottom`() {
|
||||
fun `a bucket's rows keep their own order under its one header`() {
|
||||
val rows = listOf(row(1, dueKind = DueKind.TODAY), row(2, dueKind = DueKind.TODAY), row(3, dueKind = DueKind.TODAY))
|
||||
val sections = widgetSections(rows, groupByList = false, listNames = emptyMap()) { "x" }
|
||||
val positions = sections.filterIsInstance<WidgetSection.TaskItem>().map { it.position }
|
||||
assertThat(positions).containsExactly(Position.Top, Position.Middle, Position.Bottom).inOrder()
|
||||
assertThat(sections.filterIsInstance<WidgetSection.Header>()).hasSize(1)
|
||||
assertThat(sections.filterIsInstance<WidgetSection.TaskItem>().map { it.row.taskId }).containsExactly(1L, 2L, 3L).inOrder()
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user