Re: [Koha-devel] An idea for consolidating staff interface header search forms
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 On Behalf Of Owen Leonard Sent: Friday, 7 August 2020 9:56 AM To: Koha Devel Subject: Re: [Koha-devel] An idea for consolidating staff interface header search forms On Wed, Jul 15, 2020 at 10:41 PM 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/ signature.asc Description: PGP signature ___ 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
On Wed, Jul 15, 2020 at 10:41 PM 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
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/18831f86eabbb16aeccacc3e4cec54139b804f68).
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 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 = [] %]
[% content %]
[% FOREACH tab IN search_tabs %]
[% tab %]
[% END %]
[%# vendor-search.inc %]
...
[% 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/
signature.asc
Description: PGP signature
___
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/
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 = [] %]
[% content %]
[% FOREACH tab IN search_tabs %]
[% tab %]
[% END %]
[%# vendor-search.inc %]
...
[% 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
> 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 ___ 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
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-searches 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/ ___ 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
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 ___ 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
I've posted a work in progress here: https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-07-08-combine-header-searches 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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
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
___
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/
Re: [Koha-devel] An idea for consolidating staff interface header search forms
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/
[Koha-devel] 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 ___ 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/
