Add grid-based navigation with wrapping and visual feedback#62
Conversation
Prompt: Currently, we have Cmd+arrows buttons to navigate the grid and Cmd+Shift+[ or ] to navigate left/right between the terminals in full screen mode. I'd like to change that: 1. Cmd+arrows in the grid screen mode should wrap around the grid 2. Cmd+arrows in the full screen mode should navigate between the terminals according to their grid position: e.g., if I'm at terminal (0, 0) and I hit Cmd+right, there should be a panning animation from left to right to terminal (0, 1), as it now does on Cmd+Shift+]; but if I hit Cmd+down, the animation should pan down to the terminal (1, 0). It should also wrap around the grid. Solution: Implemented a unified grid navigation system with the following changes: 1. Added vertical panning animations (.PanningUp, .PanningDown) to ViewMode enum to support up/down navigation in full screen mode. 2. Implemented vertical panning rendering in renderer.zig that animates terminals along the Y-axis, mirroring the existing horizontal panning behavior. 3. Created navigateGrid() function that handles navigation with wrapping in both Grid and Full modes: - In Grid mode: Wraps around edges without animations (just changes focus) - In Full mode: Wraps with directional panning animations - Wrapping animations show opposite direction for visual continuity (e.g., wrapping right shows PanningRight animation to give feedback of going "around") 4. Removed legacy Cmd+Shift+[ ] shortcuts and isSwitchTerminalShortcut() function, replacing them with the new grid-based navigation. 5. Enhanced toast notifications with visual grid feedback: - Shows a 3×3 grid with filled square (■) for current terminal - Empty squares (□) for other terminals - Multi-line rendering support added to toast component - Proper horizontal and vertical spacing between grid cells 6. Grid mode navigation no longer spawns terminals on focus change, maintaining the original lightweight behavior. The new system provides intuitive, carousel-like navigation where the grid feels continuous, with wrapping at edges and clear visual feedback of position through both animations and toast notifications.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive grid-based navigation with wrapping support and visual feedback for both Grid and Full screen modes. It unifies terminal navigation by removing legacy keyboard shortcuts in favor of arrow-key-based grid navigation.
Key Changes:
- Implemented wrapping navigation in both Grid and Full modes using Cmd+arrow keys
- Added vertical panning animations (PanningUp/PanningDown) to complement horizontal panning
- Created multi-line toast rendering to display a 3×3 grid showing current terminal position
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/components/toast.zig | Implements multi-line toast rendering by splitting messages on newlines and compositing surfaces |
| src/render/renderer.zig | Adds vertical panning animation modes (.PanningUp and .PanningDown) for up/down navigation |
| src/main.zig | Refactors navigation logic into unified navigateGrid() function, removes legacy shortcuts, adds grid notification formatting |
| src/input/mapper.zig | Removes deprecated isSwitchTerminalShortcut() function |
| src/c.zig | Exposes additional SDL surface manipulation functions for composite rendering |
| src/app/app_state.zig | Adds PanningUp and PanningDown view modes to support vertical animations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (enable_wrapping) { | ||
| new_row = GRID_ROWS - 1; | ||
| is_wrapping = true; | ||
| } |
There was a problem hiding this comment.
The animation direction is inverted for upward navigation. When wrapping from row 0 to the bottom row, the visual should pan upward (showing content coming from below), so it should use .PanningDown. When moving up normally, it should use .PanningUp. The conditional should be: if (is_wrapping) .PanningDown else .PanningUp
| } | |
| animation_mode = if (is_wrapping) .PanningDown else .PanningUp; |
| } else if (enable_wrapping) { | ||
| new_row = 0; | ||
| is_wrapping = true; | ||
| } |
There was a problem hiding this comment.
The animation direction is inverted for downward navigation. When wrapping from the bottom row to row 0, the visual should pan downward (showing content coming from above), so it should use .PanningUp. When moving down normally, it should use .PanningDown. The conditional should be: if (is_wrapping) .PanningUp else .PanningDown
| } | |
| animation_mode = if (is_wrapping) .PanningUp else .PanningDown; |
| defer c.SDL_DestroySurface(surface); | ||
|
|
||
| const texture = c.SDL_CreateTextureFromSurface(renderer, surface) orelse return error.TextureFailed; | ||
| var lines: [16][]const u8 = undefined; |
There was a problem hiding this comment.
The magic number 16 for maximum lines is hardcoded in multiple places (lines 161, 179, 180). Consider extracting this as a named constant at the file or function level to improve maintainability and make it easier to adjust if needed.
| } | ||
|
|
||
| for (lines[0..line_count], 0..) |line, idx| { | ||
| var line_buf: [256]u8 = undefined; |
There was a problem hiding this comment.
The magic number 256 for the line buffer size is undocumented. Consider adding a comment explaining this size limit or extracting it as a named constant to clarify the maximum line length supported.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 650ec5179b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (anim_state.mode == .Grid) { | ||
| try navigateGrid(&anim_state, &sessions, direction, now, true, false); | ||
| const new_session = anim_state.focused_session; | ||
| sessions[new_session].dirty = true; |
There was a problem hiding this comment.
Invalidate old grid cell cache on focus move
Grid navigation only marks the new session dirty, so the previously focused session’s cached texture is reused even though renderSessionContent draws the cursor only when is_focused is true. Because renderGridSessionCached re-renders content only when session.dirty, moving focus with Cmd+arrows in Grid mode can leave the old cell showing a stale cursor/highlight until it outputs again. Consider marking the previous focused session dirty (or otherwise invalidating its cache) when focus changes.
Useful? React with 👍 / 👎.
Replaced hardcoded values with named constants for better maintainability: - MAX_TOAST_LINES (16): Maximum number of lines in multi-line toast - MAX_LINE_LENGTH (256): Maximum character length per line This addresses code review feedback about magic numbers.
Addressing Review CommentsThank you for the review! I've addressed the feedback: ✅ Fixed: Magic numbers (comments #3 and #4)
❌ Not changing: Animation directions (comments #1 and #2) The current animation directions are correct for the intended UX. The user explicitly requested a "carousel" effect where wrapping animations show the direction of movement:
This means when wrapping, the animation should pan in the same direction as the key press to give visual feedback of going "around" the grid. Current behavior (which is correct):
This matches the horizontal wrapping behavior that was tested and approved:
The suggested changes would invert this and make wrapping feel like normal navigation instead of giving the carousel feedback the user requested. |
Summary
This PR implements enhanced grid-based navigation with wrapping support and visual feedback for both Grid and Full screen modes.
Changes
Navigation Enhancements:
Visual Feedback:
Technical Implementation:
.PanningUpand.PanningDownanimation modesnavigateGrid()function for both modesTest Plan
zig build)Screenshots/Demo
The grid notification now displays as:
When at terminal (0,1), with proper spacing and multi-line layout.