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

[Feature Request] Add loading class for navigating between gallery images #356

Open
dbworth opened this issue Apr 15, 2018 · 4 comments
Open

Comments

@dbworth
Copy link
Contributor

dbworth commented Apr 15, 2018

If using the Gallery for images with a large file size, it would be useful to have the option of displaying a loading animation when you click the "next" or "previous" buttons.

A previous solution was to always display the animation in the background:
https://stackoverflow.com/questions/49582414/featherlight-gallery-doesnt-display-loader-animation-loading-next-image-lightb/49583318#49583318

Here is my attempt to toggle the loading class when navigating in the gallery:
PR #355

To see the result, look here:
Down in the bottom-right you can see 3 images.
Click the middle image and a loading animation will display (current behaviour).
Click "previous" and the loading animation will display again (new behaviour).
http://jsfiddle.net/cvjxydfd/

I called the class "loading-gallery" so it doesn't change existing functionality for current users, and allows new users the choice.

I welcome any alternative ideas or suggestions to improve this PR.

@marcandre
Copy link
Collaborator

Thanks for the report & PR.
I was wondering: wouldn't the existing "loading" class be sufficient? (i.e. instead of setting it it once as it is currently, it should be set whenever the content change)

@dbworth
Copy link
Contributor Author

dbworth commented Apr 24, 2018

Yes you could also use the existing "loading" class. That's ok with me.
I was worried that maybe some users preferred the existing behaviour, and such a change would alter that existing behaviour if they upgraded the library.

@marcandre
Copy link
Collaborator

Right. Still, I think it's semantically correct to have the 'loading' class toggle while the content is loading, no matter if it's the first load or subsequent ones. Would you like to tweak your PR?

@dbworth
Copy link
Contributor Author

dbworth commented Apr 29, 2018

No worries, I updated the PR #355
and the test:
http://jsfiddle.net/zehu9vge/

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