After looking at bug 34624, I thought we could abstract (thus reducing) a lot of code and, making it more maintainable.
Created attachment 155202 [details] [review] Bug 34707: New common/header-search-tabs.inc Proof of concept of new abstract header-search-tabs.inc to be called everywhere its needed
Created attachment 155203 [details] [review] Bug 34707: Apply *-search.inc abstraction to cities PROOF OF CONCEPT I've applied this idea to cities to start discussion around this.
I believe we can abstract this even further but, for now, test plan is to apply patches and play around cities. Check that the top search bar works as before, etc.
I am not sure, that's a lot of different variables to define. Also some inc files are included from different places, you are going to duplicate this new big block. Another approach (to test!) would be to define all the 'tabs' in a single include, then pass the idea from the tt? include would contain all those blocks (here is one): + { + id => 'city_search', + is_active => 1, + is_allowed => 1, + label => 'Search cities', #TODO:translate + icon => 'fa-building', + form_action => undef, + form_id => undef, + form_method => 'POST', + inputs => [ + { + id => 'city_name_filter', + name => 'city_name_filter', + type => 'text', + value => city_name_filter, + placeholder => 'City search', + class => 'head-searchbox form-control', + } + ] + }, and tt would simply include: [% INCLUDE header-search-tabs.inc tabs => ['city-search', 'circ_search', 'catalog_search'] %]
(In reply to Jonathan Druart from comment #4) > I am not sure, that's a lot of different variables to define. Also some inc > files are included from different places, you are going to duplicate this > new big block. > > Another approach (to test!) would be to define all the 'tabs' in a single > include, then pass the idea from the tt? > > include would contain all those blocks (here is one): > + { > + id => 'city_search', > + is_active => 1, > + is_allowed => 1, > + label => 'Search cities', #TODO:translate > + icon => 'fa-building', > + form_action => undef, > + form_id => undef, > + form_method => 'POST', > + inputs => [ > + { > + id => 'city_name_filter', > + name => 'city_name_filter', > + type => 'text', > + value => city_name_filter, > + placeholder => 'City search', > + class => 'head-searchbox form-control', > + } > + ] > + }, > > and tt would simply include: > [% INCLUDE header-search-tabs.inc tabs => ['city-search', 'circ_search', > 'catalog_search'] %] s/idea/ids