Skip to content

Multi-check field#140

Merged
mattheu merged 1 commit into
masterfrom
fix-groups-not-displayed
Dec 14, 2013
Merged

Multi-check field#140
mattheu merged 1 commit into
masterfrom
fix-groups-not-displayed

Conversation

@mattheu
Copy link
Copy Markdown
Member

@mattheu mattheu commented Dec 11, 2013

It would be useful if the check field supported multiple checks.

I tried using a group field but it didn't really work easily. I also tried creating one but it didn't save, so I went back to using a multichoice select field.

@mattheu
Copy link
Copy Markdown
Member

mattheu commented Dec 11, 2013

I think the correct way to do this would be to use a group. Something like this?

image

    array( 'id' => 'group',  'name' => 'Checkbox field Group', 'type' => 'group', 'fields' => array(
        array( 'id' => 'field-1',  'name' => 'Checkbox field', 'type' => 'checkbox', 'cols' => 4 ),
        array( 'id' => 'field-2',  'name' => 'Checkbox field', 'type' => 'checkbox', 'cols' => 4 ),
        array( 'id' => 'field-3',  'name' => 'Checkbox field', 'type' => 'checkbox', 'cols' => 4 ),
    ) ),

@mattheu
Copy link
Copy Markdown
Member

mattheu commented Dec 11, 2013

Turns out there is a bug with the group not displaying. This PR should fix it if you can try it for me.

The group field display method was not handling empty fields values in the same way as the standard display method.

@chrishoward-au
Copy link
Copy Markdown
Author

Thanks, Matthew, I'll give that a try tonight

Chris

@chrishoward-au
Copy link
Copy Markdown
Author

Thanks, Matthew. That seems to work much better now!

One thing you're missing though, group fields don't have a unique identifier (class or id), which therefore makes them impossible to uniquely target with jQuery or CSS unless you know the field number.

So I'd suggest changing:

<div class="field CMB_Group_Field">

to

<div id="my-group-field-id" class="field CMB_Group_Field">

Each checkbox is uniquely id'ed but that makes it more hassle to code - i can't just grab all the inputs as one object in jQuery.

Thanks

Chris

@mattheu
Copy link
Copy Markdown
Member

mattheu commented Dec 14, 2013

I think adding an ID attribute usuing the field ID is a good idea - but I'm going to merge this and open a new pull request for the ID

mattheu added a commit that referenced this pull request Dec 14, 2013
@mattheu mattheu merged commit 58be270 into master Dec 14, 2013
@mattheu mattheu deleted the fix-groups-not-displayed branch December 14, 2013 12:06
@chrishoward-au
Copy link
Copy Markdown
Author

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants