An idea for consolidating staff interface header search forms
I'm playing around with the idea of eliminating all the many search form include files (serials-search.inc, cat-search.inc, checking-search.inc, etc.) and replacing them with a single template which can be processed in different ways. For instance, in the circulation-home.tt template, "[% INCLUDE 'circ-search.inc' %]" would be replaced with "[% PROCESS header_searches.inc searches="circulation_searches" %]" In header_searches.inc (see https://gitlab.com/snippets/1994424), an array of search forms and their labels would be output according to the "searches" variable passed to the include file for processing. Unfortunately this doesn't appear to be translatable, but Bug 20988 might make it possible. Does this sound like a worthwhile pursuit? A sensible way to leverage Template toolkit's functionality? Thanks, -- Owen -- Web Developer Athens County Public Libraries (740) 737-6006 https://www.myacpl.org
Hi Owen, I like the idea of centralizing this code a bit more. I am not sure if bug 20988 will fix the translation issue, but maybe we could work around it for the time being? Maybe instead of using [% form.label | html %] we could use the form.formid and translate that to labels. Then we'd also not have to repeat the label descriptions in the search definitions for the different use cases. Hope that makes sense, Katrin On 09.07.20 19:25, Owen Leonard wrote:
I'm playing around with the idea of eliminating all the many search form include files (serials-search.inc, cat-search.inc, checking-search.inc, etc.) and replacing them with a single template which can be processed in different ways.
For instance, in the circulation-home.tt template, "[% INCLUDE 'circ-search.inc' %]" would be replaced with "[% PROCESS header_searches.inc searches="circulation_searches" %]"
In header_searches.inc (see https://gitlab.com/snippets/1994424), an array of search forms and their labels would be output according to the "searches" variable passed to the include file for processing.
Unfortunately this doesn't appear to be translatable, but Bug 20988 might make it possible.
Does this sound like a worthwhile pursuit? A sensible way to leverage Template toolkit's functionality?
Thanks,
-- Owen
Hi, You don't need bug 20988, you can already use the t() macro to make strings translatable [% USE Koha %] [% PROCESS 'i18n.inc' %] [% catalog_searches = [ { formid => 'circ_search', label => t('Check out') }, { formid => 'checkin_search', label => t('Check in') }, { formid => 'renew_search', label => t('Renew') }, { formid => 'catalog_search', label => t('Search the catalog'), open => 1 }, ] %] Le 12/07/2020 à 14:19, Katrin Fischer a écrit :
Hi Owen,
I like the idea of centralizing this code a bit more.
I am not sure if bug 20988 will fix the translation issue, but maybe we could work around it for the time being?
Maybe instead of using [% form.label | html %] we could use the form.formid and translate that to labels. Then we'd also not have to repeat the label descriptions in the search definitions for the different use cases.
Hope that makes sense,
Katrin
On 09.07.20 19:25, Owen Leonard wrote:
I'm playing around with the idea of eliminating all the many search form include files (serials-search.inc, cat-search.inc, checking-search.inc, etc.) and replacing them with a single template which can be processed in different ways.
For instance, in the circulation-home.tt template, "[% INCLUDE 'circ-search.inc' %]" would be replaced with "[% PROCESS header_searches.inc searches="circulation_searches" %]"
In header_searches.inc (see https://gitlab.com/snippets/1994424), an array of search forms and their labels would be output according to the "searches" variable passed to the include file for processing.
Unfortunately this doesn't appear to be translatable, but Bug 20988 might make it possible.
Does this sound like a worthwhile pursuit? A sensible way to leverage Template toolkit's functionality?
Thanks,
-- Owen
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre
I've posted a work in progress here: https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-07-08-combine-header-se... Katrin I liked your idea about defining the labels, but I'm not sure I did exactly what you were suggesting. Even with the t() macro the labels are still not picked up for translation correctly. Translation issues aside do folks think this is worth pursuing? Thanks, Owen
Le 13/07/2020 à 16:36, Owen Leonard a écrit :
Even with the t() macro the labels are still not picked up for translation correctly.
What do you mean ? Translation works for me on your branch : https://pic.infini.fr/nqc3mEkM/mvaUg6k2.png -- Julian Maurice BibLibre
Hi Owen, this looks good to me - why do you think it might not be worth pursuing? Katrin On 13.07.20 16:36, Owen Leonard wrote:
I've posted a work in progress here:
https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-07-08-combine-header-se...
Katrin I liked your idea about defining the labels, but I'm not sure I did exactly what you were suggesting. Even with the t() macro the labels are still not picked up for translation correctly.
Translation issues aside do folks think this is worth pursuing?
Thanks,
Owen _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
this looks good to me - why do you think it might not be worth pursuing?
No particular reason, I just wanted to get some opinions in case there was a downside I wasn't seeing. Also: Julian is right, the t() macro works perfectly, I was just looking at the wrong po file! Thanks, Owen -- Web Developer Athens County Public Libraries (740) 737-6006 https://www.myacpl.org
I'm personally not a big fan of the "single big file" approach. I find it difficult to navigate through, and can cause git conflicts more often. Also, it means that all blocks need to be parsed by TT on every page. Maybe it doesn't affect performance too much (or maybe it affects performance in a good way), but it makes more sense to me to only include what's needed. I'd try to use WRAPPER + PROCESS directives to achieve the same thing: [%# acq-searches.inc %] [% WRAPPER 'header.inc' %] [% PROCESS 'vendor-search.inc' %] [% PROCESS 'orders-search.inc' %] [% END %] [%# header.inc %] [% search_tabs = [] %] <header> <div id="header_search"> [% content %] </div> <ul> [% FOREACH tab IN search_tabs %] <li>[% tab %]</li> [% END %] </ul> </header> [%# vendor-search.inc %] <form>...</form> [% search_tabs.push('Search vendors') %] Not sure if it works or if it is better than your solution, I just thought about that while reading your code. It probably has downsides too. Le 14/07/2020 à 02:58, Owen Leonard a écrit :
this looks good to me - why do you think it might not be worth pursuing? No particular reason, I just wanted to get some opinions in case there was a downside I wasn't seeing.
-- Julian Maurice BibLibre
Yeah, I'm not a fan of a single big file either. Owen, what's your motivation for the change? As the front-end person, I'm thinking maybe we should trust you on this one. As for numbers... It looks like that 1 file is 764 lines long (https://gitlab.com/koha-dev/koha-dev/-/commit/18831f86eabbb16aeccacc3e4cec54...). According to Gitlab, the stats are +1308 -1595. So it doesn't look like that much reduction of text. But it looks like maybe 21 files have been replaced by 1? Maybe that has removed unnecessary duplication. Having 1 file would probably mean more git merge conflicts, but this also isn't an area that should change very often, so having it centralized could make those infrequent/rare changes easier to make all at 1 time. In terms of performance, if a template cache is used, I think the header_searches.inc file should only need to be parsed 1 time. But that .ttc cache file would need to be read from the disk for each request and stored in memory. "/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt" has 721 lines and has a cached template of "/var/cache/koha/INSTANCE/templates/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt.ttc" which is 102KB. "/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/includes/circ-search.inc" has 43 lines and a cached template of /var/cache/koha/INSTANCE/templates/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/includes/circ-search.inc.ttc which is 4.6KB. But without actual response time metrics, those numbers aren't very helpful. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Online: 02 8005 0595 -----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Julian Maurice Sent: Wednesday, 15 July 2020 6:46 PM To: koha-devel@lists.koha-community.org Subject: Re: [Koha-devel] An idea for consolidating staff interface header search forms I'm personally not a big fan of the "single big file" approach. I find it difficult to navigate through, and can cause git conflicts more often. Also, it means that all blocks need to be parsed by TT on every page. Maybe it doesn't affect performance too much (or maybe it affects performance in a good way), but it makes more sense to me to only include what's needed. I'd try to use WRAPPER + PROCESS directives to achieve the same thing: [%# acq-searches.inc %] [% WRAPPER 'header.inc' %] [% PROCESS 'vendor-search.inc' %] [% PROCESS 'orders-search.inc' %] [% END %] [%# header.inc %] [% search_tabs = [] %] <header> <div id="header_search"> [% content %] </div> <ul> [% FOREACH tab IN search_tabs %] <li>[% tab %]</li> [% END %] </ul> </header> [%# vendor-search.inc %] <form>...</form> [% search_tabs.push('Search vendors') %] Not sure if it works or if it is better than your solution, I just thought about that while reading your code. It probably has downsides too. Le 14/07/2020 à 02:58, Owen Leonard a écrit :
this looks good to me - why do you think it might not be worth pursuing? No particular reason, I just wanted to get some opinions in case there was a downside I wasn't seeing.
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
On Wed, Jul 15, 2020 at 10:41 PM <dcook@prosentient.com.au> wrote:
Yeah, I'm not a fan of a single big file either.
...
It looks like that 1 file is 764 lines long
For comparison, columns_settings.yml is currently 1434 lines long. -- Owen -- Web Developer Athens County Public Libraries (740) 737-6006 https://www.myacpl.org
I don't like columns_settings.yml either heh, although that's also a bit of a different case, since it's just a YAML serialization of 1 large data structure, which would be easy to validate. In this case, we'd be having 1 huge template file with much more complex syntax. But... may as well go ahead and do it. I can't think of any reasons not to that would be real blockers. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Online: 02 8005 0595 -----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Owen Leonard Sent: Friday, 7 August 2020 9:56 AM To: Koha Devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] An idea for consolidating staff interface header search forms On Wed, Jul 15, 2020 at 10:41 PM <dcook@prosentient.com.au> wrote:
Yeah, I'm not a fan of a single big file either.
...
It looks like that 1 file is 764 lines long
For comparison, columns_settings.yml is currently 1434 lines long. -- Owen -- Web Developer Athens County Public Libraries (740) 737-6006 https://www.myacpl.org _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
participants (4)
-
dcook＠prosentient.com.au -
Julian Maurice -
Katrin Fischer -
Owen Leonard