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.

CommentFileSizeAuthor
make_search_themable.txt3.99 KBrobertDouglass
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Steven’s picture

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). 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.

Jose Reyero’s picture

I think this is not needed anymore, now we have that multiple regions for blocks and can just place a search block anywhere in the page. In fact, that search forms in themes should be replaced by standard blocks.

robertDouglass’s picture

Status: Needs review » Closed (won't fix)