Summary: | Add hook to add a tab in intranet biblio details page | ||
---|---|---|---|
Product: | Koha | Reporter: | Matthias Meusburger <matthias.meusburger> |
Component: | Plugin architecture | Assignee: | Julian Maurice <julian.maurice> |
Status: | CLOSED FIXED | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Severity: | new feature | ||
Priority: | P5 - low | CC: | arthur.suzuki, julian.maurice, kyle, magnus, martin.renvoize, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This new feature allows plugin authors to add additional tabs to the intranet biblio details page. The new `intranet_catalog_biblio_tab` method which should return an array of `Koha::Plugins::Tab` objects is introduced.
**Warning**: Care should be taken when installing any plugins and only plugins you trust should be used.
|
Version(s) released in: |
19.11.00
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 27114 | ||
Attachments: |
Bug 23050: Plugin hook to add tabs in intranet biblio details page
Bug 23050: Plugin hook to add tabs in intranet biblio details page Bug 23050: Plugin hook to add tabs in intranet biblio details page Bug 23050: Add missing template filters Bug 23050: Fix tab's id to avoid compatibility issues Bug 23050: Fix creation of biblio tab's id Bug 23050: Plugin hook to add tabs in intranet biblio details page Bug 23050: Add missing template filters Bug 23050: Fix tab's id to avoid compatibility issues Bug 23050: Fix creation of biblio tab's id Bug 23050: (QA follow-up) Add Koha::Plugins::Tab class Bug 23050: (RM follow-up) Add Try::Tiny to controller |
Description
Matthias Meusburger
2019-06-05 12:30:58 UTC
Created attachment 90337 [details] [review] Bug 23050: Plugin hook to add tabs in intranet biblio details page Test Plan: 1) Enable plugins 2) Download and install the latest version of this plugin https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook 3) Browse to catalogue/detail.pl for a record 4) Note you see two new tabs with content Created attachment 90348 [details] [review] Bug 23050: Plugin hook to add tabs in intranet biblio details page Test Plan: 1) Enable plugins 2) Download and install the latest version of this plugin https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook 3) Browse to catalogue/detail.pl for a record 4) Note you see two new tabs with content Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Hi Julian, nice patch! 1) Some complaints form the QA script: FAIL koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt FAIL filters wrong_html_filter at line 173 ( <li><a href="#[% tab.id %]">[% tab.title %]</a></li>) missing_filter at line 173 ( <li><a href="#[% tab.id %]">[% tab.title %]</a></li>) missing_filter at line 173 ( <li><a href="#[% tab.id %]">[% tab.title %]</a></li>) 2) Wondering: What happens if the tab name consists non-latin characters, diacritics etc. Will this still work ok as id? + $tab->{id} = $tab->{title}; + $tab->{id} =~ s/[^\w]+/-/g; I've found this looking for valid characters in ids: he value must not contain any space characters. HTML 4: ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods ("."). 3) Can you please document the new hooks on the wiki (once we have this in PQA)? https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks Created attachment 91523 [details] [review] Bug 23050: Plugin hook to add tabs in intranet biblio details page Test Plan: 1) Enable plugins 2) Download and install the latest version of this plugin https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook 3) Browse to catalogue/detail.pl for a record 4) Note you see two new tabs with content Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Created attachment 91524 [details] [review] Bug 23050: Add missing template filters Created attachment 91525 [details] [review] Bug 23050: Fix tab's id to avoid compatibility issues MDN says: Using characters except ASCII letters, digits, '_', '-' and '.' may cause compatibility problems, as they weren't allowed in HTML 4. Though this restriction has been lifted in HTML5, an ID should start with a letter for compatibility. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id All issues mentioned in comment 3 should be fixed. I also modified the https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks page to include the bug number and the first Koha version that includes the hooks. Julian, I think this is a pretty cool idea. I don't like it that you generate content on the .pl (tab id, title, etc). It feels like all that could be done on the templates if we pass the plugins like: Why not just: my @tab_plugins = Koha::Plugins->new()->GetPlugins({ method => 'intranet_catalog_biblio_tab', }); $template->param( plugins => \@plugins, tab_plugins => \@tab_pluginss, ); Also, (for a later enhancement) there should be a way to define the order in which the tabs will be placed. Not sure how/where. (In reply to Tomás Cohen Arazi from comment #8) > Julian, I think this is a pretty cool idea. > I don't like it that you generate content on the .pl (tab id, title, etc). > It feels like all that could be done on the templates It could be done in templates, but, as we need the tab id in two different places, it means it would duplicate the code that creates that id, right ? Created attachment 93222 [details] [review] Bug 23050: Fix creation of biblio tab's id (Too much javascript in my head...) Created attachment 95004 [details] [review] Bug 23050: Plugin hook to add tabs in intranet biblio details page Test Plan: 1) Enable plugins 2) Download and install the latest version of this plugin https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook 3) Browse to catalogue/detail.pl for a record 4) Note you see two new tabs with content Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 95005 [details] [review] Bug 23050: Add missing template filters Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 95006 [details] [review] Bug 23050: Fix tab's id to avoid compatibility issues MDN says: Using characters except ASCII letters, digits, '_', '-' and '.' may cause compatibility problems, as they weren't allowed in HTML 4. Though this restriction has been lifted in HTML5, an ID should start with a letter for compatibility. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 95007 [details] [review] Bug 23050: Fix creation of biblio tab's id (Too much javascript in my head...) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 95013 [details] [review] Bug 23050: (QA follow-up) Add Koha::Plugins::Tab class This minimal class encapsulates the tabs to be passed around to the templates, so error checking on missing bits is done in a single place. It throws exceptions on errors Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Thx, Tomas! Nice work! Pushed to master for 19.11.00 Tomás, didn't you forgot to "use Try::Tiny;" in catalogue/detail.pl for your last patch? (attachment 95013 [details] [review]) Without the use statement, I get the following error when trying to display content: Can't locate object method "catch" via package "1" (perhaps you forgot to load "1"?) at /home/koha/src/catalogue/detail.pl line 89. With the use statement, it works properly. (In reply to Matthias Meusburger from comment #18) > Tomás, didn't you forgot to "use Try::Tiny;" in catalogue/detail.pl for your > last patch? (attachment 95013 [details] [review] [review]) > > Without the use statement, I get the following error when trying to display > content: > > Can't locate object method "catch" via package "1" (perhaps you forgot to > load "1"?) at /home/koha/src/catalogue/detail.pl line 89. > > With the use statement, it works properly. I did Created attachment 95406 [details] [review] Bug 23050: (RM follow-up) Add Try::Tiny to controller Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> |