feat: add keyboard shortcuts for navigation (go back and go forward)#24
feat: add keyboard shortcuts for navigation (go back and go forward)#24qwdavid wants to merge 2 commits intoniklabh:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughKeyboard shortcut handling in the UI module now detects Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
oxide-browser/src/ui.rs (1)
923-923: Renamego_fowardtogo_forwardfor clarityThe typo is harmless functionally, but it hurts readability and makes future maintenance error-prone.
Suggested fix
- go_foward, + go_forward, @@ - if go_foward { + if go_forward { self.tabs[self.active_tab].go_forward(); }Also applies to: 965-965
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@oxide-browser/src/ui.rs` at line 923, Rename the misspelled identifier go_foward to go_forward everywhere it appears (declaration, struct field, method, and all call sites) to restore clarity; update the occurrences referenced in this diff (the items at/around the go_foward entries and the other occurrence noted) and any tests, docs, or pub API that reference it, making sure to adjust pattern matches, struct initializers, and method definitions to the new name so the crate compiles after the rename.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@oxide-browser/src/ui.rs`:
- Around line 915-935: The Alt+Arrow navigation booleans (go_back and go_foward)
are being set inside the ctx.input closure and will fire even when a text field
is focused; change the closure so those two values only become true when no
text-editable widget is focused (e.g., check that the input focus is not on an
editable text field via the egui input focus API) before allowing
i.modifiers.alt && i.key_pressed(egui::Key::ArrowLeft/ArrowRight) to set
go_back/go_foward; update the ctx.input closure where new_tab, close_tab,
next_tab, prev_tab, toggle_bookmark, toggle_panel, go_back, go_foward are
computed to add this extra !focused_text_edit condition when assigning go_back
and go_foward.
---
Nitpick comments:
In `@oxide-browser/src/ui.rs`:
- Line 923: Rename the misspelled identifier go_foward to go_forward everywhere
it appears (declaration, struct field, method, and all call sites) to restore
clarity; update the occurrences referenced in this diff (the items at/around the
go_foward entries and the other occurrence noted) and any tests, docs, or pub
API that reference it, making sure to adjust pattern matches, struct
initializers, and method definitions to the new name so the crate compiles after
the rename.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Partially implements #21
These shortcuts call the existing
go_backandgo_forwardmethods on the active tab.Summary by CodeRabbit
New Features
Improvements