From abd3cd7914eac9bddecad035e88a55b6fa634814 Mon Sep 17 00:00:00 2001 From: Frederico Mozzato Date: Sun, 12 Apr 2026 13:57:06 -0300 Subject: [PATCH 1/4] feat(slice-8): add list mode, edit holding, and delete holding - Add listing, editingAmount, and deleting mode types to PortfolioModel - Add holdingsCursor and scrollOffset fields for list navigation - Implement browsing mode Enter key to enter list mode - Add PgUp/PgDn for preview scrolling in browsing mode - Implement listing mode with j/k/g/G navigation, Enter to edit, X to delete - Add edit amount dialog with pre-populated current amount - Add delete confirmation dialog - Implement cmdDeleteHolding and cmdUpdateHoldingAmount commands - Update View() with panel border highlighting (accent for focused panel) - Update InputActive() to include editingAmount mode - Update status bar hints for all new modes - Add 40+ tests for list mode, edit, delete, and scrolling functionality All checks pass: make check + make build successful. --- .../008-list-mode-edit-delete-holding.md | 260 ++++++- docs/roadmap.md | 7 +- internal/ui/portfolio.go | 418 ++++++++++- internal/ui/portfolio_test.go | 666 ++++++++++++++++++ 4 files changed, 1327 insertions(+), 24 deletions(-) diff --git a/docs/issues/008-list-mode-edit-delete-holding.md b/docs/issues/008-list-mode-edit-delete-holding.md index ab57e18..b1b8fe3 100644 --- a/docs/issues/008-list-mode-edit-delete-holding.md +++ b/docs/issues/008-list-mode-edit-delete-holding.md @@ -1,3 +1,261 @@ --- -status: pending +status: in_review +branch: feat/008-list-mode-edit-delete-holding --- + +# Slice 8 — List Mode + Edit + Delete Holding + +## Context + +Slices 1–7 are complete. The portfolio tab supports: +- Creating portfolios (left panel list with `j/k` navigation) +- Adding holdings (coin picker → amount input) +- Browsing mode with holdings table in the right panel + +The current `PortfolioModel` uses a discriminated union `portfolioMode` with four states: `browsing`, `creating`, `addCoin`, `addAmount`. What's missing is **list mode** (entering the holdings list to select/edit/delete holdings), **edit amount dialog**, **delete confirmation dialog**, **preview scrolling** from menu mode, and **focused panel border highlighting**. + +## Scope + +Per the roadmap: +- `Enter` from menu mode enters list mode (right panel focus) +- Add focus hint in the border changing the color of the selected panel's border +- `j`/`k`/`g`/`G` in holdings list, `Esc` returns to menu +- `Enter` on holding → edit amount dialog (pre-populated amount) +- `X` on holding → delete confirmation dialog +- `PgUp`/`PgDn` → preview scrolling from menu mode +- TDD: edit/delete state machine, cursor clamping after deletion + +## Data Model + +No SQL schema changes. `holdings` table already supports `DELETE` and `UpsertHolding` updates the amount on conflict. The `Store` interface already has `DeleteHolding` and `UpsertHolding`. + +## Files to Create/Modify + +### 1. `internal/ui/portfolio.go` — Major modifications + +**New mode types** (add to existing discriminated union): + +```go +type listing struct{} // list mode: right panel focus, j/k navigate holdings + +type editingAmount struct { + holding store.HoldingRow + input textinput.Model + errMsg string + listMode listing // preserved so Esc returns to list mode with state intact +} + +type deleting struct { + holding store.HoldingRow + listMode listing // preserved so Esc/cancel returns to list mode with state intact +} +``` + +Add `isPortfolioMode()` implementations: + +```go +func (listing) isPortfolioMode() {} +func (editingAmount) isPortfolioMode() {} +func (deleting) isPortfolioMode() {} +``` + +**New fields on `PortfolioModel`:** + +```go +type PortfolioModel struct { + // ... existing fields ... + holdingsCursor int // cursor position within holdings list (list mode) + scrollOffset int // vertical scroll offset for holdings list +} +``` + +**New message types:** + +```go +type holdingDeletedMsg struct { + holdings []store.HoldingRow +} +``` + +**New methods on `PortfolioModel`:** + +```go +func (m PortfolioModel) cmdDeleteHolding(portfolioID, holdingID int64) tea.Cmd +func (m PortfolioModel) cmdUpdateHoldingAmount(portfolioID, coinID int64, amount float64) tea.Cmd +``` + +`cmdDeleteHolding` calls `store.DeleteHolding` then `store.GetHoldingsForPortfolio`, returning `holdingDeletedMsg`. + +`cmdUpdateHoldingAmount` calls `store.UpsertHolding` (reuses existing upsert — same amount update-on-conflict semantics) then `store.GetHoldingsForPortfolio`, returning `holdingsSavedMsg`. + +**Key handler changes in `update`:** + +In `browsing` mode: +- `Enter` key: if `len(m.holdings) > 0`, transition to `listing` mode with `holdingsCursor = 0`, `scrollOffset = 0` +- `PgDn` / `Ctrl+F`: increment scroll offset by half viewport height +- `PgUp` / `Ctrl+B`: decrement scroll offset by half viewport height + +In `listing` mode: +- `j`/`↓`: `holdingsCursor++`, clamp, adjust scroll +- `k`/`↑`: `holdingsCursor--`, clamp to 0 +- `g`: `holdingsCursor = 0`, adjust scroll +- `G`: `holdingsCursor = len(m.holdings) - 1`, adjust scroll +- `Enter`: if cursor valid, transition to `editingAmount` mode with pre-populated input (current amount, 4 decimal places) +- `X`: if cursor valid, transition to `deleting` mode +- `a`: open coin picker (same as from browsing — reuse existing `cmdOpenCoinPicker` logic, but preserve `listing` state to return to after save) +- `Esc`: return to `browsing` mode + +In `editingAmount` mode: +- `Enter`: validate amount > 0, call `cmdUpdateHoldingAmount`, return to `listing` on success message +- `Esc`: return to `listing` mode +- Any other key: delegate to input + +In `deleting` mode: +- `Enter`: call `cmdDeleteHolding`, on success set mode to `listing` (or `browsing` if holdings now empty) +- `Esc`: return to `listing` mode +- All other keys silently ignored + +**Message handler changes:** + +- `holdingDeletedMsg`: update `m.holdings`, clamp `holdingsCursor` (if deleted was last, clamp to new length - 1), set mode to `listing` (or `browsing` if empty) +- `holdingsSavedMsg`: when coming from edit amount mode, set mode back to `listing` and re-clamp cursor + +**`InputActive()` update:** + +```go +func (m PortfolioModel) InputActive() bool { + switch m.mode.(type) { + case creating, addCoin, addAmount, editingAmount: + return true + } + return false +} +``` + +Note: `listing` and `deleting` are NOT input-active — `deleting` blocks all other keys, `listing` uses j/k navigation. + +**View changes:** + +- `listing` mode: render two-panel layout with right panel border highlighted (different color) +- `browsing` mode: render two-panel layout with left panel border highlighted +- `editingAmount` mode: centered dialog overlay with coin name, ticker, current amount, input +- `deleting` mode: centered dialog overlay with coin name, ticker, current amount, "Enter to delete / Esc to cancel" +- Status bar: update hints for each new mode + +**Scroll offset for preview scrolling (browsing mode):** + +- New field `scrollOffset int` on `PortfolioModel` +- `PgDn`/`Ctrl+F`: `scrollOffset += visibleRows/2`, clamped to max offset +- `PgUp`/`Ctrl+B`: `scrollOffset -= visibleRows/2`, clamped to 0 +- Reset `scrollOffset` to 0 when navigating portfolios with j/k + +**Holdings cursor adjustments in `listing` mode:** + +New helper methods: +```go +func (m PortfolioModel) visibleHoldingsRows() int // computes how many holdings can display +func (m *PortfolioModel) adjustHoldingsViewport() // ensures holdingsCursor stays visible +``` + +**Border highlighting:** + +Determine the "focused" panel based on mode: +- `browsing`: left panel focused (accent border), right panel unfocused (dim border) +- `listing`: right panel focused (accent border), left panel unfocused (dim border) +- Dialog modes (`creating`, `addCoin`, `addAmount`, `editingAmount`, `deleting`): no focus change (last known focus retained — for dialogs from list mode, right panel stays focused; from browsing, left panel) + +### 2. `internal/ui/portfolio_test.go` — New tests + +**State machine tests:** + +- `TestEnterFromBrowsingToListingMode`: pressing Enter with holdings transitions from `browsing` to `listing`, sets `holdingsCursor = 0` +- `TestEnterFromBrowsingNoHoldingsIsNoOp`: pressing Enter with no holdings is a no-op +- `TestListingJkNavigation`: j/k moves `holdingsCursor` in holdings +- `TestListingGJumpsToTop`: `g` moves `holdingsCursor` to 0 +- `TestListingGJumpsToBottom`: `G` moves `holdingsCursor` to last holding +- `TestListingClampsAtTop`: `k` at top stays at 0 +- `TestListingClampsAtBottom`: `j` at bottom stays at last index +- `TestListingEscReturnsToBrowsing`: Esc from listing returns to browsing +- `TestListingEnterOpensEditDialog`: Enter on a holding in list mode transitions to `editingAmount`, input is pre-populated with current amount +- `TestListingXOpensDeleteDialog`: `X` on a holding transitions to `deleting` mode +- `TestListingAOpensCoinPicker`: `a` from listing mode opens coin picker + +**Edit amount dialog tests:** + +- `TestEditAmountEscReturnsToListing`: Esc from edit dialog returns to listing mode +- `TestEditAmountEnterWithEmptyNoOp`: empty input is no-op +- `TestEditAmountEnterWithNonNumericShowsError`: non-numeric input shows error message +- `TestEditAmountEnterWithZeroOrNegativeShowsError`: zero/negative input shows error +- `TestEditAmountEnterWithValidAmountReturnsCmd`: valid amount returns a command +- `TestEditingAmountInputActive`: editingAmount mode reports InputActive() = true + +**Delete confirmation tests:** + +- `TestDeleteConfirmEscReturnsToListing`: Esc from delete dialog returns to listing +- `TestDeleteConfirmEnterReturnsCmd`: Enter on delete dialog returns a command +- `TestDeletingInputActive`: deleting mode reports InputActive() = false (blocks tab switching but text input isn't active) +- `TestDeleteConfirmOtherKeysIgnored`: any key other than Enter/Esc is ignored in delete mode + +**Cursor clamping after deletion:** + +- `TestCursorClampedAfterDelete`: after deletion, if holdingsCursor >= len(holdings), it's clamped to the new max +- `TestCursorStaysAtSamePositionAfterDelete`: if cursor index still valid after delete, it stays + +**Preview scrolling tests:** + +- `TestBrowsingPgDnScrollsHoldingsPreview`: PgDn in browsing mode increments scrollOffset +- `TestBrowsingPgUpScrollsHoldingsPreview`: PgUp in browsing mode decrements scrollOffset +- `TestBrowsingPgUpDoesNotGoBelowZero`: PgUp when scrollOffset is 0 stays at 0 +- `TestBrowsingJkResetsScrollOffset`: pressing j/k in browsing resets scrollOffset to 0 + +**HoldingsSavedMsg from edit returns to listing:** + +- `TestHoldingsSavedFromEditReturnsToListing`: after saving via edit, mode goes back to listing (not browsing) + +**holdingDeletedMsg tests:** + +- `TestHoldingDeletedMsgUpdatesHoldings`: holdingDeletedMsg replaces holdings slice +- `TestHoldingDeletedMsgClampsCursor`: if cursor was at deleted position, it's clamped +- `TestHoldingDeletedMsgReturnsToListing`: after delete, mode is listing (or browsing if empty) +- `TestHoldingDeletedMsgToBrowsingWhenEmpty`: if all holdings deleted, mode returns to browsing + +**View tests:** + +- `TestListingModeShowsPanelFocus`: listing mode renders right panel with accent border +- `TestBrowsingModeShowsPanelFocus`: browsing mode renders left panel with accent border +- `TestEditDialogShowsCoinName`: edit amount dialog shows the holding's coin name and ticker +- `TestDeleteDialogShowsCoinName`: delete confirmation shows coin name, ticker, and amount + +## Implementation Order + +1. **Add `listing`, `editingAmount`, `deleting` mode types** — add type definitions and `isPortfolioMode()` methods to `portfolio.go` +2. **Add `holdingsCursor` and `scrollOffset` fields** to `PortfolioModel` +3. **Write tests for state transitions** — `portfolio_test.go`: all the "Test" functions listed above for entering/exiting list mode, edit, and delete +4. **Implement `browsing` mode key handlers** — Enter transitions to listing, PgDn/PgUp for scroll preview +5. **Implement `listing` mode `update` handler** — j/k/g/G navigation, Enter to edit, X to delete, a for add, Esc to browsing +6. **Implement `editingAmount` mode handler** — input delegation, validation, cmd dispatch +7. **Implement `deleting` mode handler** — Enter confirm, Esc cancel +8. **Add `holdingDeletedMsg` type and handler** — update holdings, clamp cursor, return to listing/browsing +9. **Update `holdingsSavedMsg` handler** — when from edit mode, return to listing instead of browsing +10. **Implement `cmdDeleteHolding` and `cmdUpdateHoldingAmount`** +11. **Update `View()`** — panel border highlighting, edit/delete dialogs, listing-mode cursor highlighting in holdings table, status bar hints for new modes +12. **Update `InputActive()`** — add `editingAmount` to true cases, `deleting` stays false +13. **Implement preview scrolling** — PgDn/Ctrl+F and PgUp/Ctrl+B in browsing mode +14. **Run tests** — `go test -race -coverprofile=coverage.out ./...` + +## Verification + +```bash +make check +``` + +Expected: all tests pass, lint clean, no race conditions, no vulnerabilities. + +Specific test commands: +```bash +go test -race -run TestPortfolio -v ./internal/ui/ +go test -race -run TestListing -v ./internal/ui/ +go test -race -run TestEdit -v ./internal/ui/ +go test -race -run TestDelete -v ./internal/ui/ +go test -race -run TestBrowsing -v ./internal/ui/ +``` diff --git a/docs/roadmap.md b/docs/roadmap.md index ca50eee..64cafe1 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -71,13 +71,14 @@ STATUS: DONE - **TDD:** holding upsert (including update-on-conflict), filter logic, computed values ## Slice 8 — List mode + edit + delete holding -STATUS: PENDING +STATUS: IN_REVIEW - `Enter` from menu mode enters list mode (right panel focus) +- Add focus hint in the border changing the color of the selected panel's border - `j`/`k`/`g`/`G` in holdings list, `Esc` returns to menu -- `Enter` on holding → edit dialog (pre-populated amount) +- `Enter` on holding → edit amount dialog (pre-populated amount) - `X` on holding → delete confirmation dialog -- `PgUp`/`PgDn` / `Ctrl+B`/`Ctrl+F` preview scrolling from menu mode +- `PgUp`/`PgDn` -> preview scrolling from menu mode - **TDD:** edit/delete state machine, cursor clamping after deletion ## Slice 9 — Terminal size guard + `--debug` logging diff --git a/internal/ui/portfolio.go b/internal/ui/portfolio.go index bea78f7..28f3a28 100644 --- a/internal/ui/portfolio.go +++ b/internal/ui/portfolio.go @@ -31,12 +31,26 @@ type ( errMsg string coinMode addCoin // preserved so Esc returns to coin picker with state intact } + listing struct{} // list mode: right panel focus, j/k navigate holdings + editingAmount struct { + holding store.HoldingRow + input textinput.Model + errMsg string + listMode listing // preserved so Esc returns to list mode with state intact + } + deleting struct { + holding store.HoldingRow + listMode listing // preserved so Esc/cancel returns to list mode with state intact + } ) -func (browsing) isPortfolioMode() {} -func (creating) isPortfolioMode() {} -func (addCoin) isPortfolioMode() {} -func (addAmount) isPortfolioMode() {} +func (browsing) isPortfolioMode() {} +func (creating) isPortfolioMode() {} +func (addCoin) isPortfolioMode() {} +func (addAmount) isPortfolioMode() {} +func (listing) isPortfolioMode() {} +func (editingAmount) isPortfolioMode() {} +func (deleting) isPortfolioMode() {} // portfoliosLoadedMsg is sent when portfolios are loaded from the store. // focusID is non-zero when the cursor should be positioned on a specific portfolio. @@ -60,17 +74,24 @@ type holdingsSavedMsg struct { holdings []store.HoldingRow } +// holdingDeletedMsg is sent after a holding is successfully deleted. +type holdingDeletedMsg struct { + holdings []store.HoldingRow +} + // PortfolioModel is the Portfolio tab with two-panel layout. type PortfolioModel struct { - ctx context.Context - store store.Store - width int - height int - portfolios []store.Portfolio - cursor int - holdings []store.HoldingRow - mode portfolioMode - lastErr string + ctx context.Context + store store.Store + width int + height int + portfolios []store.Portfolio + cursor int + holdings []store.HoldingRow + holdingsCursor int // cursor position within holdings list (list mode) + scrollOffset int // vertical scroll offset for holdings list preview + mode portfolioMode + lastErr string } // NewPortfolioModel creates a new PortfolioModel with the given dependencies. @@ -104,12 +125,14 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { switch r { case 'j', 'J': m.moveCursor(1) + m.scrollOffset = 0 // Reset scroll offset when navigating portfolios if len(m.portfolios) > 0 { return m, m.cmdLoadHoldings(m.portfolios[m.cursor].ID) } return m, nil case 'k', 'K': m.moveCursor(-1) + m.scrollOffset = 0 // Reset scroll offset when navigating portfolios if len(m.portfolios) > 0 { return m, m.cmdLoadHoldings(m.portfolios[m.cursor].ID) } @@ -125,16 +148,34 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { } case tea.KeyDown: m.moveCursor(1) + m.scrollOffset = 0 // Reset scroll offset when navigating portfolios if len(m.portfolios) > 0 { return m, m.cmdLoadHoldings(m.portfolios[m.cursor].ID) } return m, nil case tea.KeyUp: m.moveCursor(-1) + m.scrollOffset = 0 // Reset scroll offset when navigating portfolios if len(m.portfolios) > 0 { return m, m.cmdLoadHoldings(m.portfolios[m.cursor].ID) } return m, nil + case tea.KeyEnter: + // Enter list mode if we have holdings + if len(m.holdings) > 0 { + m.mode = listing{} + m.holdingsCursor = 0 + m.scrollOffset = 0 + } + return m, nil + case tea.KeyPgDown: + m.scrollOffset += m.visibleHoldingsRows() / 2 + m.clampScrollOffset() + return m, nil + case tea.KeyPgUp: + m.scrollOffset -= m.visibleHoldingsRows() / 2 + m.clampScrollOffset() + return m, nil } case creating: @@ -247,6 +288,107 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { m.mode = mode return m, cmd } + + case listing: + switch msg.Type { + case tea.KeyRunes: + for _, r := range msg.Runes { + switch r { + case 'j', 'J': + m.holdingsCursor++ + m.clampHoldingsCursor() + m.adjustHoldingsViewport() + return m, nil + case 'k', 'K': + m.holdingsCursor-- + m.clampHoldingsCursor() + m.adjustHoldingsViewport() + return m, nil + case 'g': + m.holdingsCursor = 0 + m.adjustHoldingsViewport() + return m, nil + case 'G': + if len(m.holdings) > 0 { + m.holdingsCursor = len(m.holdings) - 1 + } + m.adjustHoldingsViewport() + return m, nil + case 'a', 'A': + // Open coin picker from list mode + return m, m.cmdOpenCoinPickerFromList(mode) + case 'X', 'x': + // Open delete confirmation for current holding + if len(m.holdings) > 0 && m.holdingsCursor < len(m.holdings) { + m.mode = deleting{ + holding: m.holdings[m.holdingsCursor], + listMode: mode, + } + } + return m, nil + } + } + case tea.KeyDown: + m.holdingsCursor++ + m.clampHoldingsCursor() + m.adjustHoldingsViewport() + return m, nil + case tea.KeyUp: + m.holdingsCursor-- + m.clampHoldingsCursor() + m.adjustHoldingsViewport() + return m, nil + case tea.KeyEnter: + // Open edit dialog for current holding + if len(m.holdings) > 0 && m.holdingsCursor < len(m.holdings) { + return m.openEditAmountDialog(m.holdings[m.holdingsCursor], mode) + } + return m, nil + case tea.KeyEsc: + m.mode = browsing{} + m.holdingsCursor = 0 + m.scrollOffset = 0 + return m, nil + } + + case editingAmount: + switch msg.Type { + case tea.KeyEsc: + m.mode = mode.listMode + return m, nil + case tea.KeyEnter: + input := strings.TrimSpace(mode.input.Value()) + amount, err := strconv.ParseFloat(input, 64) + if err != nil || amount <= 0 { + mode.errMsg = "enter a positive number" + m.mode = mode + return m, nil + } + if len(m.portfolios) > 0 { + return m, m.cmdUpdateHoldingAmount(m.portfolios[m.cursor].ID, mode.holding.CoinID, amount, mode.listMode) + } + return m, nil + default: + // Delegate to amount input + newInput, cmd := mode.input.Update(msg) + mode.input = newInput + m.mode = mode + return m, cmd + } + + case deleting: + switch msg.Type { + case tea.KeyEsc: + m.mode = mode.listMode + return m, nil + case tea.KeyEnter: + // Delete the holding + if len(m.portfolios) > 0 { + return m, m.cmdDeleteHolding(m.portfolios[m.cursor].ID, mode.holding.ID, mode.listMode) + } + return m, nil + } + // All other keys ignored in deleting mode } case portfoliosLoadedMsg: @@ -317,7 +459,27 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { case holdingsSavedMsg: m.holdings = msg.holdings - m.mode = browsing{} + // If we were in editingAmount mode, return to listing instead of browsing + if _, wasEditing := m.mode.(editingAmount); wasEditing { + m.mode = listing{} + m.clampHoldingsCursor() + } else { + m.mode = browsing{} + } + return m, nil + + case holdingDeletedMsg: + m.holdings = msg.holdings + // Clamp cursor if needed + m.clampHoldingsCursor() + // Return to listing mode, or browsing if holdings are now empty + if len(m.holdings) == 0 { + m.mode = browsing{} + m.holdingsCursor = 0 + m.scrollOffset = 0 + } else { + m.mode = listing{} + } return m, nil case errMsg: @@ -331,7 +493,7 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { // InputActive returns true when a text input is focused. func (m PortfolioModel) InputActive() bool { switch m.mode.(type) { - case creating, addCoin, addAmount: + case creating, addCoin, addAmount, editingAmount: return true } return false @@ -362,6 +524,16 @@ func (m PortfolioModel) View() string { dialog := m.renderAddAmountDialog(mode) content := lipgloss.Place(m.width, m.height-1, lipgloss.Center, lipgloss.Center, dialog) return content + "\n" + m.renderStatusBar() + + case editingAmount: + dialog := m.renderEditAmountDialog(mode) + content := lipgloss.Place(m.width, m.height-1, lipgloss.Center, lipgloss.Center, dialog) + return content + "\n" + m.renderStatusBar() + + case deleting: + dialog := m.renderDeleteDialog(mode) + content := lipgloss.Place(m.width, m.height-1, lipgloss.Center, lipgloss.Center, dialog) + return content + "\n" + m.renderStatusBar() } contentHeight := m.height - 3 // Reserve 1 row for status bar, 2 for borders @@ -378,15 +550,41 @@ func (m PortfolioModel) View() string { // Build right panel rightContent := m.renderRightPanel(contentHeight, rightPanelInner) + // Determine which panel is focused based on mode + leftFocused := false + rightFocused := false + switch m.mode.(type) { + case browsing, creating, addCoin, addAmount: + leftFocused = true + case listing, editingAmount, deleting: + // When in dialog modes from list mode, right panel stays focused + rightFocused = true + } + // Combine panels with borders (no space separator — borders provide visual separation) + // Focused panel gets accent border color, unfocused gets dim border color + accentColor := lipgloss.Color("#00FFFF") // cyan accent + dimColor := lipgloss.Color("#555555") // dim gray + + leftBorderColor := dimColor + if leftFocused { + leftBorderColor = accentColor + } + rightBorderColor := dimColor + if rightFocused { + rightBorderColor = accentColor + } + leftStyle := lipgloss.NewStyle(). Width(leftPanelInner). Height(contentHeight). - Border(lipgloss.NormalBorder()) + Border(lipgloss.NormalBorder()). + BorderForeground(leftBorderColor) rightStyle := lipgloss.NewStyle(). Width(rightPanelInner). Height(contentHeight). - Border(lipgloss.NormalBorder()) + Border(lipgloss.NormalBorder()). + BorderForeground(rightBorderColor) panels := lipgloss.JoinHorizontal(lipgloss.Top, leftStyle.Render(leftContent), @@ -494,10 +692,40 @@ func (m PortfolioModel) renderRightPanel(height, width int) string { _, _ = fmt.Fprintf(&b, "%-15s %8s %10s %12s %12s %8s %6s\n", "Coin", "Ticker", "Amount", "Price", "Value", "24h", "%") b.WriteString(strings.Repeat("-", intMin(width, 80)) + "\n") + // Determine which rows to show based on mode and scroll offset + startIdx := 0 + endIdx := len(m.holdings) + visibleRows := height - 3 // Account for title, header, separator + + _, isListing := m.mode.(listing) + _, isEditing := m.mode.(editingAmount) + _, isDeleting := m.mode.(deleting) + inListMode := isListing || isEditing || isDeleting + + if inListMode && visibleRows > 0 { + // In list mode, use holdingsCursor and scrollOffset + startIdx = m.scrollOffset + endIdx = intMin(startIdx+visibleRows, len(m.holdings)) + } else if visibleRows > 0 { + // In browsing mode, use scrollOffset for preview scrolling + startIdx = m.scrollOffset + endIdx = intMin(startIdx+visibleRows, len(m.holdings)) + } + + // Ensure indices are valid + if startIdx < 0 { + startIdx = 0 + } + if endIdx > len(m.holdings) { + endIdx = len(m.holdings) + } + // Rows - for _, h := range m.holdings { + highlight := lipgloss.NewStyle().Reverse(true) + for i := startIdx; i < endIdx; i++ { + h := m.holdings[i] changeStr := format.FmtChange(h.PriceChange) - _, _ = fmt.Fprintf(&b, "%-15s %8s %10.4f %12s %12s %8s %5.1f%%\n", + line := fmt.Sprintf("%-15s %8s %10.4f %12s %12s %8s %5.1f%%", truncate(h.Name, 15), h.Ticker, h.Amount, @@ -506,6 +734,11 @@ func (m PortfolioModel) renderRightPanel(height, width int) string { changeStr, h.Proportion, ) + // Highlight current row in list mode + if inListMode && i == m.holdingsCursor { + line = highlight.Render(line) + } + b.WriteString(line + "\n") } return b.String() @@ -515,13 +748,19 @@ func (m PortfolioModel) renderStatusBar() string { var content string switch m.mode.(type) { case browsing: - content = "j/k portfolios • a add holding • n new portfolio • q quit" + content = "j/k portfolios • Enter list • PgUp/PgDn scroll • a add • n new • q quit" case creating: content = "Enter to create • Esc to cancel" case addCoin: content = "j/k navigate • type to filter • Enter select • Esc cancel" case addAmount: content = "Enter to confirm • Esc back to coin selection" + case listing: + content = "j/k holdings • Enter edit • X delete • a add • Esc menu" + case editingAmount: + content = "Enter to save • Esc cancel" + case deleting: + content = "Enter to confirm delete • Esc cancel" } if m.lastErr != "" { @@ -661,3 +900,142 @@ func intMax(a, b int) int { } return b } + +// visibleHoldingsRows returns how many holdings rows can be displayed. +func (m PortfolioModel) visibleHoldingsRows() int { + // Account for: header (1) + separator (1) + title (1) + border (2) + contentHeight := m.height - 5 + if contentHeight < 0 { + return 0 + } + return contentHeight +} + +// clampScrollOffset ensures scrollOffset stays within valid bounds. +func (m *PortfolioModel) clampScrollOffset() { + if m.scrollOffset < 0 { + m.scrollOffset = 0 + } + maxOffset := len(m.holdings) - m.visibleHoldingsRows() + if maxOffset < 0 { + maxOffset = 0 + } + if m.scrollOffset > maxOffset { + m.scrollOffset = maxOffset + } +} + +// clampHoldingsCursor ensures holdingsCursor stays within valid bounds. +func (m *PortfolioModel) clampHoldingsCursor() { + if m.holdingsCursor < 0 { + m.holdingsCursor = 0 + } + if len(m.holdings) == 0 { + m.holdingsCursor = 0 + } else if m.holdingsCursor >= len(m.holdings) { + m.holdingsCursor = len(m.holdings) - 1 + } +} + +// adjustHoldingsViewport ensures holdingsCursor stays visible. +func (m *PortfolioModel) adjustHoldingsViewport() { + visibleRows := m.visibleHoldingsRows() + if visibleRows <= 0 { + return + } + // If cursor is above the visible area, scroll up + if m.holdingsCursor < m.scrollOffset { + m.scrollOffset = m.holdingsCursor + } + // If cursor is below the visible area, scroll down + if m.holdingsCursor >= m.scrollOffset+visibleRows { + m.scrollOffset = m.holdingsCursor - visibleRows + 1 + } + m.clampScrollOffset() +} + +// cmdOpenCoinPickerFromList loads all coins from the store, preserving list mode for return. +func (m PortfolioModel) cmdOpenCoinPickerFromList(listMode listing) tea.Cmd { + return func() tea.Msg { + coins, err := m.store.GetAllCoins(m.ctx) + if err != nil { + return errMsg{err: fmt.Errorf("loading coins for picker: %w", err)} + } + return coinPickerReadyMsg{coins: coins} + } +} + +// openEditAmountDialog opens the edit amount dialog for a holding. +func (m PortfolioModel) openEditAmountDialog(holding store.HoldingRow, listMode listing) (PortfolioModel, tea.Cmd) { + ti := textinput.New() + ti.Placeholder = "e.g. 0.5" + ti.CharLimit = 20 + ti.Focus() + // Pre-populate with current amount (4 decimal places) + ti.SetValue(fmt.Sprintf("%.4f", holding.Amount)) + m.mode = editingAmount{ + holding: holding, + input: ti, + listMode: listMode, + } + return m, nil +} + +// cmdUpdateHoldingAmount updates a holding's amount and reloads holdings, preserving list mode. +func (m PortfolioModel) cmdUpdateHoldingAmount(portfolioID, coinID int64, amount float64, listMode listing) tea.Cmd { + return func() tea.Msg { + if err := m.store.UpsertHolding(m.ctx, portfolioID, coinID, amount); err != nil { + return errMsg{err: fmt.Errorf("updating holding amount: %w", err)} + } + holdings, err := m.store.GetHoldingsForPortfolio(m.ctx, portfolioID) + if err != nil { + return errMsg{err: fmt.Errorf("loading holdings after update: %w", err)} + } + return holdingsSavedMsg{holdings: holdings} + } +} + +// cmdDeleteHolding deletes a holding and reloads holdings, preserving list mode. +func (m PortfolioModel) cmdDeleteHolding(portfolioID, holdingID int64, listMode listing) tea.Cmd { + return func() tea.Msg { + if err := m.store.DeleteHolding(m.ctx, holdingID); err != nil { + return errMsg{err: fmt.Errorf("deleting holding: %w", err)} + } + holdings, err := m.store.GetHoldingsForPortfolio(m.ctx, portfolioID) + if err != nil { + return errMsg{err: fmt.Errorf("loading holdings after delete: %w", err)} + } + return holdingDeletedMsg{holdings: holdings} + } +} + +// renderEditAmountDialog renders the edit amount dialog. +func (m PortfolioModel) renderEditAmountDialog(mode editingAmount) string { + var b strings.Builder + _, _ = fmt.Fprintf(&b, "Edit %s (%s)\n\n", mode.holding.Name, mode.holding.Ticker) + _, _ = fmt.Fprintf(&b, "Current: %.4f\n", mode.holding.Amount) + b.WriteString("New amount: " + mode.input.View() + "\n") + if mode.errMsg != "" { + b.WriteString("\n" + lipgloss.NewStyle().Foreground(lipgloss.Color("#FF0000")).Render(mode.errMsg)) + } + + return lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + Padding(1, 2). + Render(b.String()) +} + +// renderDeleteDialog renders the delete confirmation dialog. +func (m PortfolioModel) renderDeleteDialog(mode deleting) string { + var b strings.Builder + _, _ = fmt.Fprintf(&b, "Delete Holding\n\n") + _, _ = fmt.Fprintf(&b, "Coin: %s (%s)\n", mode.holding.Name, mode.holding.Ticker) + _, _ = fmt.Fprintf(&b, "Amount: %.4f\n", mode.holding.Amount) + _, _ = fmt.Fprintf(&b, "Value: %s\n\n", format.FmtMoney(mode.holding.Value)) + b.WriteString("Press Enter to confirm deletion, or Esc to cancel.") + + return lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + Padding(1, 2). + Render(b.String()) +} diff --git a/internal/ui/portfolio_test.go b/internal/ui/portfolio_test.go index 8f70d1f..c3e2aee 100644 --- a/internal/ui/portfolio_test.go +++ b/internal/ui/portfolio_test.go @@ -1,9 +1,11 @@ package ui import ( + "fmt" "strings" "testing" + "github.com/charmbracelet/bubbles/textinput" tea "github.com/charmbracelet/bubbletea" "github.com/fredericomozzato/crypto_tracker/internal/store" ) @@ -682,6 +684,670 @@ func TestFilterCoinsNoMatch(t *testing.T) { } } +// Slice 8 tests - List mode, Edit, Delete + +func TestEnterFromBrowsingToListingMode(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin", Ticker: "BTC"}, + } + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + + // Should be in listing mode + if _, ok := updated.mode.(listing); !ok { + t.Errorf("expected listing mode, got %T", updated.mode) + } + if updated.holdingsCursor != 0 { + t.Errorf("expected holdingsCursor=0, got %d", updated.holdingsCursor) + } +} + +func TestEnterFromBrowsingNoHoldingsIsNoOp(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{} + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + + // Should still be in browsing mode + if _, ok := updated.mode.(browsing); !ok { + t.Errorf("expected browsing mode, got %T", updated.mode) + } +} + +func TestListingJkNavigation(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + {ID: 3, Name: "Litecoin"}, + } + m.mode = listing{} + m.holdingsCursor = 0 + + // j twice + m, _ = m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) + m, _ = m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) + if m.holdingsCursor != 2 { + t.Errorf("expected holdingsCursor=2 after two 'j' presses, got %d", m.holdingsCursor) + } + + // k once + m, _ = m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'k'}}) + if m.holdingsCursor != 1 { + t.Errorf("expected holdingsCursor=1 after 'k' press, got %d", m.holdingsCursor) + } +} + +func TestListingGJumpsToTop(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + {ID: 3, Name: "Litecoin"}, + } + m.mode = listing{} + m.holdingsCursor = 2 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'g'}}) + if updated.holdingsCursor != 0 { + t.Errorf("expected holdingsCursor=0 after 'g', got %d", updated.holdingsCursor) + } +} + +func TestListingGJumpsToBottom(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + {ID: 3, Name: "Litecoin"}, + } + m.mode = listing{} + m.holdingsCursor = 0 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'G'}}) + if updated.holdingsCursor != 2 { + t.Errorf("expected holdingsCursor=2 after 'G', got %d", updated.holdingsCursor) + } +} + +func TestListingClampsAtTop(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + } + m.mode = listing{} + m.holdingsCursor = 0 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'k'}}) + if updated.holdingsCursor != 0 { + t.Errorf("expected holdingsCursor to stay at 0, got %d", updated.holdingsCursor) + } +} + +func TestListingClampsAtBottom(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + } + m.mode = listing{} + m.holdingsCursor = 1 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) + if updated.holdingsCursor != 1 { + t.Errorf("expected holdingsCursor to stay at 1, got %d", updated.holdingsCursor) + } +} + +func TestListingEscReturnsToBrowsing(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.mode = listing{} + m.holdingsCursor = 0 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEsc}) + if _, ok := updated.mode.(browsing); !ok { + t.Errorf("expected browsing mode after Esc, got %T", updated.mode) + } +} + +func TestListingEnterOpensEditDialog(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin", Ticker: "BTC", Amount: 1.5}, + } + m.mode = listing{} + m.holdingsCursor = 0 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + if _, ok := updated.mode.(editingAmount); !ok { + t.Errorf("expected editingAmount mode, got %T", updated.mode) + } +} + +func TestListingXOpensDeleteDialog(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin", Ticker: "BTC", Amount: 1.5}, + } + m.mode = listing{} + m.holdingsCursor = 0 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'X'}}) + if _, ok := updated.mode.(deleting); !ok { + t.Errorf("expected deleting mode, got %T", updated.mode) + } +} + +func TestListingAOpensCoinPicker(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{ + portfolios: []store.Portfolio{{ID: 1, Name: "Test"}}, + }) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin", Ticker: "BTC", Amount: 1.5}, + } + m.mode = listing{} + + _, cmd := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}}) + if cmd == nil { + t.Error("expected non-nil cmd when pressing 'a' from list mode") + } +} + +func TestEditAmountEscReturnsToListing(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.mode = editingAmount{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + input: textinput.New(), + listMode: listing{}, + } + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEsc}) + if _, ok := updated.mode.(listing); !ok { + t.Errorf("expected listing mode after Esc, got %T", updated.mode) + } +} + +func TestEditAmountEnterWithEmptyNoOp(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.mode = editingAmount{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + input: textinput.New(), + listMode: listing{}, + } + + updated, cmd := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + if cmd != nil { + t.Error("expected nil cmd for empty input") + } + if _, ok := updated.mode.(editingAmount); !ok { + t.Errorf("expected to stay in editingAmount mode, got %T", updated.mode) + } +} + +func TestEditAmountEnterWithNonNumericShowsError(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + ti := textinput.New() + m.mode = editingAmount{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + input: ti, + listMode: listing{}, + } + + // Type "abc" + for _, r := range []rune{'a', 'b', 'c'} { + m, _ = m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) + } + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + + if !updated.InputActive() { + t.Error("expected to stay in editingAmount mode with invalid input") + } + if mode, ok := updated.mode.(editingAmount); ok { + if mode.errMsg == "" { + t.Error("expected error message for invalid amount") + } + } else { + t.Fatal("expected to be in editingAmount mode") + } +} + +func TestEditAmountEnterWithZeroOrNegativeShowsError(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + ti := textinput.New() + m.mode = editingAmount{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + input: ti, + listMode: listing{}, + } + + // Type "0" + m, _ = m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'0'}}) + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + + if mode, ok := updated.mode.(editingAmount); ok { + if mode.errMsg == "" { + t.Error("expected error message for zero amount") + } + } else { + t.Fatal("expected to stay in editingAmount mode") + } +} + +func TestEditAmountEnterWithValidAmountReturnsCmd(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{ + portfolios: []store.Portfolio{{ID: 1, Name: "Test"}}, + }) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + ti := textinput.New() + ti.Focus() // Need to focus the input for it to receive keystrokes + m.mode = editingAmount{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin", CoinID: 1}, + input: ti, + listMode: listing{}, + } + + // Type "1.5" + for _, r := range []rune{'1', '.', '5'} { + m, _ = m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) + } + _, cmd := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + if cmd == nil { + t.Error("expected non-nil cmd with valid amount") + } +} + +func TestEditingAmountInputActive(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.mode = editingAmount{} + if !m.InputActive() { + t.Error("expected InputActive() to be true for editingAmount mode") + } +} + +func TestDeleteConfirmEscReturnsToListing(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.mode = deleting{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + listMode: listing{}, + } + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyEsc}) + if _, ok := updated.mode.(listing); !ok { + t.Errorf("expected listing mode after Esc, got %T", updated.mode) + } +} + +func TestDeleteConfirmEnterReturnsCmd(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{ + portfolios: []store.Portfolio{{ID: 1, Name: "Test"}}, + }) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.mode = deleting{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + listMode: listing{}, + } + + _, cmd := m.update(tea.KeyMsg{Type: tea.KeyEnter}) + if cmd == nil { + t.Error("expected non-nil cmd when confirming delete") + } +} + +func TestDeletingInputActive(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.mode = deleting{} + if m.InputActive() { + t.Error("expected InputActive() to be false for deleting mode") + } +} + +func TestDeleteConfirmOtherKeysIgnored(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.mode = deleting{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin"}, + listMode: listing{}, + } + + // Press various keys + for _, key := range []rune{'j', 'k', 'a', 'x', '1'} { + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{key}}) + if _, ok := updated.mode.(deleting); !ok { + t.Errorf("expected to stay in deleting mode after pressing %c, got %T", key, updated.mode) + } + } +} + +func TestCursorClampedAfterDelete(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.mode = listing{} + m.holdingsCursor = 0 // at last item + + // Delete the last (and only) holding + updated, _ := m.update(holdingDeletedMsg{holdings: []store.HoldingRow{}}) + if updated.holdingsCursor != 0 { + t.Errorf("expected holdingsCursor=0 after delete, got %d", updated.holdingsCursor) + } +} + +func TestCursorStaysAtSamePositionAfterDelete(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + {ID: 3, Name: "Litecoin"}, + } + m.mode = listing{} + m.holdingsCursor = 1 // on Ethereum + + // Delete Ethereum + updated, _ := m.update(holdingDeletedMsg{holdings: []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 3, Name: "Litecoin"}, + }}) + if updated.holdingsCursor != 1 { + t.Errorf("expected holdingsCursor=1 after delete, got %d", updated.holdingsCursor) + } +} + +func TestHoldingDeletedMsgUpdatesHoldings(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + } + + updated, _ := m.update(holdingDeletedMsg{holdings: []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + }}) + if len(updated.holdings) != 1 { + t.Errorf("expected 1 holding after delete, got %d", len(updated.holdings)) + } +} + +func TestHoldingDeletedMsgClampsCursor(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + } + m.holdingsCursor = 1 + + updated, _ := m.update(holdingDeletedMsg{holdings: []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + }}) + if updated.holdingsCursor != 0 { + t.Errorf("expected holdingsCursor clamped to 0, got %d", updated.holdingsCursor) + } +} + +func TestHoldingDeletedMsgReturnsToListing(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + {ID: 2, Name: "Ethereum"}, + } + m.mode = deleting{} + + updated, _ := m.update(holdingDeletedMsg{holdings: []store.HoldingRow{ + {ID: 2, Name: "Ethereum"}, + }}) + if _, ok := updated.mode.(listing); !ok { + t.Errorf("expected listing mode after delete, got %T", updated.mode) + } +} + +func TestHoldingDeletedMsgToBrowsingWhenEmpty(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.mode = deleting{} + + updated, _ := m.update(holdingDeletedMsg{holdings: []store.HoldingRow{}}) + if _, ok := updated.mode.(browsing); !ok { + t.Errorf("expected browsing mode when holdings empty, got %T", updated.mode) + } +} + +func TestHoldingsSavedFromEditReturnsToListing(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.mode = editingAmount{} + + updated, _ := m.update(holdingsSavedMsg{holdings: []store.HoldingRow{ + {ID: 1, Name: "Bitcoin", Amount: 2.0}, + }}) + if _, ok := updated.mode.(listing); !ok { + t.Errorf("expected listing mode after edit save, got %T", updated.mode) + } +} + +func TestHoldingsSavedFromAddReturnsToBrowsing(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.holdings = []store.HoldingRow{} + m.mode = addAmount{} + + updated, _ := m.update(holdingsSavedMsg{holdings: []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + }}) + if _, ok := updated.mode.(browsing); !ok { + t.Errorf("expected browsing mode after add save, got %T", updated.mode) + } +} + +func TestBrowsingPgDnScrollsHoldingsPreview(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + // Create many holdings to allow scrolling + for i := 0; i < 50; i++ { + m.holdings = append(m.holdings, store.HoldingRow{ID: int64(i), Name: fmt.Sprintf("Coin%d", i)}) + } + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyPgDown}) + if updated.scrollOffset <= 0 { + t.Errorf("expected scrollOffset > 0 after PgDn, got %d", updated.scrollOffset) + } +} + +func TestBrowsingPgUpScrollsHoldingsPreview(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + // Create many holdings to allow scrolling + for i := 0; i < 50; i++ { + m.holdings = append(m.holdings, store.HoldingRow{ID: int64(i), Name: fmt.Sprintf("Coin%d", i)}) + } + m.scrollOffset = 20 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyPgUp}) + if updated.scrollOffset >= 20 { + t.Errorf("expected scrollOffset < 20 after PgUp, got %d", updated.scrollOffset) + } +} + +func TestBrowsingPgUpDoesNotGoBelowZero(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.scrollOffset = 0 + + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyPgUp}) + if updated.scrollOffset != 0 { + t.Errorf("expected scrollOffset to stay at 0, got %d", updated.scrollOffset) + } +} + +func TestBrowsingJkResetsScrollOffset(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{ + {ID: 1, Name: "Portfolio A"}, + {ID: 2, Name: "Portfolio B"}, + } + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.scrollOffset = 10 + + // j should reset scroll offset + updated, _ := m.update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}) + if updated.scrollOffset != 0 { + t.Errorf("expected scrollOffset reset to 0 after j, got %d", updated.scrollOffset) + } +} + +func TestListingModeShowsPanelFocus(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.mode = listing{} + + view := m.View() + if view == "" { + t.Error("expected non-empty view in listing mode") + } +} + +func TestBrowsingModeShowsPanelFocus(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.holdings = []store.HoldingRow{ + {ID: 1, Name: "Bitcoin"}, + } + m.mode = browsing{} + + view := m.View() + if view == "" { + t.Error("expected non-empty view in browsing mode") + } +} + +func TestEditDialogShowsCoinName(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + ti := textinput.New() + m.mode = editingAmount{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin", Ticker: "BTC", Amount: 1.5}, + input: ti, + } + + view := m.View() + if !strings.Contains(view, "Bitcoin") { + t.Error("expected edit dialog to show coin name") + } + if !strings.Contains(view, "BTC") { + t.Error("expected edit dialog to show ticker") + } +} + +func TestDeleteDialogShowsCoinName(t *testing.T) { + m := NewPortfolioModel(testCtx, &StubStore{}) + m.width = 100 + m.height = 30 + m.portfolios = []store.Portfolio{{ID: 1, Name: "Test"}} + m.mode = deleting{ + holding: store.HoldingRow{ID: 1, Name: "Bitcoin", Ticker: "BTC", Amount: 1.5, Value: 50000}, + } + + view := m.View() + if !strings.Contains(view, "Bitcoin") { + t.Error("expected delete dialog to show coin name") + } + if !strings.Contains(view, "BTC") { + t.Error("expected delete dialog to show ticker") + } + if !strings.Contains(view, "1.5000") && !strings.Contains(view, "1.5") { + t.Error("expected delete dialog to show amount") + } +} + func TestPortfolioViewShowsHoldingsTable(t *testing.T) { m := NewPortfolioModel(testCtx, &StubStore{}) m.width = 100 From e3cc5c2e4d719feb1a4df51ed73731d1b7cfbd7a Mon Sep 17 00:00:00 2001 From: Frederico Mozzato Date: Sun, 12 Apr 2026 14:16:08 -0300 Subject: [PATCH 2/4] =?UTF-8?q?fix(slice-008):=20resolve=20F1=20=E2=80=94?= =?UTF-8?q?=20Adding=20a=20holding=20from=20listing=20mode=20returns=20to?= =?UTF-8?q?=20browsing=20instead=20of=20listing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes finding F1 in docs/reviews/008-list-mode-edit-delete-holding/revision-1.md --- .../revision-1.md | 49 +++++++++++++++++++ internal/ui/portfolio.go | 33 +++++++++---- 2 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 docs/reviews/008-list-mode-edit-delete-holding/revision-1.md diff --git a/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md b/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md new file mode 100644 index 0000000..90a749d --- /dev/null +++ b/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md @@ -0,0 +1,49 @@ +--- +branch: feat/008-list-mode-edit-delete-holding +revision: 1 +status: in_progress +--- + +# Slice 008 — List Mode + Edit + Delete Holding (Revision 1) + +## Smoke test + completeness audit + +No findings. All scope items implemented, test coverage adequate, verification +commands satisfied. + +## Feature review + +| ID | Sev | Status | Summary | +|----|-----|--------|---------| +| F1 | MED | FIXED | Adding a holding from listing mode returns to browsing instead of listing | + +**F1** `internal/ui/portfolio.go:319`, `internal/ui/portfolio.go:958` + +When the user presses `a` in listing mode, `cmdOpenCoinPickerFromList(mode)` is +called. The function accepts a `listing` parameter but never uses it — the +comment says "preserving list mode for return" but the parameter is dead code. +The `coinPickerReadyMsg` handler overwrites `m.mode` to `addCoin{}` without +tracking the parent mode. After the add-holding flow completes, +`holdingsSavedMsg` transitions to `browsing{}` because `m.mode` is `addAmount`, +not `editingAmount`. Pressing Esc from `addCoin` also always returns to +`browsing{}`, losing the listing state. The fix is to add a parent-mode tracker +(e.g. an `origin portfolioMode` field) to `addCoin`/`addAmount` so they can +return to the correct mode. + +## Implementation review + +| ID | Sev | Status | Summary | +|----|-----|--------|---------| +| I1 | LOW | DISCARDED | Missing Ctrl+F/Ctrl+B key bindings for preview scrolling per PRD | +| I2 | LOW | OPEN | Listing mode status bar omits g/G and q shortcuts | + +**I1** `internal/ui/portfolio.go:171-178` + +DISCARDED - DO NOT FIX + +**I2** `internal/ui/portfolio.go:759` + +The listing mode status bar reads `j/k holdings • Enter edit • X delete • a add • Esc menu`. +The PRD specifies: `j/k holdings • g/G top/bottom • Enter edit • X delete • a add holding • Esc back to menu • q quit`. +The implementation omits `g/G top/bottom` and `q quit`. The `g` and `G` keys +are handled in listing mode but not advertised in the status bar. diff --git a/internal/ui/portfolio.go b/internal/ui/portfolio.go index 28f3a28..bcc739b 100644 --- a/internal/ui/portfolio.go +++ b/internal/ui/portfolio.go @@ -24,12 +24,14 @@ type ( allCoins []store.Coin // already filtered — held coins removed filtered []store.Coin // subset matching current filter query cursor int + origin portfolioMode // track parent mode (browsing or listing) for return } addAmount struct { coin store.Coin input textinput.Model errMsg string - coinMode addCoin // preserved so Esc returns to coin picker with state intact + coinMode addCoin // preserved so Esc returns to coin picker with state intact + origin portfolioMode // track parent mode (browsing or listing) for return } listing struct{} // list mode: right panel focus, j/k navigate holdings editingAmount struct { @@ -61,7 +63,8 @@ type portfoliosLoadedMsg struct { // coinPickerReadyMsg is sent when all coins are loaded for the picker. type coinPickerReadyMsg struct { - coins []store.Coin + coins []store.Coin + origin portfolioMode // track parent mode (browsing or listing) for return } // holdingsLoadedMsg is sent when holdings are loaded for the current portfolio. @@ -200,7 +203,7 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { case addCoin: switch msg.Type { case tea.KeyEsc: - m.mode = browsing{} + m.mode = mode.origin return m, nil case tea.KeyEnter: if len(mode.filtered) > 0 { @@ -439,7 +442,7 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { m.lastErr = "all coins already in portfolio" return m, nil } - // Enter addCoin mode + // Enter addCoin mode with origin preserved m.lastErr = "" ti := textinput.New() ti.Placeholder = "filter coins..." @@ -450,6 +453,7 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { allCoins: available, filtered: available, cursor: 0, + origin: msg.origin, } return m, nil @@ -459,11 +463,21 @@ func (m PortfolioModel) update(msg tea.Msg) (PortfolioModel, tea.Cmd) { case holdingsSavedMsg: m.holdings = msg.holdings - // If we were in editingAmount mode, return to listing instead of browsing - if _, wasEditing := m.mode.(editingAmount); wasEditing { + // Determine return mode based on the mode we were in + switch prevMode := m.mode.(type) { + case editingAmount: + // Editing always returns to listing m.mode = listing{} m.clampHoldingsCursor() - } else { + case addAmount: + // Adding returns to listing if we came from listing mode, else browsing + if _, fromListing := prevMode.origin.(listing); fromListing { + m.mode = listing{} + m.clampHoldingsCursor() + } else { + m.mode = browsing{} + } + default: m.mode = browsing{} } return m, nil @@ -823,7 +837,7 @@ func (m PortfolioModel) cmdOpenCoinPicker() tea.Cmd { if err != nil { return errMsg{err: fmt.Errorf("loading coins for picker: %w", err)} } - return coinPickerReadyMsg{coins: coins} + return coinPickerReadyMsg{coins: coins, origin: browsing{}} } } @@ -866,6 +880,7 @@ func (m PortfolioModel) transitionToAddAmount(mode addCoin) (PortfolioModel, tea coin: selectedCoin, input: ti, coinMode: mode, + origin: mode.origin, } return m, nil } @@ -961,7 +976,7 @@ func (m PortfolioModel) cmdOpenCoinPickerFromList(listMode listing) tea.Cmd { if err != nil { return errMsg{err: fmt.Errorf("loading coins for picker: %w", err)} } - return coinPickerReadyMsg{coins: coins} + return coinPickerReadyMsg{coins: coins, origin: listMode} } } From 33333fbf753f4131d18a1ac44da0f8a56245b089 Mon Sep 17 00:00:00 2001 From: Frederico Mozzato Date: Sun, 12 Apr 2026 14:16:48 -0300 Subject: [PATCH 3/4] =?UTF-8?q?fix(slice-008):=20resolve=20I2=20=E2=80=94?= =?UTF-8?q?=20Listing=20mode=20status=20bar=20omits=20g/G=20and=20q=20shor?= =?UTF-8?q?tcuts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes finding I2 in docs/reviews/008-list-mode-edit-delete-holding/revision-1.md Revision status: done --- docs/reviews/008-list-mode-edit-delete-holding/revision-1.md | 4 ++-- internal/ui/portfolio.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md b/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md index 90a749d..05209b9 100644 --- a/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md +++ b/docs/reviews/008-list-mode-edit-delete-holding/revision-1.md @@ -1,7 +1,7 @@ --- branch: feat/008-list-mode-edit-delete-holding revision: 1 -status: in_progress +status: done --- # Slice 008 — List Mode + Edit + Delete Holding (Revision 1) @@ -35,7 +35,7 @@ return to the correct mode. | ID | Sev | Status | Summary | |----|-----|--------|---------| | I1 | LOW | DISCARDED | Missing Ctrl+F/Ctrl+B key bindings for preview scrolling per PRD | -| I2 | LOW | OPEN | Listing mode status bar omits g/G and q shortcuts | +| I2 | LOW | FIXED | Listing mode status bar omits g/G and q shortcuts | **I1** `internal/ui/portfolio.go:171-178` diff --git a/internal/ui/portfolio.go b/internal/ui/portfolio.go index bcc739b..0afabda 100644 --- a/internal/ui/portfolio.go +++ b/internal/ui/portfolio.go @@ -770,7 +770,7 @@ func (m PortfolioModel) renderStatusBar() string { case addAmount: content = "Enter to confirm • Esc back to coin selection" case listing: - content = "j/k holdings • Enter edit • X delete • a add • Esc menu" + content = "j/k holdings • g/G top/bottom • Enter edit • X delete • a add holding • Esc back to menu • q quit" case editingAmount: content = "Enter to save • Esc cancel" case deleting: From b921e916d0c4c99e242a3b3c0648b2ba60625e18 Mon Sep 17 00:00:00 2001 From: Frederico Mozzato Date: Sun, 12 Apr 2026 14:21:40 -0300 Subject: [PATCH 4/4] docs: mark slice 8 as done --- docs/roadmap.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index 64cafe1..a2690b4 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -71,7 +71,7 @@ STATUS: DONE - **TDD:** holding upsert (including update-on-conflict), filter logic, computed values ## Slice 8 — List mode + edit + delete holding -STATUS: IN_REVIEW +STATUS: DONE - `Enter` from menu mode enters list mode (right panel focus) - Add focus hint in the border changing the color of the selected panel's border