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

Implement loading spinner, improve UX for slow internet connection #187

Merged
merged 3 commits into from Dec 27, 2017

Conversation

mkalygin
Copy link
Contributor

@mkalygin mkalygin commented Dec 22, 2017

Description of changes:

For testing, I've been using Chrome's network throttler. I tested these changes in all modern browsers. Don't forget to add react-spinners to rollup.config.js.

  • Implement srcset preloading.
  • Don't allow spamming of next/prev when image is not loaded yet.
  • Implement loading spinner, use react-spinners BounceLoader by default.
  • Implement new properties: spinner, spinnerColor, spinnerSize for spinner customization.
  • Improve UX for slow internet connection: show spinner, fade-in animation for loaded image.
  • Refactor code of Lightbox by moving rendering code to renderers.
  • Update examples with new spinner properties customizations.
  • Add documentation about new spinner properties to README.md.

Related issues (if any):

#93

Checks:

  • Please confirm npm run lint ran successfully
  • Please confirm that only /src and /examples/src are committed
  • [if new feature] Please confirm that documentation was added to the README.md

Screenshots:

image

@mkalygin
Copy link
Contributor Author

I'm still working on improving UX. Currently there is a blink of spinner when the connection is fast. Very annoying. After fixing it, I think this PR is good to go, unless you find any other issues with it.

@mkalygin
Copy link
Contributor Author

Now this is a lot better. It's ready for review and QA.

@jossmac
Copy link
Owner

jossmac commented Dec 27, 2017

This is awesome! Thanks @mkalygin 🎉

@jossmac jossmac merged commit d67f857 into jossmac:master Dec 27, 2017
@igorescobar
Copy link

@mkalygin was this released or it is just on master branch yet?

@igorescobar
Copy link

igorescobar commented Jan 12, 2018

Does anyone managed to get this working? I see that this was released already but I really couldn't make it work. No errors, nothing, the loader just doesn't appear. Everything else works fine... except the arrows. It is working kinda of buggy. Seems that if the next image isn't loaded the arrow doesn't work. I have to close the Lightbox and click on the images to be able to see it.

What kind of information do you want me to provide?

@mkalygin
Copy link
Contributor Author

@igorescobar it seems that @jossmac forgot to update JS build for this and released a new version without these changes. In the meanwhile feel free to use it from my original branch: https://github.com/mkalygin/react-images.

@igorescobar
Copy link

cool! thanks @mkalygin!
@jossmac let us know once it's fixed, thanks!

@mkalygin
Copy link
Contributor Author

@igorescobar to be precise, the ready-to-use version in loading-build branch: https://github.com/mkalygin/react-images/tree/loading-build

@igorescobar
Copy link

@mkalygin that explains a lot ^^
Thanks!

@neptunian
Copy link
Collaborator

I published the build files for this functionality. It looks great thanks @mkalygin !

@mkalygin
Copy link
Contributor Author

@neptunian thank you very much, I'm glad to help!

@igorescobar
Copy link

@mkalygin @neptunian thank you so much guys!

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

Successfully merging this pull request may close these issues.

None yet

4 participants