Support <optgroup> in selects [#291498]

Hi,

I've attached a patch which allows users to specify optgroup labels within the select component. Labels are specified in the following form:

optgroup_label|key|value

If you have optgroup's specified but are rendering as radios or checkboxes, the list is automatically flattened. I'm not sure if the label format is the best way to do it, so if you have any better ideas feel free to suggest them. Even if it takes a while for this to be rolled in, I'd like to have the format specified so I can implement it on a site. Otherwise, I'll roll another patch with updated documentation and Drupal 6 support.

Thanks!
--Andrew

Comment	File	Size	Author
#7	291498-select-optgroup.7.patch	3.31 KB	deviantintegral
#2	select-optgroup.patch	2.07 KB	deviantintegral
#1	select-optgroup.patch	1.83 KB	deviantintegral

Comments

Comment #1

deviantintegral commented 5 August 2008 at 17:22

Status	File	Size
new	select-optgroup.patch	1.83 KB

Comment #2

deviantintegral commented 5 August 2008 at 18:19

Status	File	Size
new	select-optgroup.patch	2.07 KB

I had a thought of a different way to do it; this is easier for users, but could break anyone who happens to use 'group' as a key:

To unset the grouping, to have selectable items at the root, you use.
group|

Any thoughts as two these two approaches would be appreciated.

Comment #3

quicksketch

he/him

commented 8 August 2008 at 00:37

This looks pretty good. I think I prefer the first approach more than the second, even though it results in more redundancy. Do you have a particular leniency towards one way or the other?

Comment #4

deviantintegral commented 8 August 2008 at 01:57

The main reason for the second version was being able to specify options without keys:

group|Group 1
option
option
group|Group 2

etc...

My main concern is making it as clear as possible for non-technical users, and the 2nd way seems to be simpler to explain, and visually matches what will be rendered.

--Andrew

Comment #5

quicksketch

he/him

commented 8 August 2008 at 03:19

Ah, I see that makes a lot of sense. Do you think we could possibly devise another format instead of group|Name for creating groups? It doesn't seem like the word "group" is really necessary at all. Maybe something like this?

<Group 1>
option
option
<Group 2>
etc

The <word> syntax is relatively common in Drupal for special meanings e.g. <front> or <hidden>. Any user wanting to actually include brackets would need to do something like this:

|<empty>
hidden|<hidden>
<front>|<front>

So all of those should render with the brackets in place, only a single line with brackets and no pipe would be considered a group. What do you think?

Comment #6

deviantintegral commented 8 August 2008 at 17:06

I like the brackets. I'll start hacking and will post a patch soon.

--Andrew

Comment #7

deviantintegral commented 8 August 2008 at 18:06

Status	File	Size
new	291498-select-optgroup.7.patch	3.31 KB

Here's a version which uses <Group> syntax. Unfortunately, the only place brackets are allowed is for group names; in every other place, they are filtered for XSS in _webform_filter_values(). All of the calls could be replaced with ones which don't filter for XSS, but filtering seems like a good idea to me where we can. I don't think anything is exposed by not filtering the group tag, and I wasn't able to get any suspicious situations as we watch for the closing bracket on the preg_match.

This also breaks the array flatten out into it's own function.

--Andrew

Comment #8

quicksketch

he/him

commented 8 August 2008 at 18:35

Ah, well that's nice in a way. That means no one is currently using brackets because they get stripped out anyway :)

I'll review when I get a minute. Thanks for your patches!

Comment #9

quicksketch

he/him

commented 21 October 2008 at 05:25

Status:

Needs review

» Fixed

I rerolled for the latest CVS version, tidied up a little bit by consolidating _webform_flatten_options() into _webform_select_options(). I added a SimpleTest for this new feature also. I opened #324028: Add help text for using Optgroups in Select list after noticing that there's absolutely no description on how to actually use this feature outside of the inline PHP code.