[wp-trac] [WordPress Trac] #24731: Add a 'multiple' option to wp_dropdown_categories
WordPress Trac
noreply at wordpress.org
Wed Sep 18 15:58:18 UTC 2013
#24731: Add a 'multiple' option to wp_dropdown_categories
-------------------------+------------------------------
Reporter: louisremi | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: Awaiting Review
Component: Template | Version: 3.6
Severity: normal | Resolution:
Keywords: has-patch |
-------------------------+------------------------------
Comment (by aaroncampbell):
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.
--
Ticket URL: <http://core.trac.wordpress.org/ticket/24731#comment:1>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software
More information about the wp-trac
mailing list