Bug 7594

Summary: Google Cover Javascript contains hardcoded CSS style
Product: Koha Reporter: Shane Sammons <ssammons>
Component: OPACAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: brendan, katrin.fischer, veron, wizzyrea
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 7594 - Google Cover Javascript contains hardcoded CSS style
Bug 7594 - Google Cover Javascript contains hardcoded CSS style
Bug 7594: Google Cover Javascript contains hardcoded CSS style

Description Shane Sammons 2012-02-24 19:22:52 UTC
In google-jackets.js there is hard coded "style" on line 48 using...
http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=koha-tmpl/opac-tmpl/prog/en/js/google-jackets.js;h=70b1b84337c4c3807cd8e1997a2fda6cd9563004;hb=HEAD

I suggest this is moved to a .css file, as to allow the end user to impose changes to the style.

Additionally, it would be nice if a unique class can be added to all the image preview types. Currently they are all dynamic (class & id). Added a class such as "gbs-cover" or similar would allow end users to use CSS to uniquely style each book cover used. I am not able to find where this is generated or I would suggest the lines of code to change.

Finally, the "No Image Available" does not appear in the HTML source code. The JS shows it creating a "span" element with a class of no-image. Is there anyway to actually have that appear in the actual source code?

All suggested changes should allow for an end user to have easier and clearer control of OPAC search generated images.
Comment 1 Liz Rea 2012-02-24 19:54:31 UTC
(In reply to comment #0)
> In google-jackets.js there is hard coded "style" on line 48 using...
> http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=koha-tmpl/opac-tmpl/prog/en/js/google-jackets.js;h=70b1b84337c4c3807cd8e1997a2fda6cd9563004;hb=HEAD
> 
> I suggest this is moved to a .css file, as to allow the end user to impose
> changes to the style.

Changes that make the styling of the OPAC more granular are always welcome.

> 
> Additionally, it would be nice if a unique class can be added to all the image
> preview types. Currently they are all dynamic (class & id). Added a class such
> as "gbs-cover" or similar would allow end users to use CSS to uniquely style
> each book cover used. I am not able to find where this is generated or I would
> suggest the lines of code to change.

A good point.

> 
> Finally, the "No Image Available" does not appear in the HTML source code. The
> JS shows it creating a "span" element with a class of no-image. Is there anyway
> to actually have that appear in the actual source code?
> 
> All suggested changes should allow for an end user to have easier and clearer
> control of OPAC search generated images.
Comment 2 Owen Leonard 2012-03-06 21:01:54 UTC
(In reply to comment #0)
> In google-jackets.js there is hard coded "style" on line 48 using...
> http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=koha-tmpl/opac-
> tmpl/prog/en/js/google-jackets.js;h=70b1b84337c4c3807cd8e1997a2fda6cd9563004;
> hb=HEAD

The line in question reads:

$("."+book.bib_key).each(function() {

This is the script's way of looping over elements with that particular class (i.e. any title with that same ISBN.). There is nothing at that line which sets a hard-coded style.

There is an invalid style hard-coded in opac-results.tt at line 542 and 543 (in master):

<div style="block"...

The correct way to say that would be <div style="display:block"... but that is redundant for divs, so I recommend we remove the style attribute.

> Finally, the "No Image Available" does not appear in the HTML source code.

It does in two places. In the template (opac-results.tt) it is output if the record has no ISBN:

[% IF ( SEARCH_RESULT.normalized_isbn ) %]...[% ELSE %]
<span class="no-image">No cover image available</span>[% END %]

Then the various scripts output the same thing if they don't return an image. For instance, in google-jackets.js:

    var message = document.createElement("span");
    $(message).attr("class","no-image");
    $(message).html(NO_GOOGLE_JACKET);
    $(this).append(message);

This creates the <span> element, adds the "no-image" class, fills the tag with the NO_GOOGLE_JACKET message (specified in opac-result.tt) and appends the newly-built <span> to the <div>.

The "no-image" class can be modified globally, but if you want multiple cover sources enabled at the same time and want the messages styled differently you'll have to add another class as well, both in the JS and the template:

    $(message).attr("class","no-image google");

Then your custom stylesheet could address it:

span.no-image.google { border: 3px dotted blue; }

And you could write some JS for OpacUserJS to change the message using that class to target it.
Comment 3 Shane Sammons 2012-03-07 04:07:22 UTC
Yeah, we addressed everything today via IRC...

However, I made a mistake, not line 48, but 58 has the following:

    $(this).append(
  58                              '<div style="margin-bottom:5px; margin-top:-5px;font-size:9px">' +
  59                              '<a '+target+'href="' +
  60                              book.info_url +
  61                              '"><img src="' +
  62                              'http://books.google.com/intl/en/googlebooks/images/gbs_preview_sticker1.gif' +
  63                              '"></a></div>'

We should probably fix that to be a class and put it in the .css file. I will look into what file (hopefully tomorrow).
Comment 5 Owen Leonard 2016-03-22 12:24:37 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2016-03-23 08:42:42 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2016-03-23 14:42:31 UTC
Created attachment 49440 [details] [review]
Bug 7594: Google Cover Javascript contains hardcoded CSS style

The JavaScript which handles the display of Google book cover images
includes a hard-coded "style" attribute. It may have been relevant to
the design of the old prog template, but it doesn't seem to have any use
in the Bootstrap one. This patch removes it.

I have replaced the style attribute with a class in case someone needs
a hook for some custom CSS.

To test, apply the patch, enable the GoogleJackets system preference,
and clear your browser cache if necessary.

- Search for a title in the OPAC which has a Google book cover image
  associated with it.
- View the detail page for that title. Confirm that the "Google Preview"
  link underneath the cover image looks correct.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Brendan Gallagher 2016-03-23 23:08:24 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!