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

Add a spinner while loading fullsize image #75

Open
jimblue opened this issue Jun 4, 2018 · 8 comments
Open

Add a spinner while loading fullsize image #75

jimblue opened this issue Jun 4, 2018 · 8 comments
Labels

Comments

@jimblue
Copy link

jimblue commented Jun 4, 2018

Hi there!

Actually when clicking on an image, Zooming doesn't provide any visual information to let the user know a better resolution image will show when loaded.

This can be a problem on devices with poor connection as users might close the Zoomed image before the full size image is displayed.

To let users know something better is coming, what do you think about adding a div with a css spinner while the full size image is loading.

Thanks for the good work on this awesome library btw 😄 !!

Cheers

@kingdido999
Copy link
Owner

kingdido999 commented Jun 4, 2018

Yeah this is a good idea. However, I want to be careful about making it an opt-in feature because some people might not needed it. Maybe we can introduce two new event hooks onImageLoading and onImageLoaded, then user can add/remove loading indicator with their own styling. I'll experiment with it and see how it goes :)

@jimblue
Copy link
Author

jimblue commented Jun 5, 2018

Adding new event hooks also make sens for customisation, everyone will be able to add their own spinner design. That sounds great!

@kingdido999 kingdido999 changed the title [Feature request] add a spinner while loading fullsize image Add a spinner while loading fullsize image Jun 5, 2018
kingdido999 pushed a commit that referenced this issue Jun 5, 2018
@kingdido999
Copy link
Owner

kingdido999 commented Jun 12, 2018

The above two event hooks have been added to the latest release 2.1.0:

But I haven't figured out the most appropriate way to indicate the loading status yet. One thing I'm sure about is that we need to set a certain time threshold for the indicator to only show up in slow internet connection or the first fetch, otherwise it blinks which is a bit annoying.

@migueldemoura
Copy link
Collaborator

One thing I'm sure about is that we need to set a certain time threshold for the indicator to only show up in slow internet connection or the first fetch, otherwise it blinks which is a bit annoying.

Turbolinks does this with a 500ms delay before showing the progress bar. I doubt this is based on any UX study, but I quite like the value.

@jimblue
Copy link
Author

jimblue commented Jun 12, 2018

Hey @kingdido999,

Thanks for the new release!
I've wrote a promise function to check image load properly.
I use it to lazyload image everyday, it's pretty solid!
You can probably improve the new event onImageLoaded with it.

/**
 * Method to fetch image from source
 *
 * @param {URL} imgSource
 * @returns {Promise}
 */
const fetchImage = imgSource => {
  // Return a promise to fetch the image
  return new Promise((resolve, reject) => {
    // Create a new image
    const imgElement = new Image()

    // Define events callbacks
    const loadCallback = () => {
      // Remove events listener
      imgElement.removeEventListener('load', loadCallback)
      imgElement.removeEventListener('error', errorCallback)

      // Resolve Promise
      resolve()
    }

    const errorCallback = () => {
      // Remove events listener
      imgElement.removeEventListener('load', loadCallback)
      imgElement.removeEventListener('error', errorCallback)

      // Reject Promise
      reject(Error(`Error while loading image at ${imgSource}`))
    }

    // Listen to load and error events on image
    imgElement.addEventListener('load', loadCallback)
    imgElement.addEventListener('error', errorCallback)

    // Set image source
    imgElement.src = imgSource
  })
}

You can use it like so:

fetchImage(imgSource).then(() => { console.log('image loaded') })

@jimblue
Copy link
Author

jimblue commented Jun 13, 2018

About the time threshold, as usage will probably be different for each case, I suggest you to simply add an exemple into the doc.

In the end it's simple as adding a timeout to onImageLoaded callback... nope?

@jimblue
Copy link
Author

jimblue commented Jun 13, 2018

Lastly to prevent the problem of the first fetch you could create an empty loaded array that will be filled as data-original source are loaded:

let loaded = []

if (loaded[imgIndex] === true) {
    onImageLoaded()
} else {
     fetchImage(imgSource).then(() => {
        onImageLoaded()
        loaded[imgIndex] = true
    })
}

This will help not impact performances.

@jimblue
Copy link
Author

jimblue commented Nov 18, 2018

Hi @kingdido999!

Can we have your input on this please?

Tks

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

No branches or pull requests

3 participants