Implement ExpenseListScreen, AddEditExpenseScreen, AddEditRecurringScreen (#6) #18

Merged
admin merged 2 commits from feat/6-implement-expense-list-add-edit-screens into main 2026-06-28 12:57:17 +00:00
Owner

Summary

  • Add ExpenseListScreen with 2 tabs (Expenses | Recurring) using PrimaryTabRow + foundation HorizontalPager, tab-aware FloatingActionButton (Manual + Import Bank Statement on Expenses tab, Add Recurring on Recurring tab, collapses on tab change), search field with date-range filter chips, long-press delete with confirmation dialog, and ExportAction wired to ExportExpensesToCsv with a snackbar on failure.
  • Add AddEditExpenseScreen and AddEditRecurringScreen as shared add/edit forms; edit mode prefills via GetExpenses.awaitOne / GetRecurringExpenses.awaitOne, the Save action is disabled until amount > 0 && categoryId != null, recurring edit preserves the loaded nextDueDate per docs/03-function-todos.md:116, and the expense form's "Manage Categories" button pushes CategoryScreen.
  • Extract the shared DateField composable to ui/components/DateField.kt (UTC roundtrip, matches the pattern from #7's ExportAction).

Test plan

  • ./gradlew assembleDebug succeeds (37 tasks, 0 errors)
  • Manual: tap FAB on Expenses tab → Manual opens AddEditExpenseScreen with an empty form; fill amount + category → Save pops back to the list with the new row
  • Manual: tap an existing expense row → fields prefill; change amount → Save → row updates
  • Manual: long-press an expense row → delete confirmation dialog → confirm → row disappears
  • Manual: switch to Recurring tab → toggle the active switch on a row → isActive flips and persists
  • Manual: FAB on Recurring tab → "Add Recurring" → fill form including interval + start date → Save → row appears in Recurring list
  • Manual: edit an existing recurring template → start date change does NOT reset nextDueDate
  • Manual: Export icon on ExpenseListScreen → date range dialog → SAF CreateDocument("text/csv") → file written with UTF-8 BOM, ISO 8601 dates, category names, and quoted notes

Closes #6

## Summary - Add `ExpenseListScreen` with 2 tabs (Expenses | Recurring) using `PrimaryTabRow` + foundation `HorizontalPager`, tab-aware `FloatingActionButton` (Manual + Import Bank Statement on Expenses tab, Add Recurring on Recurring tab, collapses on tab change), search field with date-range filter chips, long-press delete with confirmation dialog, and `ExportAction` wired to `ExportExpensesToCsv` with a snackbar on failure. - Add `AddEditExpenseScreen` and `AddEditRecurringScreen` as shared add/edit forms; edit mode prefills via `GetExpenses.awaitOne` / `GetRecurringExpenses.awaitOne`, the Save action is disabled until `amount > 0 && categoryId != null`, recurring edit preserves the loaded `nextDueDate` per `docs/03-function-todos.md:116`, and the expense form's "Manage Categories" button pushes `CategoryScreen`. - Extract the shared `DateField` composable to `ui/components/DateField.kt` (UTC roundtrip, matches the pattern from #7's `ExportAction`). ## Test plan - [x] `./gradlew assembleDebug` succeeds (37 tasks, 0 errors) - [ ] Manual: tap FAB on Expenses tab → Manual opens `AddEditExpenseScreen` with an empty form; fill amount + category → Save pops back to the list with the new row - [ ] Manual: tap an existing expense row → fields prefill; change amount → Save → row updates - [ ] Manual: long-press an expense row → delete confirmation dialog → confirm → row disappears - [ ] Manual: switch to Recurring tab → toggle the active switch on a row → `isActive` flips and persists - [ ] Manual: FAB on Recurring tab → "Add Recurring" → fill form including interval + start date → Save → row appears in Recurring list - [ ] Manual: edit an existing recurring template → start date change does NOT reset `nextDueDate` - [ ] Manual: Export icon on `ExpenseListScreen` → date range dialog → SAF `CreateDocument("text/csv")` → file written with UTF-8 BOM, ISO 8601 dates, category names, and quoted notes Closes #6
admin added 1 commit 2026-06-28 12:36:41 +00:00
admin reviewed 2026-06-28 12:44:40 +00:00
admin left a comment
Author
Owner

Summary

Verdict: clean — no blocking concerns.

This PR implements issue #6 end-to-end. The three new screens, their ScreenModels, and the shared DateField composable all match the issue's acceptance criteria:

  • ExpenseListScreen has 2 tabs on PrimaryTabRow (the Material3 1.2+ successor of TabRow, which is the correct modern choice) + foundation HorizontalPager
  • The FAB is genuinely tab-aware — TabAwareFab takes selectedTab: Int and renders different sub-actions per tab, and LaunchedEffect(selectedTab) { expanded = false } collapses the menu on tab change
  • AddEditExpenseScreen and AddEditRecurringScreen both handle add and edit; prefill goes through getExpenses.awaitOne / getRecurringExpenses.awaitOne
  • The Save button is correctly disabled until amount > 0 && categoryId != null (line 78 of each screen)
  • The Save action uses navigator.pop() on success
  • nextDueDate preservation on recurring edit is implemented per docs/03-function-todos.md:116loadedNextDueDate is captured in init { } and used as the fallback at save time, so a startDate change in edit mode does not reset nextDueDate. New records correctly fall back to startDate
  • DateField uses UTC midnight roundtrip, which is the right call given M3's DatePicker operates in UTC
  • ExportAction (from #7) is reused correctly, with a snackbar wired to R.string.settings_export_failure on Result.failure
  • The recurring edit form correctly preserves the loaded nextDueDate and the "Manage Categories" button on the expense form pushes the CategoryScreen singleton
  • LocalDate flows cleanly through the screen model and is stored as epoch day by the existing LocalDateConverter — the UI never sees epoch longs

Architecture rules are all respected: ScreenModels are NOT in Koin (they use inject() default parameters via the KoinExtensions helpers); interactors are factory-scoped per DomainModule; no repository layer; no TabNavigator; no Canvas charts; SAF only (no manifest permissions).

Non-blocking notes (not in inline comments)

  • RecurringRow does not show note (ExpenseListScreen.kt:367-414). ExpenseRow renders the note as a second line (ExpenseListScreen.kt:343-351) and RecurringExpense.note exists in the data model (docs/01-data-model.md:104). For parity, consider rendering item.recurring.note in the same spot. Issue #6 doesn't specify row layout, so this is a design call, not a bug.
  • setDate / setNote / setStartDate / setInterval don't call refreshIsValid() — flagged by the explore agent but not a real concern: those fields aren't part of the validity check, so omitting the call is correct (only setAmount and setCategory need it).
  • save() early-returns silently on invalid input (?: return at AddEditExpenseScreenModel.kt:89-90 and AddEditRecurringScreenModel.kt:109-110) — defensive but the button is enabled = isValid && !isLoading, so the path is unreachable from the UI. A require(...) would catch bugs earlier if you prefer fail-fast.

All test plan checkboxes are addressed by the implementation; the ./gradlew assembleDebug one is the only one the agent could mark itself. The seven manual checkboxes will be exercised in a follow-up round.

## Summary **Verdict: clean — no blocking concerns.** This PR implements issue #6 end-to-end. The three new screens, their ScreenModels, and the shared `DateField` composable all match the issue's acceptance criteria: - `ExpenseListScreen` has 2 tabs on `PrimaryTabRow` (the Material3 1.2+ successor of `TabRow`, which is the correct modern choice) + foundation `HorizontalPager` - The FAB is genuinely tab-aware — `TabAwareFab` takes `selectedTab: Int` and renders different sub-actions per tab, and `LaunchedEffect(selectedTab) { expanded = false }` collapses the menu on tab change - `AddEditExpenseScreen` and `AddEditRecurringScreen` both handle add and edit; prefill goes through `getExpenses.awaitOne` / `getRecurringExpenses.awaitOne` - The Save button is correctly disabled until `amount > 0 && categoryId != null` (line 78 of each screen) - The Save action uses `navigator.pop()` on success - `nextDueDate` preservation on recurring edit is implemented per `docs/03-function-todos.md:116` — `loadedNextDueDate` is captured in `init { }` and used as the fallback at save time, so a `startDate` change in edit mode does not reset `nextDueDate`. New records correctly fall back to `startDate` - `DateField` uses UTC midnight roundtrip, which is the right call given M3's `DatePicker` operates in UTC - `ExportAction` (from #7) is reused correctly, with a snackbar wired to `R.string.settings_export_failure` on `Result.failure` - The recurring edit form correctly preserves the loaded `nextDueDate` and the "Manage Categories" button on the expense form pushes the `CategoryScreen` singleton - `LocalDate` flows cleanly through the screen model and is stored as epoch day by the existing `LocalDateConverter` — the UI never sees epoch longs Architecture rules are all respected: ScreenModels are NOT in Koin (they use `inject()` default parameters via the `KoinExtensions` helpers); interactors are `factory`-scoped per `DomainModule`; no repository layer; no `TabNavigator`; no Canvas charts; SAF only (no manifest permissions). ### Non-blocking notes (not in inline comments) - **`RecurringRow` does not show `note`** (`ExpenseListScreen.kt:367-414`). `ExpenseRow` renders the note as a second line (`ExpenseListScreen.kt:343-351`) and `RecurringExpense.note` exists in the data model (`docs/01-data-model.md:104`). For parity, consider rendering `item.recurring.note` in the same spot. Issue #6 doesn't specify row layout, so this is a design call, not a bug. - **`setDate` / `setNote` / `setStartDate` / `setInterval` don't call `refreshIsValid()`** — flagged by the explore agent but not a real concern: those fields aren't part of the validity check, so omitting the call is correct (only `setAmount` and `setCategory` need it). - **`save()` early-returns silently on invalid input** (`?: return` at `AddEditExpenseScreenModel.kt:89-90` and `AddEditRecurringScreenModel.kt:109-110`) — defensive but the button is `enabled = isValid && !isLoading`, so the path is unreachable from the UI. A `require(...)` would catch bugs earlier if you prefer fail-fast. All test plan checkboxes are addressed by the implementation; the `./gradlew assembleDebug` one is the only one the agent could mark itself. The seven manual checkboxes will be exercised in a follow-up round.
@@ -0,0 +138,4 @@
@OptIn(ExperimentalMaterial3Api::class)
@Composable
private fun CategoryDropdownField(
Author
Owner

Nit: CategoryDropdownField is duplicated almost verbatim between AddEditExpenseScreen.kt (lines 141–180) and AddEditRecurringScreen.kt (lines 153–192) — only the label string resource differs. Extract it to ui/components/CategoryDropdownField.kt, taking the label: String as a parameter, so both screens share one implementation. The component already only depends on public models and strings.

**Nit:** `CategoryDropdownField` is duplicated almost verbatim between `AddEditExpenseScreen.kt` (lines 141–180) and `AddEditRecurringScreen.kt` (lines 153–192) — only the label string resource differs. Extract it to `ui/components/CategoryDropdownField.kt`, taking the `label: String` as a parameter, so both screens share one implementation. The component already only depends on public models and strings.
@@ -0,0 +19,4 @@
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.OutlinedTextField
import androidx.compose.material3.Scaffold
import androidx.compose.material3.Switch
Author
Owner

Nit: The import androidx.compose.material3.Switch is unused. The active toggle on the form is rendered via ToggleItem (from ui/components/CardSection.kt), which wraps Switch internally. Remove this import.

**Nit:** The `import androidx.compose.material3.Switch` is unused. The active toggle on the form is rendered via `ToggleItem` (from `ui/components/CardSection.kt`), which wraps `Switch` internally. Remove this import.
@@ -0,0 +150,4 @@
@OptIn(ExperimentalMaterial3Api::class)
@Composable
private fun CategoryDropdownField(
Author
Owner

Nit: The matching duplicate in the recurring screen. See the comment on AddEditExpenseScreen.kt:141 — the two CategoryDropdownField implementations should be consolidated into a single shared component in ui/components/.

**Nit:** The matching duplicate in the recurring screen. See the comment on `AddEditExpenseScreen.kt:141` — the two `CategoryDropdownField` implementations should be consolidated into a single shared component in `ui/components/`.
@@ -0,0 +414,4 @@
}
}
@Composable
Author
Owner

Suggestion: TabAwareFab (and the inner MiniFab) is defined as a private helper inside ExpenseListScreen.kt. The design doc and the implementor rules (docs/04-implementation-plan.md:288, .opencode/agent/implementor.md:156) describe the expanded mini-FAB as a shared component used by both HomeScreen and ExpenseListScreen. HomeScreen is currently a stub (ui/screens/home/HomeScreen.kt), so duplication isn't a problem yet — but once the home screen grows its FAB, move TabAwareFab + MiniFab into ui/components/TabAwareFab.kt (taking the action list as a parameter) so both screens share the same expansion UI and only differ in which actions they pass in.

**Suggestion:** `TabAwareFab` (and the inner `MiniFab`) is defined as a private helper inside `ExpenseListScreen.kt`. The design doc and the implementor rules (`docs/04-implementation-plan.md:288`, `.opencode/agent/implementor.md:156`) describe the expanded mini-FAB as a shared component used by both `HomeScreen` and `ExpenseListScreen`. `HomeScreen` is currently a stub (`ui/screens/home/HomeScreen.kt`), so duplication isn't a problem yet — but once the home screen grows its FAB, move `TabAwareFab` + `MiniFab` into `ui/components/TabAwareFab.kt` (taking the action list as a parameter) so both screens share the same expansion UI and only differ in which actions they pass in.
@@ -0,0 +106,4 @@
private fun matchesQuery(item: ExpenseWithCategory, query: String): Boolean {
if (query.isBlank()) return true
val note = item.expense.note.orEmpty()
val amountStr = item.expense.amount.toString()
Author
Owner

Suggestion: item.expense.amount.toString() uses Java's default Double.toString, which is locale-naive and uses scientific notation for very large or very small values. A user searching for "1000000" won't find an amount of 1_000_000.0 (which prints as "1.0E6"), and a user searching for "12.34" won't find 12.345 (which prints as "12.345", not the "12.35" they see in the row). For consistency with the display format in ExpenseRow ("%.2f".format(item.expense.amount) at ExpenseListScreen.kt:359), use the same formatter here — or strip scientific notation explicitly.

**Suggestion:** `item.expense.amount.toString()` uses Java's default `Double.toString`, which is locale-naive and uses scientific notation for very large or very small values. A user searching for `"1000000"` won't find an amount of `1_000_000.0` (which prints as `"1.0E6"`), and a user searching for `"12.34"` won't find `12.345` (which prints as `"12.345"`, not the `"12.35"` they see in the row). For consistency with the display format in `ExpenseRow` (`"%.2f".format(item.expense.amount)` at `ExpenseListScreen.kt:359`), use the same formatter here — or strip scientific notation explicitly.
@@ -64,0 +79,4 @@
<string name="expense_list_delete_message">This will permanently delete the expense.</string>
<string name="expense_list_empty">No expenses yet</string>
<string name="recurring_list_empty">No recurring expenses</string>
<string name="recurring_list_active">Active</string>
Author
Owner

Nit: recurring_list_active is defined here but never referenced anywhere in the new code (I searched the whole tree for R.string.recurring_list_active and there are zero call sites). Either remove it or wire it up — e.g., as a label next to the Switch in RecurringRow (ExpenseListScreen.kt:410-413).

**Nit:** `recurring_list_active` is defined here but never referenced anywhere in the new code (I searched the whole tree for `R.string.recurring_list_active` and there are zero call sites). Either remove it or wire it up — e.g., as a label next to the `Switch` in `RecurringRow` (`ExpenseListScreen.kt:410-413`).
admin added 1 commit 2026-06-28 12:49:04 +00:00
- matchesQuery: use %.2f format to match the display (was Double.toString, which can use scientific notation and disagree with the row's %.2f)
- Remove unused Switch import in AddEditRecurringScreen (the active toggle uses ToggleItem)
- Extract CategoryDropdownField to ui/components/CategoryDropdownField.kt so both add/edit screens share one implementation; takes label as a parameter
- Remove unused recurring_list_active string
admin reviewed 2026-06-28 12:55:04 +00:00
admin left a comment
Author
Owner

Summary

Verdict: still clean — all blocking concerns were already addressed; all 4 of the inline comments you addressed in e8c7a14 are correctly applied.

Verified each of the 4 fixes against the new commit:

  • matchesQuery format (ExpenseListScreenModel.kt:109) — "%.2f".format(item.expense.amount) now matches the row display. 1_000_000.0 will still render as "1000000.00" rather than "1.0E6". The single-line hunk is minimal and doesn't touch anything else.
  • Switch import removed (AddEditRecurringScreen.kt) — the import androidx.compose.material3.Switch line is gone; the active toggle continues to use ToggleItem (which wraps Switch internally via CardSection.kt). Grep confirms no other Switch reference in the file.
  • CategoryDropdownField extracted to ui/components/CategoryDropdownField.kt — new file is 61 lines, takes label: String as a parameter (4th positional), and both screens now call it with their own stringResource(R.string.add_edit_expense_field_category) / stringResource(R.string.add_edit_recurring_field_category). The two old inlined definitions were removed, the now-unused DropdownMenuItem / ExposedDropdownMenu* / mutableStateOf imports in both screens were dropped. No duplication remains.
  • recurring_list_active string removed from strings.xml — confirmed absent from the file.

Scope discipline: 5 files, 66 insertions, 106 deletions — every changed line maps to one of the 4 fixes, no drive-by edits, no other files modified.

The two items you left unaddressed are fine to leave as-is:

  • TabAwareFab extraction to ui/components/ — the design doc has HomeScreen and ExpenseListScreen sharing the same expanded mini-FAB, but HomeScreen is still a stub (ui/screens/home/HomeScreen.kt:6), so there's no current duplication. Extract this when HomeScreen actually grows its FAB; for now, leaving it local is the right call.
  • RecurringRow not showing note — I flagged this as a parity/design call, not a bug, and issue #6 doesn't specify row layout. Defer to the design pass.

The previous review is now stale; this one supersedes it. No new inline comments. The PR is ready from a review perspective — ./gradlew assembleDebug passes per the test plan, and the 7 manual checkboxes will be exercised on a follow-up round.

## Summary **Verdict: still clean — all blocking concerns were already addressed; all 4 of the inline comments you addressed in `e8c7a14` are correctly applied.** Verified each of the 4 fixes against the new commit: - **`matchesQuery` format** (`ExpenseListScreenModel.kt:109`) — `"%.2f".format(item.expense.amount)` now matches the row display. `1_000_000.0` will still render as `"1000000.00"` rather than `"1.0E6"`. The single-line hunk is minimal and doesn't touch anything else. - **`Switch` import removed** (`AddEditRecurringScreen.kt`) — the `import androidx.compose.material3.Switch` line is gone; the active toggle continues to use `ToggleItem` (which wraps `Switch` internally via `CardSection.kt`). Grep confirms no other `Switch` reference in the file. - **`CategoryDropdownField` extracted** to `ui/components/CategoryDropdownField.kt` — new file is 61 lines, takes `label: String` as a parameter (4th positional), and both screens now call it with their own `stringResource(R.string.add_edit_expense_field_category)` / `stringResource(R.string.add_edit_recurring_field_category)`. The two old inlined definitions were removed, the now-unused `DropdownMenuItem` / `ExposedDropdownMenu*` / `mutableStateOf` imports in both screens were dropped. No duplication remains. - **`recurring_list_active` string removed** from `strings.xml` — confirmed absent from the file. Scope discipline: 5 files, 66 insertions, 106 deletions — every changed line maps to one of the 4 fixes, no drive-by edits, no other files modified. The two items you left unaddressed are fine to leave as-is: - **TabAwareFab extraction to `ui/components/`** — the design doc has `HomeScreen` and `ExpenseListScreen` sharing the same expanded mini-FAB, but `HomeScreen` is still a stub (`ui/screens/home/HomeScreen.kt:6`), so there's no current duplication. Extract this when `HomeScreen` actually grows its FAB; for now, leaving it local is the right call. - **`RecurringRow` not showing `note`** — I flagged this as a parity/design call, not a bug, and issue #6 doesn't specify row layout. Defer to the design pass. The previous review is now stale; this one supersedes it. No new inline comments. The PR is ready from a review perspective — `./gradlew assembleDebug` passes per the test plan, and the 7 manual checkboxes will be exercised on a follow-up round.
admin merged commit 8ce0dcc678 into main 2026-06-28 12:57:17 +00:00
admin deleted branch feat/6-implement-expense-list-add-edit-screens 2026-06-28 15:01:55 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/ledgerr#18