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

Allow slide to close for videos #286

Open
Beagon opened this issue Sep 5, 2021 · 3 comments
Open

Allow slide to close for videos #286

Beagon opened this issue Sep 5, 2021 · 3 comments

Comments

@Beagon
Copy link

Beagon commented Sep 5, 2021

Please describe the feature you want to be implemented.
I would like to be able to close a video with sliding up or down just like an image on mobile. With an opt-in option of course.

Explain why the feature is useful
This would help making the experience more "mobile app like" by being able to have the same interaction on images as videos. This would also remove the need for the close button (if images and videos are the only content shown in the gallery).

Have you seen it somewhere else?
Most social media apps/sites that have videos and images.

Additional context
I've been able to do this myself by using a custom version of gLightbox. I've added the following code to touch-navigation.js:

if (hasClass(media, 'gslide-video')) { mediaImage = media.querySelector('.gvideo'); }

On line 104 underneath the if of if (hasClass(media, 'gslide-image')) {.

This solution doesn't interfere with other types of iFrames and Inline content. I would make a full PR but my Javascript is quite rusty. (I can easily add the hasClass but no clue how to add the configurable option without going trough the full code)

@gingerchew
Copy link
Collaborator

Hey there @Beagon,

That's an interesting idea! You're more than welcome to give it a shot yourself!

Make sure to check out the contributing guide as well.

@Beagon
Copy link
Author

Beagon commented Sep 14, 2021

@Frankie-tech I'll give it a go in my free time. Would you like this to be optional or included in touch events?

Beagon added a commit to Beagon/glightbox that referenced this issue Sep 14, 2021
Beagon added a commit to Beagon/glightbox that referenced this issue Sep 14, 2021
Beagon added a commit to Beagon/glightbox that referenced this issue Sep 14, 2021
Beagon added a commit to Beagon/glightbox that referenced this issue Sep 14, 2021
@gingerchew
Copy link
Collaborator

Yeah, it should be optional, the way you added the setting seems fine to me.

It should be covered by the existing TouchEvents class. Since you've made a PR, I'm assuming you've already figured that out, haha.

Beagon added a commit to Beagon/glightbox that referenced this issue Sep 15, 2021
Beagon added a commit to Beagon/glightbox that referenced this issue Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants