Skip to content

Conversation

@Aman-Maheshwari
Copy link
Contributor

@Aman-Maheshwari Aman-Maheshwari commented Nov 30, 2021

  • I have read the Contributing Guide
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Proposed changes (including videos or screenshots)

Removed onClose function from the Box Component inside searchList.js as this component wraps the complete list which is rendered and when clicked on the any rendered row inside the component it closes the searchList preventing the context menu to be displayed.
Also added usePreventDefault hook.

Before

before_sidebar_contextmenu.mp4

After

after_sidebar_contextmenu.mp4

Issue(s)

Closes #22146

Steps to test or reproduce

Mentioned in the issue

Further comments

This PR solves this issue but In this PR
User can close the searchList only by clicking out somewhere which is handled via useOutsideClick hook or by clicking on the cross icon in the search box. Also this gives user a chance to handle multiple search results inside searchList unlike everytime clicking the search icon on sidebar and perform action to a individual searched room or a user.
Before this PR selecting a room or a user also closed the searchList but this action also prevented the context menu to get open as the searchList was closed immediately after clicking.

@ankar84
Copy link

ankar84 commented Dec 1, 2021

Nice fix!

@Aman-Maheshwari
Copy link
Contributor Author

Thank you @ankar84 😄

@dougfabris dougfabris self-assigned this Dec 16, 2021
@CLAassistant
Copy link

CLAassistant commented Dec 17, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Aman-Maheshwari Nice fix, thanks for your help. In touch with our product team, we decided we should keep closing the spotlight after selecting a room. Also there's some conflicts, could you check it?

@dougfabris dougfabris added this to the 5.4.0 milestone Nov 3, 2022
@dougfabris dougfabris requested a review from a team as a code owner November 18, 2022 17:16
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Nov 18, 2022
@kodiakhq kodiakhq bot merged commit 5701525 into RocketChat:develop Nov 18, 2022
gabriellsh added a commit that referenced this pull request Nov 21, 2022
…age-ignore-reactivity

* 'develop' of github.com:RocketChat/Rocket.Chat: (23 commits)
  Chore: removing useEndpointData from license api (#26634)
  Chore: Create unique index for `E2EKey` field (#27301)
  [FIX] LDAP "Sync Roles" option doesn't work for custom roles (#26842)
  Chore: UserAvatar wrapper missing key in RoomForeword (#27300)
  Chore: Add deprecation warning to settings (#27295)
  Chore: Refactor LeaveTeam to Typescript (#27197)
  [FIX] Sidebar context menu in searchList (#23830)
  [NEW] Federation events coverage expansion (#27119)
  [FIX] Message Parser version upgrade (#27284)
  [FIX] Replace regex not compatible with safari (#27294)
  [NEW] Emphasis Elements (italic, strike and bold) in Message Parser Components (#27003)
  [FIX] Set default value "false" for global search. (#25568)
  Regression: Custom fields form not showing in user profile nor admin (#27244)
  [FIX] Sort RecordList items in REVERSE chronological order, not FORWARD (#27201)
  Chore: Remove unused css (#27289)
  Chore: also send thread to channel translations (#27242)
  [FIX] Marketplace app details page back-button behavior (#27062)
  Chore: Add tests for omni-jobs & add more context in system messages for jobs (#27048)
  Chore: Fix missing license for forks (#27258)
  i18n: Language update from LingoHub 🤖 on 2022-11-14Z (#27255)
  ...
MartinSchoeler pushed a commit that referenced this pull request Nov 28, 2022
MartinSchoeler pushed a commit that referenced this pull request Nov 28, 2022
@ggazzo ggazzo mentioned this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Sidebar context menu (hamburger) not working from search (magnifying glass)

5 participants