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.

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

Lightbox closes when image clicked when backdropClosesModal = true #152

Closed
benhowell opened this issue Jul 20, 2017 · 5 comments
Closed

Comments

@benhowell
Copy link
Contributor

benhowell commented Jul 20, 2017

Steps to reproduce the behavior:
Set backdropClosesModal = true
Click on image in lightbox.

Expected behavior:
Lightbox shouldn't close.

Actual behavior:
Lightbox closes.

Further info
It appears that backdropClosesModal overrides onImageClick. Further, supplying a custom onImageClick function with event.preventDefault() and event.stopPropagation() allows onClickImage to function properly but disables backdropClosesModal.

This issue has been indirectly reported previously here: #135

@neptunian
Copy link
Collaborator

I tested this and overrriding of onClickImage doesn't seem to actually happen however the image is indeed closing when backdropClosesModal is set to true and you click on the image. Adding event.stopPropagation(); in the onClickImage handler fixed this for me. I will update the demo code.

@benhowell
Copy link
Contributor Author

benhowell commented Jul 25, 2017

onClickImage handler is optional though so propagation needs to be handled in lightbox for cases where that prop is not supplied. Please re-open this issue.

@neptunian
Copy link
Collaborator

let me know if that looks ok. this also resolved #155. seemed liked the cleanest/safest way to only target the backdrop click.

@benhowell
Copy link
Contributor Author

I haven't tested but the logic seems correct to me. Personally, the route I'd take would be to have a default prop for the onClickImage func containing a call to event.stopPropagation() to prevent the onClick func of Container being triggered. If one were to chose to provide their own onClickImage function, they would also need to provide event.stopPropagation() if they didn't want the backdrop to be closed. A third option would be to always call stopPropagation and if a custom onClickImage were provided to just call that prior to calling stopPropagation.

Anyway, whatever works, works. 👍

@neptunian
Copy link
Collaborator

neptunian commented Jul 28, 2017

@benhowell That wouldnt solve #155 or any other items the user might click on that is within the container that should not close the lightbox. it's not just an onClickImage problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants