Bug 28314

Summary: Spinning icon is not always going away for local covers in staff
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: Staff interfaceAssignee: Hammat wele <hammat.wele>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: andrew, arthur.suzuki, caroline.cyr-la-rose, gmcharlt, lucas, m.de.rooy, martin.renvoize, olivier.hubert, philippe.blouin, wainuiwitikapark
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28179
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32642
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.03,22.05.10, 21.11.16
Circulation function:
Bug Depends on:    
Bug Blocks: 33839    
Attachments: 140 x 50 px image for tests
Bug 28314: Spinning icon is not always going away for local covers in staff
Bug 32366: (bug 30460 follow-up) Add tests
Bug 28314: Spinning icon is not always going away for local covers in staff

Description Katrin Fischer 2021-05-10 21:47:17 UTC
Noticed while testing bug 28179 that sometimes the 'spinning icon' for local covers won't go away on the small display on the left of the detail page in the staff interface. It looks like this is related to the size of the picture. I was using cropped screenshots, so nothing too high quality.

Joubu wrote:
>If the image is small you will see the loading image in the background.
>It's worth to open a new bug report.
Comment 1 Caroline Cyr La Rose 2023-01-16 15:41:16 UTC
The spinner is always visible with images which are under 80-90 px high 

To test:
1. Make sure the LocalCoverImages system preference is enabled
   1.1. Go to Administration > Global system preferences
   1.2. Search for Coverimages
   1.3. Set LocalCoverImages to 'Show'
   1.4. Click 'Save all Enhanced content preferences'
2. Add a short local cover image to a record (I added one which is just a grey rectangle with the size printed on it, to facilitate the test)
   2.1. Download the attached image
   2.2. Find a record without an image
   2.3. Click the 'Images' tab
   2.4. Click 'Upload'
   2.5. Click 'Drop files here or click to select a file' and choose the downloaded image
   2.6. Click 'Process images'
   2.7. Click on the title of the record in the page heading to access the detailed record
   --> Note that the image is displayed normally, but the spinner is also displayed

In the staff interface, the problem is only on the detailed record page. The search results are OK.
Comment 2 Caroline Cyr La Rose 2023-01-16 15:41:34 UTC
Created attachment 145333 [details]
140 x 50 px image for tests
Comment 3 Hammat wele 2023-01-19 15:54:52 UTC
Created attachment 145481 [details] [review]
Bug 28314: Spinning icon is not always going away for local covers in staff

The spinner is always visible with images which are under 80-90 px high

this patch remove the spinner when the image is loaded.

To test:
1. Make sure the LocalCoverImages system preference is enabled
   1.1. Go to Administration > Global system preferences
   1.2. Search for Coverimages
   1.3. Set LocalCoverImages to 'Show'
   1.4. Click 'Save all Enhanced content preferences'
2. Add a short local cover image to a record (I added one which is just a grey rectangle with the size printed on it, to facilitate the test)
   2.1. Download the attached image
   2.2. Find a record without an image
   2.3. Click the 'Images' tab
   2.4. Click 'Upload'
   2.5. Click 'Drop files here or click to select a file' and choose the downloaded image
   2.6. Click 'Process images'
   2.7. Click on the title of the record in the page heading to access the detailed record
   --> Note that the image is displayed normally, but the spinner is also displayed
4. Apply the patch
5. Search for the previous record with the small image
6. Click on the title of the record in the page heading to access the detailed record
   --> Note that the spinner is no more displayed
Comment 4 ByWater Sandboxes 2023-01-27 14:31:18 UTC
Created attachment 145748 [details]
Bug 32366: (bug 30460 follow-up) Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 5 Andrew Fuerste-Henry 2023-01-27 14:33:03 UTC
Comment on attachment 145748 [details]
Bug 32366: (bug 30460 follow-up) Add tests

This attachment was added in error by the sandbox.
Comment 6 Andrew Fuerste-Henry 2023-01-27 14:35:13 UTC
I tested this successfully on my Koha Testing Docker, but got an error when trying to sign off. I then tried to add a signoff via the sandbox and it attached a patch from another bug. Here's the error I got in KTD:

root@kohadevbox:koha(master)$ git bz attach -e 28314 HEAD
/usr/bin/git-bz:1908: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
  obsoleted = (initial_description == patch.description)
Traceback (most recent call last):
  File "/usr/bin/git-bz", line 2722, in <module>
    do_attach(*args)
  File "/usr/bin/git-bz", line 2074, in do_attach
    attach_commits(bug, commits, edit_comments=global_options.edit)
  File "/usr/bin/git-bz", line 1988, in attach_commits
    description, body, obsoletes, statuses, patch_complexities, depends = edit_attachment_comment(bug, commit.subject, body)
  File "/usr/bin/git-bz", line 1940, in edit_attachment_comment
    lines = edit_template(template.getvalue())
  File "/usr/lib/python2.7/StringIO.py", line 271, in getvalue
    self.buf += ''.join(self.buflist)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 3: ordinal not in range(128)

I've gotten this error on other patches, too. I don't believe it's anything to go with this code in particular. This patch works as described.
Comment 7 Martin Renvoize (ashimema) 2023-02-01 13:16:40 UTC
Created attachment 145938 [details] [review]
Bug 28314: Spinning icon is not always going away for local covers in staff

The spinner is always visible with images which are under 80-90 px high

this patch remove the spinner when the image is loaded.

To test:
1. Make sure the LocalCoverImages system preference is enabled
   1.1. Go to Administration > Global system preferences
   1.2. Search for Coverimages
   1.3. Set LocalCoverImages to 'Show'
   1.4. Click 'Save all Enhanced content preferences'
2. Add a short local cover image to a record (I added one which is just a grey rectangle with the size printed on it, to facilitate the test)
   2.1. Download the attached image
   2.2. Find a record without an image
   2.3. Click the 'Images' tab
   2.4. Click 'Upload'
   2.5. Click 'Drop files here or click to select a file' and choose the downloaded image
   2.6. Click 'Process images'
   2.7. Click on the title of the record in the page heading to access the detailed record
   --> Note that the image is displayed normally, but the spinner is also displayed
4. Apply the patch
5. Search for the previous record with the small image
6. Click on the title of the record in the page heading to access the detailed record
   --> Note that the spinner is no more displayed

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2023-02-01 13:17:07 UTC
Nice little fix, no regressions. QA scripts happy, Passed!
Comment 9 Tomás Cohen Arazi (tcohen) 2023-02-03 13:32:34 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 10 Jacob O'Mara 2023-02-06 16:27:50 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 11 Lucas Gass (lukeg) 2023-02-14 22:12:14 UTC
Backported to 22.05.x for upcoing 22.05.10
Comment 12 Arthur Suzuki 2023-02-17 10:34:23 UTC
applied to 21.11.x for 21.11.16
Comment 13 Wainui Witika-Park 2023-03-15 01:45:37 UTC
Not backported to 21.05.x
Comment 14 Marcel de Rooy 2023-04-21 11:05:09 UTC
This code may get further attention. See change on 32642 for OPAC.

The line $(coverSlide).addClass("cover-slides"); is actually hilarious, since we start showing the spinner after all images have been loaded. And then rightaway try to remove it.