Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @dhth, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request refines the navigation and keybinding behavior within the TUI, primarily focusing on the escape and q keys. It introduces more context-aware handling for these keys, ensuring that users can intuitively navigate back to previous views or quit the application, depending on the current interface state. Additionally, it includes minor code cleanup by removing unused constants.
Highlights
- Improved Escape Key Handling: The
escapekey's behavior is now context-dependent, specifically callinghandleEscapeInForms()only when in form-related views, preventing unintended exits from other views. - Refined 'Go Back' Logic: The
helpViewnow returns to thelastViewinstead of alwaystaskListView, enhancing user flow. ThetaskLogViewnow correctly returns to itself when 'q' or 'escape' is pressed. - Clearer Quit vs. Go Back: The
ctrl+ckey is now explicitly designated for immediate quitting, whileqandescapeare used for navigating back or gracefully exiting, aligning with updated help text. - Code Cleanup: Unused time format constants (
dayFormat,friendlyTimeFormat) have been removed frominternal/ui/model.go, contributing to cleaner code.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request significantly improves the user experience of the TUI by refining traversal and key handling. The changes make navigation more intuitive and consistent, particularly with the escape and q keys. Separating the immediate quit action to ctrl+c is a good clarification of user intent. The refactoring also fixes a potential bug where the escape key could be handled twice. I have one high-severity suggestion to improve maintainability by consolidating key-press handling logic.
There was a problem hiding this comment.
Pull Request Overview
This PR improves the keyboard navigation and user experience in the TUI by adding proper escape key handling and refining the quit behavior. The changes separate immediate quit (Ctrl+C) from navigational back actions (q/escape), and improve view traversal logic.
Key changes:
- Added escape key constant and handling to provide consistent navigation between views
- Separated immediate quit (Ctrl+C) from back navigation (q/escape) for better UX
- Improved view traversal by remembering the last view when returning from help
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/ui/update.go | Added escape key handling, restructured quit logic, and improved view navigation flow |
| internal/ui/model.go | Removed unused time format constants |
| internal/ui/help.go | Updated help text to reflect new keyboard shortcuts (escape for back, Ctrl+C for quit) |
| internal/ui/handle.go | Renamed escape handler function and improved back navigation to return to last view from help |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRefactors Escape-key handling and back/quit routing across UI update logic; adds a constant for Escape; introduces early window-resize handling; adjusts view transitions for back action; updates help text to reflect new shortcuts; and removes two time-format constants. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI Model
participant View Router
participant Form Handler
participant Window Resizer
User->>UI Model: Key/Window events
alt WindowSizeMsg
UI Model->>Window Resizer: handleWindowResizing
Window Resizer-->>UI Model: updated dims/commands
else Escape key
alt In form views
UI Model->>Form Handler: handleEscapeInForms
Form Handler-->>UI Model: commands
else Other views
UI Model->>View Router: handleRequestToGoBackOrQuit (via q/esc)
View Router-->>UI Model: GoBack/Quit decision
end
else q key
UI Model->>View Router: handleRequestToGoBackOrQuit
View Router-->>UI Model: GoBack/Quit decision
else Ctrl+C
UI Model-->>User: Quit
else Enter
alt entryComment focused
UI Model-->>User: No-op
else Create/Update action
UI Model-->>User: Execute command
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
internal/ui/handle.go (1)
199-201: Bug: clearing the wrong input slice in editActiveTLView.You’re clearing taskInputs instead of TL inputs when handling Escape for editActiveTLView. This wipes the task summary field instead of the begin timestamp.
Apply:
- m.taskInputs[entryBeginTS].SetValue("") + m.tLInputs[entryBeginTS].SetValue("")internal/ui/update.go (1)
167-173: ‘q’ does not go back in forms (help says q/ go back).Currently, Escape in forms routes to handleEscapeInForms(), but ‘q’ falls through to handleRequestToGoBackOrQuit(), which doesn’t handle form views—so ‘q’ is a no-op in forms. This contradicts the help text and will confuse users.
Apply this to align ‘q’ with Escape in forms:
- case "q", escape: - shouldQuit := m.handleRequestToGoBackOrQuit() + case "q", escape: + // Treat 'q' like Escape in form views + switch m.activeView { + case taskInputView, editActiveTLView, finishActiveTLView, manualTasklogEntryView, editSavedTLView: + m.handleEscapeInForms() + return m, tea.Batch(cmds...) + } + shouldQuit := m.handleRequestToGoBackOrQuit() if shouldQuit { return m, tea.Quit }
♻️ Duplicate comments (2)
internal/ui/update.go (2)
80-85: Escape focuses only forms here; consider a safety net.This block handles Escape in form views only. If new form views are added, they’ll need to be added here as well.
Document this requirement or add a default branch to handle future form views defensively.
49-144: Two separate key-handling switch blocks reduce maintainability.Key handling happens in two disjoint switches (Lines 49–144 and again at 164–279). Centralizing into one would reduce drift and double-handling risk.
Also applies to: 164-279
🧹 Nitpick comments (1)
internal/ui/handle.go (1)
205-211: Move comment clearing out of the loop.m.tLCommentInput.SetValue("") is called on each iteration unnecessarily.
Apply:
- for i := range m.tLInputs { - m.tLInputs[i].SetValue("") - m.tLCommentInput.SetValue("") - } + for i := range m.tLInputs { + m.tLInputs[i].SetValue("") + } + m.tLCommentInput.SetValue("")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
internal/ui/handle.go(2 hunks)internal/ui/help.go(1 hunks)internal/ui/model.go(1 hunks)internal/ui/update.go(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/ui/handle.go (1)
internal/ui/model.go (1)
Model(81-115)
🔇 Additional comments (6)
internal/ui/help.go (1)
43-45: Help shortcuts update looks correct.Documenting q/ for “Go back” and adding “<ctrl+c> Quit immediately” aligns with the updated input handling and improves clarity for users.
internal/ui/handle.go (1)
331-335: Back navigation target changed; confirm intent.inactiveTaskListView now goes back to taskLogView (was taskListView), and helpView goes back to lastView. If this is meant to create a consistent traversal cycle, all good—just confirm it matches expected UX flows.
internal/ui/update.go (4)
15-18: Introducing an escape key constant is a good cleanup.Centralizing "esc" as a constant reduces stringly-typed errors and improves readability.
26-37: Early window-resize handling is solid.Handling tea.WindowSizeMsg upfront and normalizing insufficientDimensionsView behavior improves clarity and consistency of returns.
50-59: Enter bail-out for comment textarea is a nice UX fix.Avoiding accidental submit while entryComment is focused mirrors typical form behavior.
375-378: recordsModel: Escape included for quitting — good.Adding Escape alongside ctrl+C and q for quit in records view keeps behavior consistent across UI.
Summary by CodeRabbit
New Features
Documentation