fix(navbar): prevent Escape key from resetting search when not active#39463
fix(navbar): prevent Escape key from resetting search when not active#39463gauravsingh001-cyber wants to merge 2 commits intoRocketChat:developfrom
Conversation
…rch is not active
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Escape key handler in NavBarSearch now checks whether the currently focused element is the search trigger (or the list is open) before processing Escape, avoiding global Escape from resetting the navbar search when the input is not active. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx">
<violation number="1" location="apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx:66">
P2: Escape no longer closes search when focus moves into the open listbox (options), because the global handler now ignores cases where the input is not active and the listbox handler doesn’t handle Escape.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx`:
- Around line 64-69: The Escape handler currently only triggers when the input
element equals triggerRef.current, but per PR it should also close the dropdown
when state.isOpen is true; update the condition inside the 'Escape' case to
allow the handler to run when either document.activeElement ===
triggerRef.current OR state.isOpen is true, keep calling event.preventDefault()
and handleEscSearch(), and then add state (or state.isOpen) to the useEffect
dependency array so the effect updates correctly; reference triggerRef,
handleEscSearch, and state.isOpen when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46a8d6e7-adb8-446a-a3e8-45dd7b855810
📒 Files selected for processing (1)
apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx
FIx #39462
Description
The Escape key listener in NavBarSearch.tsx was attached globally using tinykeys(window). Because of this, pressing Escape anywhere in the application triggered handleEscSearch(), even when the search input was not focused or the search dropdown was not open.
This PR adds a guard to ensure that the Escape handler only runs when the search input is active.
Steps to reproduce
Start the Rocket.Chat development server
Open the application
Do not open the Navbar search
Focus the chat input
Press Escape
Actual behavior
Escape triggers handleEscSearch() even when the search is not active.
Expected behavior
Escape should reset the search only when the search input is focused or the search dropdown is open.
solution.10.mp4
Summary by CodeRabbit