Like bug 34570, we want to get rid of "onclick".
Created attachment 155255 [details] [review] Bug 34724: Remove inline onclick handler for opac-imageviewer.pl This patch moves the click handler for thumbnails for opac-imageviewer.pl out of the inline HTML and out to an existing <script> element. Test plan: 0. Apply the patch (including dependent patches) 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=localcover 2. Change both *LocalCoverImages sysprefs to "Show" 3. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AllowMultipleCovers 4. Change "AllowMultipleCovers" to "Allow" 5. Click "Images (0)" tab 6. Download or make two JPEG images 7. Upload the images 8. Go to http://localhost:8080/cgi-bin/koha/opac-imageviewer.pl?biblionumber=29 9. See one large image in the centre and two smaller thumbnails on the right side 10. Click on the thumbnail of the image that isn't displayed in the centre 11. Note that the image changes 12. Click the other thumbnail and note that the image changes back
Created attachment 155299 [details] [review] Bug 34724: Remove inline onclick handler for opac-imageviewer.pl This patch moves the click handler for thumbnails for opac-imageviewer.pl out of the inline HTML and out to an existing <script> element. Test plan: 0. Apply the patch (including dependent patches) 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=localcover 2. Change both *LocalCoverImages sysprefs to "Show" 3. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AllowMultipleCovers 4. Change "AllowMultipleCovers" to "Allow" 5. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29 6. Click "Images (0)" tab 7. Download or make two JPEG images 8. Upload the images 9. Go to http://localhost:8080/cgi-bin/koha/opac-imageviewer.pl?biblionumber=29 10. See one large image in the centre and two smaller thumbnails on the right side 11. Click on the thumbnail of the image that isn't displayed in the centre 12. Note that the image changes 13. Click the other thumbnail and note that the image changes back PA amended patch: Added missing test plan step Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 155300 [details] [review] Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist The link does not contain translatable strings, it doesnt need to be repeated twice in if-else just for a css class change Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Hi David, I've submitted a follow-up patch. Please take a look and SO if you agree, or make obsolete if you don't.
Comment on attachment 155300 [details] [review] Bug 34724: (QA follow-up): The "src" and Javascript change looked all right, but not the change to "loop.first". I think the translation tools don't like Template Toolkit syntax embedded in HTML except for things like [% image.imagenumber | html %]. So even though there's a lot of duplicated HTML in the current template, it's for reasons. And I think that is the main reason.
(In reply to Pedro Amorim from comment #4) > Hi David, I've submitted a follow-up patch. > Please take a look and SO if you agree, or make obsolete if you don't. If you want to redo your patch to not include that loop.first change, then I'd SO it.
(In reply to David Cook from comment #5) > I think the translation tools don't like Template Toolkit syntax embedded in > HTML except for things like [% image.imagenumber | html %]. Correct: https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML
Created attachment 155445 [details] [review] Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist
Cool! Thank you both!
Created attachment 155472 [details] [review] Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist Signed-off-by: David Cook <dcook@prosentient.com.au>
(In reply to Pedro Amorim from comment #9) > Cool! Thank you both! Np!
Created attachment 155758 [details] [review] Bug 34724: Remove inline onclick handler for opac-imageviewer.pl This patch moves the click handler for thumbnails for opac-imageviewer.pl out of the inline HTML and out to an existing <script> element. Test plan: 0. Apply the patch (including dependent patches) 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=localcover 2. Change both *LocalCoverImages sysprefs to "Show" 3. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AllowMultipleCovers 4. Change "AllowMultipleCovers" to "Allow" 5. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29 6. Click "Images (0)" tab 7. Download or make two JPEG images 8. Upload the images 9. Go to http://localhost:8080/cgi-bin/koha/opac-imageviewer.pl?biblionumber=29 10. See one large image in the centre and two smaller thumbnails on the right side 11. Click on the thumbnail of the image that isn't displayed in the centre 12. Note that the image changes 13. Click the other thumbnail and note that the image changes back PA amended patch: Added missing test plan step Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 155759 [details] [review] Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.04
Nice work everyone! Pushed to oldstable for 22.11.x