Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: close image gallery on backdrop click #31344

Merged
merged 2 commits into from
Jan 2, 2024
Merged

fix: close image gallery on backdrop click #31344

merged 2 commits into from
Jan 2, 2024

Conversation

Sayan4444
Copy link
Contributor

@Sayan4444 Sayan4444 commented Dec 29, 2023 •
edited by hugocostadev
Loading

Proposed changes (including videos or screenshots)
BEFORE THE FIX
https://github.com/RocketChat/Rocket.Chat/assets/112304873/c3948a9b-21c0-40f9-8f4c-97e79909a698

AFTER THE FIX
https://github.com/RocketChat/Rocket.Chat/assets/112304873/c7a48b70-4951-43e3-9e35-a879107d61de

REASON BEHIND CHANGES
There are far better ways to implement this but with this configuration,this is the best way to implement this feature

Issue(s)
This should close issues :-
closes #31341

Steps to test or reproduce

Login to rocket chat.
Go to any Community chat.
Click on any media

@Sayan4444 Sayan4444 requested a review from a team as a code owner December 29, 2023 21:18
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Dec 29, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: df207ea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov Codecov
Copy link

codecov bot commented Dec 29, 2023 •
edited
Loading

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (9ef1442) 49.33% compared to head (df207ea) 49.35%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #31344      +/-   ##
===========================================
+ Coverage    49.33%   49.35%   +0.01%     
===========================================
  Files         3308     3311       +3     
  Lines        81299    81337      +38     
  Branches     16650    16657       +7     
===========================================
+ Hits         40109    40143      +34     
+ Misses       36496    36491       -5     
- Partials      4694     4703       +9
Flag Coverage Δ
e2e 52.51% <0.00%> (+0.04%) ⬆️
e2e-api 40.68% <ø> (+<0.01%) ⬆️
unit 76.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@hugocostadev
Copy link
Contributor

Instead of doing that I would create a new function and check if the element that was clicked (overlay) contains the class that I want, in that case, swiper-container.

WDYT @juliajforesti ?

@juliajforesti
Copy link
Contributor

Instead of doing that I would create a new function and check if the element that was clicked (overlay) contains the class that I want, in that case, swiper-container.

WDYT @juliajforesti ?

That selector might not work, since there's another layer of container on top (swiper-zoom-container).
IMO it would be confusing to check if the element contains that class to conditionally close...

I like the current solution :)

@dougfabris dougfabris added this to the 6.6 milestone Jan 2, 2024
@juliajforesti juliajforesti changed the title fix: Minimize the media by clicking outside the image, instead of using the minimize arrow fix: close image gallery on backdrop click Jan 2, 2024
juliajforesti
juliajforesti approved these changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@juliajforesti juliajforesti left a comment

Choose a reason for hiding this comment

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

@Sayan4444 thank you for your contribution!

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Jan 2, 2024
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Jan 2, 2024
@kodiakhq kodiakhq bot merged commit c2e7116 into RocketChat:develop Jan 2, 2024
43 checks passed
@Sayan4444 Sayan4444 deleted the image_minimize branch January 2, 2024 15:51
gabriellsh added a commit that referenced this pull request Jan 3, 2024
@gabriellsh
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into new/…
096265a 
…hideUi

* 'develop' of github.com:RocketChat/Rocket.Chat: (112 commits)
  chore: update yarn.lock
  chore: Replace `FramedIcon` in favor of fuselage component (#31356)
  chore: fix dep version
  Release 6.5.2
  fix: wrong value used for workspace registration
  ci: update release PR description with change log
  fix: inquiries being limited to 50 items (#31267)
  fix: close image gallery on backdrop click (#31344)
  fix: No error message when uploading a file that is not allowed (#31346)
  fix: Text area only shows the last line of the message for very long texts #31252 #31257 (#31262)
  chore: adding new streamer event for imported messages (#31063)
  chore: adapt read receipts feature to work without oplog on msgs (#30996)
  chore: remove Messages model dependency of Rooms model (#31272)
  chore: `Card` normalization (#31335)
  refactor: unify useStream and useSingleStream (#31326)
  chore: add new msg stream for reported msgs (#31087)
  Bump 6.5.2
  feat: Highlight words in thread messages (#31281)
  fix: Show only departments that are allowed on registration page on livechat widget (#31302)
  fix: Fallback forward department not working as expected on chat forwarding (#31296)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliajforesti juliajforesti juliajforesti approved these changes

Assignees
No one assigned
Labels
communityPR stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Milestone
6.6
Development

Successfully merging this pull request may close these issues.

Minimize the media by clicking outside the image, instead of using the minimize arrow.
4 participants
@Sayan4444 @hugocostadev @juliajforesti @dougfabris