|
Description
Didier Gautheron
2020-02-27 09:42:13 UTC
Created attachment 99695 [details] [review] BUG 24740: Use biblio title rather than biblio number in OPAC search result cover images tooltips In OPAC biblionum doesn't mean much to patrons, use biblio title field if not empty. Test plan: OPAC search 1) Apply the patch 2) For each cover images providers available to tester in: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=Cover OPACLocalCoverImages, AdlibrisEnabled, OPACAmazonCoverImages, GoogleJackets, Coce provider. 3) Enable the provider. 4) Search a Library Catalog for a book with a cover image. 6) test mouse over cover image display biblio title. OPAC Shelves 1) Enable OPAC virtualshelves preference 2) Create a list. 3) For each covers providers available. 4) Search the List 5) test mouse over cover image display biblio title. Indeed, the biblio number doesn't mean much when used as a cover tooltip. I'm keen to Sign Off on your patch and have the following points to make: 1) "Author: Didier GAutheron" <- unless you want to be credited this way in the Koha release notes and in the About page timeline you may want to redo your git config and make a small change to your surname. 2) "BUG 24740: [...]": as per the official guidelines in the Koha wiki (https://wiki.koha-community.org/wiki/Commit_messages#Subject_line) the word "Bug" should be capitalised, and not be in all uppercase, otherwise the QA tools will complain. 3) The QA tools will also flag the missing filters at the lines where you've added [% img_title %], this is due to the coding guidelines' requirement that _all_ template variables must be filtered: https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML9:_Filter_all_template_variables You should probably remove the filters from the assignments inside the IF-ELSE blocks and use the "html" filter whenever the img_title variable is used later on in the TT code: [% IF ( SEARCH_RESULT.title ) %] [% img_title = SEARCH_RESULT.title %] [% ELSE %] [% img_title = SEARCH_RESULT.biblionumber %] [% END %] [% IF ( itemsloo.title ) %] [% img_title = itemsloo.title %] [% ELSE %] [% img_title = itemsloo.biblionumber %] [% END %] <span title="[% img_title | html %]" ... ></span> 4) You have used hard tabs to indent some of the code in your patch, the QA tools will notice this. I suggest you change them to soft tabs (spaces). The offending lines are: 254 and 256 in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt 327 and 329 in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt 5) Should we take this opportunity and fix the cover tooltip in the biblio details page as well? And the shelf browser, too? The relevant TT files are: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt koha-tmpl/opac-tmpl/bootstrap/en/includes/shelfbrowser.inc Hi Andreas, please use "Failed QA" if you run into problems when testing - the developers will usually check and reply. In Discussion should be used only if there is a disagreement that cannot be resolved without getting more people involved. (In reply to Katrin Fischer from comment #3) > Hi Andreas, please use "Failed QA" if you run into problems when testing - > the developers will usually check and reply. In Discussion should be used > only if there is a disagreement that cannot be resolved without getting more > people involved. Hi Katrin, thanks for the clarification! I've taken the opportunity to include the gist of your comment in the relevant page on the Wiki: https://wiki.koha-community.org/wiki/Development_workflow#Failure_cases
> Hi Katrin, thanks for the clarification! I've taken the opportunity to
> include
> the gist of your comment in the relevant page on the Wiki:
> https://wiki.koha-community.org/wiki/Development_workflow#Failure_cases
Great, thx!
Hi, thanks for you reply. (In reply to Andreas Roussos from comment #2) > 1) "Author: Didier GAutheron" <- unless you want to be credited this way Oops, thanks. > > 2) "BUG 24740: [...]": as per the official guidelines in the Koha wiki > (https://wiki.koha-community.org/wiki/Commit_messages#Subject_line) > the word "Bug" should be capitalised, and not be in all uppercase, > otherwise the QA tools will complain. Will do. > > 3) The QA tools will also flag the missing filters at the lines where > you've added [% img_title %], this is due to the coding guidelines' > requirement that _all_ template variables must be filtered: > Same. > 4) You have used hard tabs to indent some of the code in your patch, > the QA tools will notice this. I suggest you change them to soft > tabs (spaces). The offending lines are: Note: Tips in wiki guidlines seem to be wrong, You can fix tabs by using git config --global core.whitespace trailing-space,space-before-tab git config --global apply.whitespace fix Don't replace tab, I have it but still get tab. > > 254 and 256 in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt > 327 and 329 in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt > > 5) Should we take this opportunity and fix the cover tooltip in the biblio > details page as well? And the shelf browser, too? > > The relevant TT files are: > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt > koha-tmpl/opac-tmpl/bootstrap/en/includes/shelfbrowser.inc I'll look at it. Regards Didier Created attachment 102096 [details] [review] Bug 24740: Use biblio title rather than biblio number in OPAC search result cover images tooltips In OPAC biblionum doesn't mean much to patrons, use biblio title field if not empty. Test plan: OPAC search 1) Apply the patch 2) For each cover images providers available to tester in: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=Cover OPACLocalCoverImages, AdlibrisEnabled, OPACAmazonCoverImages, GoogleJackets, Coce provider. 3) Enable the provider. 4) Search a Library Catalog for a book with a cover image. 6) test mouse over cover image display biblio title. OPAC Shelves 1) Enable OPAC virtualshelves preference 2) Create a list. 3) For each covers providers available. 4) Search the List 5) test mouse over cover image display biblio title. Created attachment 102187 [details] [review] Bug 24740: Use biblio title rather than biblio number in OPAC search result cover images tooltips In OPAC biblionum doesn't mean much to patrons, use biblio title field if not empty. Test plan: OPAC search 1) Apply the patch 2) For each cover images providers available to tester in: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=Cover OPACLocalCoverImages, AdlibrisEnabled, OPACAmazonCoverImages, GoogleJackets, Coce provider. 3) Enable the provider. 4) Search a Library Catalog for a book with a cover image. 6) test mouse over cover image display biblio title. OPAC Shelves 1) Enable OPAC virtualshelves preference 2) Create a list. 3) For each covers providers available. 4) Search the List 5) test mouse over cover image display biblio title. Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Worked fine with books. Didn't get either title or biblionumber for e-books and films. (In reply to Stina Hallin from comment #9) > Worked fine with books. Didn't get either title or biblionumber for e-books > and films. Hi, Are you using a sandbox for testing, if yes which one? Hi, yes I'm using Bywaters sandbox. I used it with my library's account with syndetics to display covers. Hi, (In reply to Stina Hallin from comment #11) > Hi, yes I'm using Bywaters sandbox. I used it with my library's account with > syndetics to display covers. I haven't a syndetics account but have you an e-books or films' biblionumbers not showing tips? When I test it again, I can see the titles for different kinds of materials. So now it looks great. Everything I looked at showed tips. Created attachment 104170 [details] [review] Bug 24740: Use biblio title rather than biblio number in OPAC search result cover images tooltips In OPAC biblionum doesn't mean much to patrons, use biblio title field if not empty. Test plan: OPAC search 1) Apply the patch 2) For each cover images providers available to tester in: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=Cover OPACLocalCoverImages, AdlibrisEnabled, OPACAmazonCoverImages, GoogleJackets, Coce provider. 3) Enable the provider. 4) Search a Library Catalog for a book with a cover image. 6) test mouse over cover image display biblio title. OPAC Shelves 1) Enable OPAC virtualshelves preference 2) Create a list. 3) For each covers providers available. 4) Search the List 5) test mouse over cover image display biblio title. Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Nice work everyone! Pushed to master for 20.05 Backported to 19.11.x for 19.11.06 enhancement will not be backported to 19.05.x |