fix: close members dropdown when clicking outside in Create Channel m…#39633
fix: close members dropdown when clicking outside in Create Channel m…#39633Pavani12314 wants to merge 5 commits intoRocketChat:developfrom
Conversation
|
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 |
🦋 Changeset detectedLatest commit: d2116cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
WalkthroughAdds outside-click detection to UserAutoCompleteMultiple so the component clears its filter and closes the members dropdown when clicking outside. Also adds two changeset files to record a patch release for the fix. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 file
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
There was a problem hiding this comment.
Pull request overview
Fixes the Create Channel modal “Members” autocomplete dropdown not closing when clicking outside by adding an outside-click handler to UserAutoCompleteMultiple.
Changes:
- Added a ref-wrapped container around
MultiSelectFilteredto detect clicks outside the component. - Added an effect that listens for
mousedownevents and clears the filter when the click occurs outside the component.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Outdated
Show resolved
Hide resolved
![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
🤖 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/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx`:
- Around line 91-101: The outside-click handler using containerRef and
document.addEventListener incorrectly closes the filter because
MultiSelectFiltered renders its dropdown in a Portal; replace this manual logic
with the useOutsideClick hook from `@rocket.chat/fuselage-hooks`: remove the
useEffect, keep containerRef and setFilter, call useOutsideClick(containerRef,
() => setFilter('')) (or equivalent signature used in the codebase) so
portal-rendered clicks are handled correctly; ensure imports include
useOutsideClick and confirm the same behavior as
NavBarSearchListbox/EmojiPicker/DropDownListWrapper.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 652308a1-38f9-4582-91eb-c715ce541ad6
📒 Files selected for processing (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.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). (2)
- GitHub Check: Agent
- 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/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
🧠 Learnings (1)
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
🔇 Additional comments (3)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (3)
6-6: Hook import update looks correct.
useRefanduseEffectare correctly added for the new outside-click behavior.
105-130: Container ref wiring is clean and minimal.Wrapping
MultiSelectFilteredwith a ref container is a straightforward integration for outside-click handling.
135-135: Memoized export remains appropriate.No concerns with the final export after this change.
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (1)
91-124: Please add a regression test for this interaction path.Given this was a user-facing bug, adding a focused test (component or e2e) for “type in Members → click outside → dropdown closes/filter clears” would protect against regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx` around lines 91 - 124, Add a regression test (component or e2e) that simulates typing into UserAutoCompleteMultiple, verifies the dropdown/opened options from MultiSelectFiltered are visible, then triggers a click outside the container referenced by containerRef (the useOutsideClick handler) and asserts that the filter state is cleared and the dropdown is closed; target the UserAutoCompleteMultiple component, exercise the renderSelected path (selectedCache) and ensure after the outside click the filter prop passed into MultiSelectFiltered is empty and options are not visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx`:
- Around line 91-124: Add a regression test (component or e2e) that simulates
typing into UserAutoCompleteMultiple, verifies the dropdown/opened options from
MultiSelectFiltered are visible, then triggers a click outside the container
referenced by containerRef (the useOutsideClick handler) and asserts that the
filter state is cleared and the dropdown is closed; target the
UserAutoCompleteMultiple component, exercise the renderSelected path
(selectedCache) and ensure after the outside click the filter prop passed into
MultiSelectFiltered is empty and options are not visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7e4a7ef-80c7-4618-9a8d-b6d82dfa05c2
📒 Files selected for processing (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
📜 Review details
🧰 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/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
🧠 Learnings (4)
📚 Learning: 2026-03-06T18:10:23.330Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:23.330Z
Learning: In the RocketChat/Rocket.Chat `packages/gazzodown` package and more broadly, the HTML `<code>` element has an implicit ARIA role of `code` per WAI-ARIA 1.3, and `testing-library/dom` / jsdom supports it. Therefore, `screen.getByRole('code')` / `screen.findByRole('code')` correctly locates `<code>` elements without needing an explicit `role="code"` attribute. Do NOT flag `findByRole('code')` as invalid in future reviews.
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
🔇 Additional comments (2)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (2)
91-95: Good fix: outside-click handling is now correctly centralized viauseOutsideClick.This cleanly addresses the dropdown-close behavior without manual document listener management.
99-124: The ref wrapper is a safe, minimal integration point for the outside-click hook.No API surface change, and the behavior stays scoped to this component.
|
can you add a screen-shot too , I think this query already raised |
|
Hi @Pavani12314, I think there is already a PR for this issue with a better fix in fuselage repo: RocketChat/fuselage#1870 |
|
Thank you @vmridul for pointing this out! I see the fix in the fuselage
repo. Should I close this PR and wait for that fix to be merged? Or is
there value in keeping this as a temporary workaround?"
…On Mon, 16 Mar, 2026, 1:54 am Mridul verma, ***@***.***> wrote:
*vmridul* left a comment (RocketChat/Rocket.Chat#39633)
<#39633 (comment)>
Hi @Pavani12314 <https://github.com/Pavani12314>, I think there is
already a PR for this issue with a better fix in fuselage repo:
RocketChat/fuselage#1870
<RocketChat/fuselage#1870>
—
Reply to this email directly, view it on GitHub
<#39633 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BOJ3S76HG756YJNRVLLQSGD4Q4GP7AVCNFSM6AAAAACWSAOKDSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRTHAZDCMZQGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@Pavani12314 I'm the author of the fix in the fuselage repo. I haven't been able to contact any one of the maintainers to discuss getting merged could you please help me with that? |
|
Hi! I tested the behavior again and observed that:
I wanted to confirm whether this is the intended behavior for multi-select components, or if it should close after each selection. Happy to update or close the PR based on feedback. Thanks! |
|
This looks like a very useful fix. While going through the issue flow, I was wondering if the same outside-click behavior has been checked for keyboard navigation / focus loss cases as well, since that could affect accessibility and modal consistency. |
Description
Fixes #39620
The members dropdown in the Create Channel modal was not closing
when clicking outside of it.
Root Cause
The
UserAutoCompleteMultiplecomponent had no click-outside handler.Fix
Added a
useEffectwith amousedownevent listener that clearsthe filter and closes the dropdown when clicking outside the component.
Steps to Test
Summary by CodeRabbit
Bug Fixes
Refactor
Chores
Proposed changes (including videos or screenshots)