This repository has been archived by the owner on May 22, 2020. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All of these bugs can be seen from the demo
The lightbox gallery is very powerful, but the UX lacks a lot.
Bug 1: Overlay's too good at overlaying
Let's check the Image gallery demo, since the nav-overlay takes up the whole container, is impossible to right click images and, for example, save them.
Fix: No more overlay
Instead of having a nav-overlay, simply create a common class .ekko-lightbox-nav and 2 classes .ekko-lightbox-nav-left and .ekko-lightbox-nav-right for each control, style them accordingly and voilá, pretty much the same effect. Obviously the event assignment is a bit different, but works the same as before
Bug 2: currentContainer and toUse
The $currentContainer and $toUse properties when switching between slides tend to mess up with the z-index, resulting in, for example, unpausable embedded videos (check the Mixed Gallery example, and select first an image, and nav your way to a video, all events are captured by the slide now has no content).
Fix: z-index modifications.
Simply change the z-index of the previous slide to 0, and set the new slide's to 1. Now video controls are always available, regardless of the previous element
Bonus: No controls on videos
I don't know why, but nav controls are purposely hidden on videos. I figured this decision was because since the overlay covers the whole container, it would be impossible to have some sort of control over the video
Fix: see bugfix 1
With the new controls design, you can still pause/unpause the videos and have the controls at the same time. I tested that on a project and so far it has worked with no problems. (Tested only with YT vids so far, though).
In conclussion, I hope you consider the changes I've made, I think they can improve a lot the power of this awesome plugin :D
Greetz,
Tomás