search

jpilfold / ngx-image-viewer

An image viewer component for Angular 2+
MIT License
49 stars 55 forks source link

Enhencement #14

Closed saberrahul closed 6 years ago

saberrahul commented 6 years ago

While the current image is loading, some spinner or loader should be displayed. As soon as the image is loaded or failed to load, spinner or loader should be hidden.

jpilfold commented 6 years ago

I've made this change and released to npm as part of 1.0.8, let me know if you're happy with the change so we can close this ticket

jpilfold commented 6 years ago

@saberrahul This has been implemented as well. Let me know if you're happy with the change. Thanks

saberrahul commented 6 years ago

The black screen while image loading looks good. Although, a spinner could have looked better, but this serves the purpose.

Great work...

Can this be extended in such a way that when image is changed using arrows, spinner or the same black screen is displayed until the image is ready ?

jpilfold commented 6 years ago

That's interesting; it should be showing a spinner.... If you visit https://stackblitz.com/edit/angular-wfmjkb?file=app%2Fapp.component.html do you see the spinner, or a black screen?

Also, the loading icon should be displayed any time the image load event is fired, so it should be showing every time the image changes, if it isn't loaded from the browser cache.

@saberrahul Can I ask which browser and browser version you're using?

saberrahul commented 6 years ago

When I visit the said URL on chrome (version 65.0.xxx), I don't see any spinner or black screen.

Though, this works well on Firefox.

Apart from this, below is a link to some really awesome and easy to plug spinners. https://www.npmjs.com/package/angular2-loaders-css

jpilfold commented 6 years ago

I was trying to avoid bringing in another dependency if I could at all help it

jpilfold commented 6 years ago

@saberrahul It looks like this was an issue with events, not the spinner itself. Chrome doesn't fire the onloadstart event, so the component did not know when to show the spinner.

I've changed the way this is done. Can you take another look when you're free?

Thanks

saberrahul commented 6 years ago

Checked again, sorry but chrome still does not shows any spinner. On the other hand Firefox and edge are showing the spinner perfectly.

jpilfold commented 6 years ago

Is that using the version in npm, or did you check out the code in the master branch? I've not yet pushed it to npm

saberrahul commented 6 years ago

My bad, I tried it with what's there on npm.

saberrahul commented 6 years ago

Since you have fixed the issue on chrome as well, I will go ahead and close this issue. Kindly add it to npm asap. Great Work @jpilfold

jpilfold commented 6 years ago

@saberrahul Thanks. This is now published to npm, v1.0.10