[drupal-devel] [bug] New themable function theme_search_form to unify search form and put logic at the correct level

[Steven]

Issue status update for 
http://drupal.org/node/27843
Post a follow up: 
http://drupal.org/project/comments/add/27843

 Project:      Drupal
 Version:      cvs
 Component:    search.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  robertDouglass
 Reported by:  robertDouglass
 Updated by:   Steven
 Status:       patch

This patch collides with another patch where search_form() is turned
into theme_search_form(). But search_form() is more complicated
(especially in light of my upcoming search changes [1]). So there is a
compact and a full-blown style to accomodate.


Still, typically search forms in the theme are designed specifically to
fit in, with a custom search button and whatnot. They are a very typical
theme feature: why not keep it simple and inside page.tpl.php? The
dependency problem is also hairy... theme_search_form() does not belong
in theme.inc at all IMO, especially because it will become quite
logic-heavy.


Possible solutions:
- Modify this patch to use theme_search_form_compact(), while
theme_search_form() is reserved for the full search form.
- Keep the built-in theme search in the themes and apply the other
patch. The only themable function would be for the full search form.
[1] http://www.acko.net/dumpx/searcholeet.png




Steven



Previous comments:
------------------------------------------------------------------------

Sat, 30 Jul 2005 12:03:02 +0000 : robertDouglass

Attachment: http://drupal.org/files/issues/make_search_themable.txt (3.99 KB)

The search form should be themable, but it shouldn't be left to the
theme('page') function to do this, as is the current implementation.
This patch adds a theme_search_form function to theme.inc (not search
module because that would cause a dependency) and removes the then
unneeded variables from the phptemplate.engine. Pushbutton and
Bluemarine are updated to take advantage of the new function.


In the current implementation, several variables are created needlessly
in phptemplate.engine (like the text for the button) and then pushed to
the template. This seems messy and this patch fixes that.


Remaining questions: should the function accept any parameters? I think
not, but other opinions are welcome.