WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 20 months ago

Last modified 20 months ago

#23850 closed enhancement (fixed)

Searchform Format

Reported by: WraithKenny Owned by: SergeyBiryukov
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Widgets Keywords: has-patch commit needs-codex
Focuses: Cc:

Description

We should use a filter instead of an argument.

Attachments (11)

searchform_format.patch (908 bytes) - added by WraithKenny 2 years ago.
general-template.php.patch (896 bytes) - added by WraithKenny 2 years ago.
23850.diff (1.1 KB) - added by DrewAPicture 2 years ago.
23850.2.diff (630 bytes) - added by SergeyBiryukov 22 months ago.
23850.3.diff (1.2 KB) - added by SergeyBiryukov 22 months ago.
23850.4.patch (1.9 KB) - added by obenland 22 months ago.
23850.5.patch (1.9 KB) - added by obenland 22 months ago.
Like .4 but looking for 'search-form' support instead of 'search'
23850.4.diff (876 bytes) - added by nacin 21 months ago.
23850-remove-comments-arg.patch (637 bytes) - added by obenland 21 months ago.
23850-search-form-classes.patch (2.8 KB) - added by obenland 21 months ago.
Adds .search-field class and hyphenates other classes in html5 output
23850.5.diff (703 bytes) - added by obenland 20 months ago.

Download all attachments as: .zip

Change History (42)

comment:1 @WraithKenny2 years ago

The advantage of a filter is themes wouldn't have to add args to each get_search_form call, and places where core calls it would be affected (which seems desirable), like (search widget), and would obsolete #23848

comment:2 @WraithKenny2 years ago

  • Keywords has-patch added

comment:3 @lancewillett2 years ago

+1 -- cleaner and easier. In a theme, it'd be rare you'd want one search form call to be XHTML and another be HTML5.

comment:4 @SergeyBiryukov2 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.6

comment:5 @lancewillett2 years ago

@WraithKenny -- something's weird with your patch. Did not apply cleanly in my local WP install.

comment:6 @SergeyBiryukov2 years ago

  • Keywords needs-refresh added

PHPDoc needs to be updated to reflect the change.

comment:7 @SergeyBiryukov2 years ago

#23848 was marked as a duplicate.

comment:8 @lancewillett2 years ago

See also #23701 (.2 patch).

comment:9 @WraithKenny2 years ago

I created it from git unified diff, thought it'd be compatible... Making a new one, to include docs.

comment:10 follow-up: @WraithKenny2 years ago

New patch is good ol' fashioned svn patch. Just removed the @param doc (didn't need anything else, right?)

comment:11 @WraithKenny2 years ago

  • Keywords needs-refresh removed

@DrewAPicture2 years ago

comment:12 in reply to: ↑ 10 @DrewAPicture2 years ago

Replying to WraithKenny:

Just removed the @param doc (didn't need anything else, right?)

23850.diff adds the _deprecated_argument() bits.

Rather than removing the @param and argument, you replace it with the like-typed $deprecated argument for back-compat :)

Version 0, edited 2 years ago by DrewAPicture (next)

comment:13 @DrewAPicture2 years ago

  • Keywords needs-codex added

comment:14 follow-up: @WraithKenny2 years ago

The param was only there for 11 days, and we didn't hit beta with it. I don't think we should add the deprecated arg.

comment:15 in reply to: ↑ 14 @DrewAPicture2 years ago

Replying to WraithKenny:

The param was only there for 11 days, and we didn't hit beta with it. I don't think we should add the deprecated arg.

Ha, good call. Didn't realize the param was added in this cycle. By all means general-template.php.patch should do the trick.

comment:16 @SergeyBiryukov2 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 23798:

Use a filter instead of recently added $format argument in get_search_form(). props WraithKenny. fixes #23850. see #15081.

comment:17 follow-up: @lancewillett2 years ago

In 23799:

Twenty Thirteen: remove HTML5 argument for search form, using a filter instead. See #23850, closes #23701.

comment:18 in reply to: ↑ 17 @azizur22 months ago

Replying to lancewillett:

Can we update the PHPDocs to highlight that search_form_format filter is 3.6 feature.

Last edited 22 months ago by azizur (previous) (diff)

comment:19 @nacin22 months ago

I'm not sure this should be a filter. Or rather, while it should be a filter, this should also be theme support. There's no reason to force a theme to write a function just to return 'html5' when a single add_theme_support() function will do.

comment:20 @SergeyBiryukov22 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@SergeyBiryukov22 months ago

comment:21 @SergeyBiryukov22 months ago

23850.2.diff checks for html5-search-form theme feature.

comment:22 @SergeyBiryukov22 months ago

Created #24269 for Twenty Thirteen.

comment:23 @SergeyBiryukov22 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 24225:

Allow to switch get_search_form() to HTML5 with an add_theme_support() call. fixes #23850.

comment:24 @nacin22 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Thinking we should make a few more changes here.

@SergeyBiryukov22 months ago

comment:25 follow-up: @obenland22 months ago

I love the move to a simple 'html5' support call.

In .3.diff, can we make it 'comment-form' and 'search-form' maybe? We could use 'comments' or 'comment-list' for the html5 markup in wp_list_comments() then.

We also introduced html5 in the Quote format fallback, would it make sense to add a switch there, too? Happy to add a patch if you think we should, btw :)

comment:26 in reply to: ↑ 25 @lancewillett22 months ago

Replying to obenland:

We also introduced html5 in the Quote format fallback, would it make sense to add a switch there, too? Happy to add a patch if you think we should, btw :)

I'd vote no switch to 3.6+ features because a) we don't need back compat and b) we're pushing the web forward to modern markup.

@obenland22 months ago

@obenland22 months ago

Like .4 but looking for 'search-form' support instead of 'search'

comment:27 @nacin21 months ago

In 24417:

`add_theme_support( 'html5', array( 'comment-list', 'search-form', 'comment-form' ) );'

props obenland.
see #23850.

@nacin21 months ago

comment:28 @lancewillett21 months ago

In 24421:

Twenty Thirteen: Remove twentythirteen_search_form_format() filter in favor of add_theme_support() call for search form, comment form, and comment list markup. Props SergeyBiryukov, fixes #24269; see #23850.

@obenland21 months ago

Adds .search-field class and hyphenates other classes in html5 output

comment:29 @nacin20 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 24524:

Improve the HTML classes for the new html5 format in get_search_form().

Update Twenty Thirteen to reflect the changes.

props obenland.
fixes #23850.

@obenland20 months ago

comment:30 @lancewillett20 months ago

In 24536:

Twenty Thirteen: update RTL styles to match changes in r24524, props obenland. See #23850.

comment:31 @lancewillett20 months ago

In 24540:

Twenty Thirteen: update IE styles to match changes in r24524. See #23850.

Note: See TracTickets for help on using tickets.