Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#24731 closed enhancement (duplicate)

Add a 'multiple' option to wp_dropdown_categories

Reported by: louisremi's profile louisremi Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Taxonomy Keywords: has-patch
Focuses: template Cc:

Description

wp_dropdown_categories has an alternative that allows to select multiple categories: wp_category_checklist. But sometimes it would be useful to create a simple <select multiple> using wp_dropdown_categories: there are a large number of JS libs and plugins that enhance multiple selects (see http://harvesthq.github.io/chosen/ or http://ivaynberg.github.io/select2/ ) and are not able to do the same with checklists.

By adding a few lines of code to wp_dropdown_categories, I have been able to add a 'multiple' option that adds this attribute to the select element and allows an array to be passed as the 'selected' option.

Attachments (1)

multiple-select.patch (2.8 KB) - added by louisremi 11 years ago.
add a 'multiple' option to wp_dropdown_categories and handle multiple selected options

Download all attachments as: .zip

Change History (4)

@louisremi
11 years ago

add a 'multiple' option to wp_dropdown_categories and handle multiple selected options

#1 @aaroncampbell
11 years ago

Hey louisremi, thanks for the patch! I took a look at it, but unfortunately it doesn't look ready to go in. Here are some notes on the code in the patch:

  • There are several problems with coding standards. I'll try to be more specific about most of them, but please take a look at the handbook page: http://make.wordpress.org/core/handbook/coding-standards/php/
  • There's an if and an if/else that have only one line of code in each and shouldn't have braces: http://make.wordpress.org/core/handbook/coding-standards/php/#brace-style
  • The comment inside the if that pertains to the else makes the code harder to read. Try rewording it to something like "Set multiple attribute or limit to only one selected value" and put it above the if instead.
  • I know it was already like that, but if we're touching the code we should probably move it to use our selected() helper function.

A patch like this should also have some unit tests associated with it to test usage both as single and multiple (and all the error conditions like passing multiple selected elements to single select dropdown). We also need at least one test that use a custom walker (for reasons I'm getting to).

The biggest issue though is backwards compatibility. The current patch changes it so the selected option is always an array. It accounts for this in core code (hopefully everywhere, but I haven't had a chance to thoroughly test) but it doesn't account for plugins and themes that have extended the Walker_Category class or built their own walker based off it. All that existing code (I know I have personally extended this walker many times) would be expecting the selected parameter to be a string not an array. We need to account for this, which probably means keeping selected as a string and only allowing it to be an array if multiple is also set.

#2 @nacin
10 years ago

  • Component changed from Template to Taxonomy
  • Focuses template added

#3 @wonderboymusic
10 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #16734.

Note: See TracTickets for help on using tickets.