feat: add keyboard shortcut to open Cookies modal.#7801
feat: add keyboard shortcut to open Cookies modal.#7801KanhaiyaPandey wants to merge 2 commits intousebruno:mainfrom
Conversation
KanhaiyaPandey
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughKeyboard shortcut support for opening the Cookies modal was added. Cookies modal visibility was moved from local React state to Redux ( Changes
Sequence DiagramsequenceDiagram
actor User
participant Hotkeys as Hotkeys Provider
participant Redux as Redux Store
participant StatusBar as StatusBar Component
User->>Hotkeys: Press shortcut (Shift+Alt+C / Cmd+Shift+C)
Hotkeys->>Redux: dispatch(toggleCookiesModal())
Redux->>Redux: toggle isCookiesModalOpen
Redux->>StatusBar: state update (isCookiesModalOpen)
StatusBar->>User: render show/hide Cookies modal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/StatusBar/index.js (1)
61-74:⚠️ Potential issue | 🟡 MinorMinor: unguarded
querySelector(...).focus()can throw.If the
[data-trigger="cookies"]button isn't in the DOM when the modal closes (e.g., StatusBar unmounted, or layout change hides it),document.querySelector(...)returnsnulland.focus()will throw aTypeError. A tiny guard keeps close robust:🛡️ Proposed guard
onClose={() => { dispatch(closeCookiesModal()); - document.querySelector('[data-trigger="cookies"]').focus(); + document.querySelector('[data-trigger="cookies"]')?.focus(); }}Also note: when the modal is closed via the
Shift+Alt+Chotkey (which dispatchestoggleCookiesModaldirectly), thisonClosedoesn't fire, so focus isn't restored in that path. Not blocking, just a small UX inconsistency worth being aware of.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/StatusBar/index.js` around lines 61 - 74, The onClose handler in the StatusBar's Cookies modal calls document.querySelector('[data-trigger="cookies"]').focus() without a null check, which can throw if the trigger isn't present; update the onClose in the Cookies component (the anonymous function that dispatches closeCookiesModal()) to first query the element into a variable and only call .focus() if that element exists (e.g., guard with if (el) or optional chaining), leaving the dispatch(closeCookiesModal()) behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/providers/Hotkeys/keyMappings.js`:
- Around line 58-63: The current binding for openCookies in keyMappings.js uses
'shift+bind+alt+bind+c' which maps to shift+option+c on macOS and fails with
Mousetrap 1.6.5 because the browser reports a dead key (ç/Ç); update the
openCookies binding to use a mac-safe shortcut or add platform-specific entries:
change the mac binding to a different modifier+letter that doesn't produce a
dead key (e.g., avoid 'option+c') or add a conditional entry for openCookies
with a mac-safe key, or upgrade/replace Mousetrap with a library that correctly
normalizes modifier+letter combos—target the openCookies binding in the bindings
object to apply the fix.
---
Outside diff comments:
In `@packages/bruno-app/src/components/StatusBar/index.js`:
- Around line 61-74: The onClose handler in the StatusBar's Cookies modal calls
document.querySelector('[data-trigger="cookies"]').focus() without a null check,
which can throw if the trigger isn't present; update the onClose in the Cookies
component (the anonymous function that dispatches closeCookiesModal()) to first
query the element into a variable and only call .focus() if that element exists
(e.g., guard with if (el) or optional chaining), leaving the
dispatch(closeCookiesModal()) behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1a9bf18-a587-42b5-bfc4-2522e45eace9
📒 Files selected for processing (4)
packages/bruno-app/src/components/StatusBar/index.jspackages/bruno-app/src/providers/Hotkeys/index.jspackages/bruno-app/src/providers/Hotkeys/keyMappings.jspackages/bruno-app/src/providers/ReduxStore/slices/app.js
|
Can it be reviewed? If anything feels like it needs to be improved, I can improve, because without a shortcut, I feel too irritated to use the mouse to go over cookies. |
2 participants
📝 PR Description
Added a keyboard shortcut to quickly open and close the Cookies modal, improving developer workflow and accessibility.
✨ Changes
Shift + Alt + CShift + Option + CPreferences → Keybindings → Cookies → "Open Cookies"📸 Screenshots
🚀 Impact
🔗 Closes
Closes #7799
Summary by CodeRabbit
New Features
Improvements