Skip to content

feat: improve tui shortcuts#48

Merged
Pertempto merged 11 commits intomainfrom
feat/improve-tui-shortcuts
Jan 16, 2026
Merged

feat: improve tui shortcuts#48
Pertempto merged 11 commits intomainfrom
feat/improve-tui-shortcuts

Conversation

@Pertempto
Copy link
Contributor

No description provided.

- Create form.go with shared form helpers
- Add renderFormContent, handleFormSubmit, parseFormTime
- Add openNewMode, openEditMode, openResumeMode functions
- Create NewMode, EditMode, ResumeMode using shared helpers
- Add FormState to Model for tracking form operation type
- Add UpdateEntry to TaskManager for editing existing entries
- n: opens new entry form with empty fields
- r: opens resume form with project/task pre-filled (disabled on blanks)
- e: opens edit form with all fields pre-filled from selected entry
- s: now only stops running entries (no longer opens forms)
- Create ConfirmMode with entry details display
- Add DeleteEntry to TaskManager (converts entry to blank)
- d shortcut shows confirmation, y confirms, n/Esc cancels
- Disabled on blank entries
- Update ListMode KeyBindings to show n, s, r, e, d shortcuts
- Add tests for resume, edit, delete, and stop shortcuts
- Verify shortcuts work correctly on blank vs non-blank entries
- Test n shortcut opens new mode
- Test r shortcut opens resume mode (and disabled on blanks)
- Test e shortcut opens edit mode
- Test d shortcut opens confirm mode
- Test s shortcut stops running entries (no-op on non-running)
- Test form submission creates new entries
- Test status bar shows all shortcuts
- Test keyboard navigation (j, k, G)
@github-actions
Copy link

In cmd/tui/modes/list.go -> renderTableRows the empty-state message still says:

"No time entries found. Press 's' to start tracking."

But the new shortcut changes make 's' only stop running entries and no longer start new ones. This will confuse users. Please either:

  • Change the message to reference the new starter key (e.g. "Press 'n' to start tracking."), or
  • Restore a behavior where s starts tracking when there are no entries.

Suggested quick fix: replace 's' with 'n' in that message so UX matches the shortcuts.

@github-actions
Copy link

Changes Requested

  • Fix s (stop) behavior so it operates on the selected entry (or explicitly only acts on the most-recent entry) to avoid stopping the wrong record.
  • Update the empty-state message that still references s as the start key (change to n or restore s start behavior).
  • Add/adjust a unit test that ensures stopping the selected (non-last) running entry does not stop a different entry (prevent regression).

Summary of Changes

  • Added new TUI modes: new, edit, resume, and confirm with shared form helpers (cmd/tui/modes/form.go, cmd/tui/modes/confirm.go).
  • Updated ListMode keybindings and handlers to include n, r, e, d and refactored s to only stop running entries (cmd/tui/modes/list.go).
  • Wired new modes into the main model (cmd/tui/model.go, cmd/tui/modes/types.go).
  • Added TaskManager methods UpdateEntry and DeleteEntry (utils/task_manager.go).
  • Added E2E and unit tests for the new shortcuts and forms (cmd/headless/e2e_test.go, cmd/tui/model_test.go).
  • Marked the spec as complete (specs/003-improve-tui-shortcuts.md).

Requested Changes (details / rationale)

  • Correct the s key behavior in cmd/tui/modes/list.go:

    • Problem: m.TaskManager.StopEntry() always stops the last entry in storage. If the user has selected a different running entry, pressing s will stop the wrong entry.
    • Fix options:
      • Preferable: add/implement TaskManager.StopEntryAt(idx int) (or StopEntryByIndex) and call it with the currently-selected index when appropriate.
      • Alternative: enforce s only acts when m.SelectedIdx == len(m.Entries)-1 and show a user-facing status message otherwise (e.g. "Stop only available on most recent entry").
    • Also add a test that creates two running entries (or a running earlier entry) and asserts that pressing s while a non-last running entry is selected does not stop the unrelated last entry.
  • Update the empty-state hint in cmd/tui/modes/list.go -> renderTableRows:

    • Current message: "No time entries found. Press 's' to start tracking."
    • Because s no longer starts entries, update to reference n (e.g. "Press 'n' to start tracking.") or restore prior s start behavior.
  • Add a unit test to cover delete behavior (optional but recommended):

    • Ensure DeleteEntry converts an entry to a blank/gap and preserves chronological ordering/Start/End expectations. Right now DeleteEntry clears project/title only; confirm this is the intended semantics and add a test documenting it.

Small suggestions / nitnotes

  • The new UpdateEntry mutates Start but doesn't touch End — confirm whether editing should preserve or reset End.
  • In cmd/headless/e2e_test.go many tests depend on time.Now(); consider using deterministic fixed timestamps where possible to avoid flakiness.

Overall Feedback

  • Negative: There is a correctness risk around the s (stop) action: calling StopEntry() without targeting the selected entry can silently stop the wrong record. This is a critical behavioral bug that should be fixed before merging.

  • Positive: The refactor to a unified form system and the new new/edit/resume/confirm modes is well-structured. Tests and spec updates show good coverage and thoughtfulness — nice work cleaning up keybindings and centralizing form logic. 🎉

If you want, I can propose a patch that adds StopEntryAt(idx int) to TaskManager and updates the list handler + tests. @Pertempto

@Pertempto Pertempto marked this pull request as ready for review January 16, 2026 17:57
@Pertempto Pertempto merged commit 4841100 into main Jan 16, 2026
1 check passed
@Pertempto Pertempto deleted the feat/improve-tui-shortcuts branch January 16, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants