Currently this theme applies the search-specific UI components only to the search form in the navigation bar. I propose a feature request to apply the Bootstrap UI components to the search forms on the search pages as well. On a related note, the search form in the navigation bar doesn't have a submit button. Of course the end user can press enter / return / etc. after entering keywords, however for usability and accessibility there should be a submit button on the search form. See the screenshot below for an example of how the current search elements are themed:

bootstrap-search-ui.jpg

CommentFileSizeAuthor
#20 bootstrap-search-ui-1852900-20.patch3.67 KBnatted
#15 bootstrap-search-ui-1852900-15.patch3.61 KBcpliakas
#7 bootstrap-search-ui-1852900-4.patch3.35 KBandregriffin
#3 bootstrap-search-ui-1852900-3.patch3.23 KBcpliakas
#2 bootstrap-search-ui-1852900-2.patch3.23 KBcpliakas
#1 bootstrap-search-ui-1852900-1.jpg49.22 KBcpliakas
bootstrap-search-ui.jpg59.41 KBcpliakas

Comments

cpliakas’s picture

cpliakas commented
Status: Active » Needs review
StatusFileSize
new bootstrap-search-ui-1852900-1.jpg49.22 KB

The attached patch makes the proposed change.

You will notice that the patch implements a hook_form_FORM_ID_alter() for the search form as opposed to using the custom _bootstrap_search_form() form constructor. This is done for two reasons. First, it allows us to apply the Bootstrap components across search forms and not just on an individual search form. Second, it adds some transparency by using common override methods so that developers can more easily modify the forms using common techniques. Otherwise they would have to discover that this theme implements a custom form constructor and implement hook_form__bootstrap_search_form_alter() to modify the form.

The screenshot below highlights the UI with the patch applied (and after a cache clear, of course).

bootstrap-search-ui-1852900-1.jpg

Thanks in advance for your consideration to this issue,
Chris

cpliakas’s picture

cpliakas commented
StatusFileSize
new bootstrap-search-ui-1852900-2.patch3.23 KB

And... forgot to attach the patch.

cpliakas’s picture

cpliakas commented
StatusFileSize
new bootstrap-search-ui-1852900-3.patch3.23 KB

Fixed some typos in the comments and modified the wording a bit.

frankbaele’s picture

frankbaele commented

I see no harm in this patch +1

andregriffin’s picture

andregriffin commented

#3: bootstrap-search-ui-1852900-3.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, bootstrap-search-ui-1852900-3.patch, failed testing.

andregriffin’s picture

andregriffin commented
Status: Needs work » Needs review
StatusFileSize
new bootstrap-search-ui-1852900-4.patch3.35 KB

New patch against latest dev.

Status: Needs review » Needs work

The last submitted patch, bootstrap-search-ui-1852900-4.patch, failed testing.

frankbaele’s picture

frankbaele commented

good idea but maybe should theme all the search forms in the same way, also the search block form. It think we should wait for

http://drupal.org/node/1460508 gets closed or done, before we start patching

cpliakas’s picture

cpliakas commented
Status: Needs work » Postponed

Makes sense to me. Marking as postponed pending #1460508: Don't use menu_tree_page_data for $main_menu and $secondary_menu.

frankbaele’s picture

frankbaele commented
Status: Postponed » Active

since all the recent big changes have been commited i'm going to reopen this one

cpliakas’s picture

cpliakas commented
Assigned: Unassigned » cpliakas

Sounds good. If you don't mind I can take this one and will propose a revised patch within the next day or two.

Thanks,
Chris

alifdalya’s picture

alifdalya commented
Status: Active » Needs review

#2: bootstrap-search-ui-1852900-2.patch queued for re-testing.

cpliakas’s picture

cpliakas commented
Status: Needs review » Needs work

The patch won't apply since the elements it depends on have been removed. Will have to rework the patch.

cpliakas’s picture

cpliakas commented
Status: Needs work » Needs review
StatusFileSize
new bootstrap-search-ui-1852900-15.patch3.61 KB

Not sure if I am overly-excited about the implementation, but the attached patch seems to work for both the search block form and regular search form.

cpliakas’s picture

cpliakas commented

In working on this issue, I discovered a bug at #1874508: Advanced search fieldset is broken for the core node search that I am pretty sure is not related to the work done here. Would love to get a second set of eyes confirming that, though.

frankbaele’s picture

frankbaele commented
Assigned: cpliakas » frankbaele

looking in too the patch

killah89’s picture

killah89 commented

Can't get the Patch too work. Please help me or Upload fresh files with this patch so that i only must replace the files. Drupal drop me a Error when i add the patch -

Parse error: syntax error, unexpected T_DOUBLE_ARROW in C:\xampp\htdocs\arti\sites\all\themes\bootstrap\template.php on line 31 -

I'll very happy if anyone help me.

Thanks and regards,
Marvin :)

natted’s picture

natted commented
Assigned: frankbaele » Unassigned

In testing the patch, I did have an issue where User search functionality broke. Content search works.

My Advanced Search fieldset did work though.

I'll run some more tests and see if I can tweak the patch.

natted’s picture

natted commented
StatusFileSize
new bootstrap-search-ui-1852900-20.patch3.67 KB

I've updated cpliakas patch, with a small adjustment to fix user search.

I've also tweaked it so that the search displays correctly in the top navbar when the search block is assigned to the Navigation block.

All appears to work from my end now.

natted’s picture

natted commented
Status: Needs review » Fixed

Ok, I've committed to dev.

Thanks cpliakas! Really appreciate the work on developing the patch.

cpliakas’s picture

cpliakas commented

Awesome! Thanks for your attention to this and committing the patch.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.