Hide icon-loading-dark after loading the image

[manishbisht]

Fixes: #160

Licence: MIT or AGPL

Description

Hide icon-loading-dark after loading the image and displaying it back when the image is loading.

Tested on

• Ubuntu 14.04/Chrome

Reviewers

@oparoz

[jancborchardt]

@jaller94 @oparoz can you review this please?

[jaller94]

For me this only works when viewing the image for the first time.

1. Click on a small image to view it in the gallery. (loading-icon will disappear).
2. Press left or right to switch to another image.
3. Switch back to your small image. This time the loading-icon won't disappear.

Can you reproduce this behaviour?

• Ubuntu 16.10/Chrome 54
• Ubuntu 16.10/Firefox 49

[codecov-io]

Current coverage is 99.75% (diff: 100%)

Merging #165 into master will not change coverage

@@             master       #165   diff @@
==========================================
  Files            39         39          
  Lines          1201       1201          
  Methods         170        170          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1198       1198          
  Misses            3          3          
  Partials          0          0          

Powered by Codecov. Last update ec0d5a0...95373a6

[manishbisht]

I have updated the PR can you review it again @jaller94 @oparoz

[jaller94]

It took me a long time to understand why you placed the two commands at those positions.
A line like // Removes loading icon if loading the image has finished and the user didn't aborted the process would be helpful. Is the function next is called on every picture (also the first picture and if you go to a previous image)? That name confused me.

[manishbisht]

@jaller94 For the first command what it does is it simply hides the loading icon after successfully loading the image. and the second command show the icon back if the image is still loading and then it is again hidden after loading the image using first command.

[jaller94]

I reviewed it and tested it on my personal Nextcloud instance. I approve of your Pull Request.

@manishbisht Hope you can convince the auto checks and someone in the project.

[jaller94]

These two additions in the code are perfectly suitable to fix the referenced issue.

[oliverpool]

You could move this before line 99.

I think it then looks more natural to call show() and then call hide() on resolution of the promise a couple of lines "later".

[oparoz]

I agree, show spinner, hide image, load image, hide spinner.

[manishbisht]

I have moved the code after the mentioned line @oparoz @oliverpool

[oliverpool]

@manishbisht it appears that you need to sign off your commit

[manishbisht]

Earlier this was not required. Good to see this feature here !!
It seems like I have to make the signoff commit using command git commit --signoff ?

[oliverpool]

It seems like I have to make the signoff commit using command git commit --signoff ?

From what I understand, you need to amend your commit with the git commit --signoff --amend command, for your commit to be signed.

[manishbisht]

@oliverpool I have signoff my commit