[Koha-bugs] [Bug 32680] Add hooks to allow cover images to be provided by plugins

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32680

--- Comment #8 from David Cook <dcook at prosentient.com.au> ---
Comment on attachment 145475
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145475
Bug 32680: Add hooks to allow cover art to come from plugins

Review of attachment 145475:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=32680&attachment=145475)
-----------------------------------------------------------------

Overall, I like the idea, but I'm not sure about the implementation. I think
this could benefit from a dedicated plugin hook.

::: catalogue/detail.pl
@@ +684,5 @@
>  
>  $template->param(biblio => $biblio);
>  
> +my $intranet_js_plugins = Koha::Template::Plugin::KohaPlugins->get_plugins_intranet_js;
> +if(index($intranet_js_plugins, "Cover Image Plugin") != -1){

To me this looks like hacking a different plugin hook rather than adding a
plugin hook 

https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks

::: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
@@ +1310,4 @@
>      [% Asset.js("js/recalls.js") | $raw %]
>      [% Asset.js("js/coce.js") | $raw %]
>      [% Asset.js("lib/Chocolat/js/chocolat.js") | $raw %]
> +    [% IF ( Cover_Images_Required ) %]

I'd suggest going with CoverImagesRequired. We tend to use camel case rather
than snake case in the templates.

::: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
@@ +465,2 @@
>      <td class="bookcoverimg">
> +        <div id="cover-slides-[% SEARCH_RESULT.biblionumber | html %]" class="cover-slides search_cover_images_required" data-biblionumber="[% SEARCH_RESULT.biblionumber | html %]">

This seems like it would be the place to add the SEARCH_RESULT data you would
need for your plugin.

@@ +782,5 @@
>      [% Asset.js("lib/jquery/plugins/humanmsg.js") | $raw %]
>      [% INCLUDE 'select2.inc' %]
> +    [% IF ( Cover_Images_Required ) %]
> +        <script>
> +            const search_results = {};

It seems strange to me to create a separate data structure when you already
have the HTML available.

-- 
You are receiving this mail because:
You are watching all bug changes.