Opened 9 years ago
Last modified 4 years ago
#34886 new enhancement
Search Form should not submit empty strings
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Themes | Keywords: | has-patch |
Focuses: | Cc: |
Description
Depending on the styling it is not always obvious to the user that there is a text box to click in before hitting the search button. On WordPress.com we are seeing about 3.5% of all site search queries are an empty string.
For the end user this costs them the extra page load, and the confusion of having the page loaded. By selecting the field instead we avoid the user changing pages, and needing to re-find the search box.
Similar problems are very prevalent in many themes. The most popular single word (that is not a stop word) in site search for WordPress.com is "search" due to the default value text being searched in themes. For most of the Twenty themes this problem is solved by not displaying the search button at all (display: none;). Fixing this in core should help point the way for fixing it in themes.
Twenty Ten does not have the search button hidden and so is a good theme for reproducing the problem.
Attachments (3)
Change History (17)
#2
follow-up:
↓ 7
@
9 years ago
@gibrown Good idea.
I think, however, that jQuery should be used for the new functionality, and that the JS should be loaded from a JS file instead of inlined to PHP.
Additionally, I think that the submit
event should be used instead, for example:
jQuery( function( $ ) { $( 'form.search-form' ).on( 'submit', function( e ) { var input = $( this ).find( 'input[name=s]' ); if ( ! input.val() ) { e.preventDefault(); input.focus(); } } ); } );
Alternatively, this same functionality can be implemented in HTML5 simply by adding a required
attribute to the s
input element:
-
src/wp-includes/general-template.php
diff --git src/wp-includes/general-template.php src/wp-includes/general-template.php index f554e1f..e65bd84 100644
function get_search_form( $echo = true ) { 226 226 $form = '<form role="search" method="get" class="search-form" action="' . esc_url( home_url( '/' ) ) . '"> 227 227 <label> 228 228 <span class="screen-reader-text">' . _x( 'Search for:', 'label' ) . '</span> 229 <input type="search" class="search-field" placeholder="' . esc_attr_x( 'Search …', 'placeholder' ) . '" value="' . get_search_query() . '" name="s" title="' . esc_attr_x( 'Search for:', 'label' ) . '" />229 <input type="search" class="search-field" required placeholder="' . esc_attr_x( 'Search …', 'placeholder' ) . '" value="' . get_search_query() . '" name="s" title="' . esc_attr_x( 'Search for:', 'label' ) . '" /> 230 230 </label> 231 231 <input type="submit" class="search-submit" value="'. esc_attr_x( 'Search', 'submit button' ) .'" /> 232 232 </form>';
#4
@
9 years ago
Thanks @westonruter. Will rework html5 version to use "require" (thanks, wasn't aware of that), and the JS to use jQuery on submit.
the JS should be loaded from a JS file instead of inlined to PHP
Seems like a performance impact to potentially load another JS file for such a small number of lines of code. Is there an existing file you would suggest adding this to? I think I heard mentioned that the only place where js is loaded is separately on the front end is for comments.
#5
@
9 years ago
@gibrown good question on the separate JS file. Given HTTP/2 and “server push”, there will (eventually) be zero performance hit to enqueue additional scripts. Moreover, there are other small scripts that get enqueued with a typical request, e.g. on Twenty Fifteen:
twentyfifteen/js/skip-link-focus-fix.js
twentyfifteen/js/functions.js
wp-includes/js/wp-embed.js
wp-includes/js/comment-reply.js
I can also envision that eventually any such search-form.js
could eventually include functionality for autocomplete and other search functionality.
Having a separate file allows it to be linted and QUnit-tested.
But I'll defer to others' ultimate recommendation on whether this should be in a separate JS file.
#6
@
9 years ago
I think a whole new JS file is too heavy handed for this - I'd be +1 on adding the required
attribute for those who support it.
#7
in reply to:
↑ 2
@
9 years ago
+1 on required
, of recent browser it only misses the Safari family.
Replying to westonruter:
I think, however, that jQuery should be used for the new functionality, and that the JS should be loaded from a JS file instead of inlined to PHP.
If a JavaScript approach is taken, a feature check for querySelector
and addEventListener
would be preferable to introducing a jQuery requirement for the front end. For similar reasons as in comment:6, loading jQuery and jQuery migrate is too heavy handed.
Uglifying and inlining during the build has worked well to allow unit tests of the emoji and embed JS.
#8
@
9 years ago
- Component changed from General to Themes
- Keywords has-patch added
- Version trunk deleted
A separate JS file just for this is definitely overkill. It's handling an edge case.
Switching to the submit
event on the search input is a good idea.
#11
@
6 years ago
Hi,
since the last comment was 3 years ago and since WP has dropped support for IE <9
and IE10 supports the required attribute, what is problematic about just adding the required attribute?
Relatedly, this was mentioned in one of the search box tickets for Gutenberg.
#12
@
5 years ago
I began a pull request to remediate this in Gutenberg's search box.
As I mentioned earlier, could this be now addressed by adding the
required
attribute?
#13
@
5 years ago
My Pull request to add 'required' for the search block in Gutenberg was tested by myself and a couple other contributors for browser and screen reader compliance, and was merged into Gutenberg.
If WordPress is only supporting HTML5 (is it?), then adding 'required' would be sufficient.
@
4 years ago
Updated patch adding 'required' parameter to the html5 search form in 'get_search_form()'
The patch was also tested to verify that an existing onclick jQuery event that has been attached to the search button will still fire with: