Collections - support for sets and symbols #885

Merged
merged 8 commits into from Apr 1, 2014

Conversation

Projects
None yet
3 participants
@dwbutler
Contributor

dwbutler commented Sep 4, 2012

I'm using the role_model gem to model roles in my application. RoleModel exposes a 'roles' attribute which is implemented as a set of symbols. I tried creating a checkboxes input, but Formtastic just ignored the selected values.

I modified Formtastic input collections to accept sets (or any array-like object that responds to :to_a). I also modified it to work with symbols.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Sep 4, 2012

This pull request fails (merged a93299a into 4bd0330).

This pull request fails (merged a93299a into 4bd0330).

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Nov 8, 2012

Contributor

Hi, just wondering if you've had a chance to look at this. Thanks!

Contributor

dwbutler commented Nov 8, 2012

Hi, just wondering if you've had a chance to look at this. Thanks!

@justinfrench

This comment has been minimized.

Show comment Hide comment
@justinfrench

justinfrench Nov 10, 2012

Owner

@dwbutler looks okay, will need some documentation!

Owner

justinfrench commented Nov 10, 2012

@dwbutler looks okay, will need some documentation!

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Nov 12, 2012

Contributor

Sure, no problem! I don't see an obvious place to add documentation though. Should I just add some comments to the modified code?

Contributor

dwbutler commented Nov 12, 2012

Sure, no problem! I don't see an obvious place to add documentation though. Should I just add some comments to the modified code?

@justinfrench

This comment has been minimized.

Show comment Hide comment
@justinfrench

justinfrench Nov 12, 2012

Owner

@dwbutler I think something would need to go in the examples at the top of CheckBoxesInput and anything else that mixes in Collections, not ideal, but shouldn't take long :)

Owner

justinfrench commented Nov 12, 2012

@dwbutler I think something would need to go in the examples at the top of CheckBoxesInput and anything else that mixes in Collections, not ideal, but shouldn't take long :)

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Jan 1, 2013

Contributor

I took a look at this again and realized I didn't actually test sets and symbols with the other collection input types. I'll have some more thorough testing and documentation ready to go later this week.

Contributor

dwbutler commented Jan 1, 2013

I took a look at this again and realized I didn't actually test sets and symbols with the other collection input types. I'll have some more thorough testing and documentation ready to go later this week.

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Jan 2, 2013

Contributor

Okay, I wrote some tests and examples for the three collection classes I could find - check box input, radio input, and select input. I think this should be good to go now.

Contributor

dwbutler commented Jan 2, 2013

Okay, I wrote some tests and examples for the three collection classes I could find - check box input, radio input, and select input. I think this should be good to go now.

@justinfrench

This comment has been minimized.

Show comment Hide comment
@justinfrench

justinfrench Jan 2, 2013

Owner

@dwbutler CI fails for 1.8.7 and ree, so I guess you're using some 1.9-only syntax? Haven't looked closely yet, ping me when you think the build is good!

Owner

justinfrench commented Jan 2, 2013

@dwbutler CI fails for 1.8.7 and ree, so I guess you're using some 1.9-only syntax? Haven't looked closely yet, ping me when you think the build is good!

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Jan 3, 2013

Contributor

Nope, everything is passing for me in 1.8.7. Looks like an unhappy coincidence that Travis happened to fail in 1.8.7 and ree. (It failed while trying to run bundle install.)

Contributor

dwbutler commented Jan 3, 2013

Nope, everything is passing for me in 1.8.7. Looks like an unhappy coincidence that Travis happened to fail in 1.8.7 and ree. (It failed while trying to run bundle install.)

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Jan 12, 2013

Contributor

Hey, just wondering if you had a chance to run the specs for this. I'm pretty sure the Travis CI results were a fluke. Here's a fairly clean Travis run (which had a misfire in 1.9.3 / Rails 4 this time): https://travis-ci.org/dwbutler/formtastic/builds/4104685

Contributor

dwbutler commented Jan 12, 2013

Hey, just wondering if you had a chance to run the specs for this. I'm pretty sure the Travis CI results were a fluke. Here's a fairly clean Travis run (which had a misfire in 1.9.3 / Rails 4 this time): https://travis-ci.org/dwbutler/formtastic/builds/4104685

@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Aug 28, 2013

Contributor

Bump! :)

I know it's been quite a while, but I merged in master yesterday and the Travis build passes. Would you mind giving this another look?

Thanks!

Contributor

dwbutler commented Aug 28, 2013

Bump! :)

I know it's been quite a while, but I merged in master yesterday and the Travis build passes. Would you mind giving this another look?

Thanks!

@justinfrench

This comment has been minimized.

Show comment Hide comment
@justinfrench

justinfrench Apr 1, 2014

Owner

Very late merge. Thanks!

Owner

justinfrench commented Apr 1, 2014

Very late merge. Thanks!

justinfrench added a commit that referenced this pull request Apr 1, 2014

Merge pull request #885 from dwbutler/master
Add support for Sets and Symbols as collections for collections (check box, radio and select inputs).

@justinfrench justinfrench merged commit 3aecd12 into justinfrench:master Apr 1, 2014

1 check passed

default The Travis CI build passed
Details
@dwbutler

This comment has been minimized.

Show comment Hide comment
@dwbutler

dwbutler Apr 1, 2014

Contributor

Nice! Thanks!

On Apr 1, 2014, at 3:01 AM, Justin French notifications@github.com wrote:

Merged #885.


Reply to this email directly or view it on GitHub.

Contributor

dwbutler commented Apr 1, 2014

Nice! Thanks!

On Apr 1, 2014, at 3:01 AM, Justin French notifications@github.com wrote:

Merged #885.


Reply to this email directly or view it on GitHub.

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