Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkbox reader behavior is inconvenient for some possible uses. #85

Closed
thefotolander opened this issue Feb 25, 2015 · 5 comments
Closed
Labels

Comments

@thefotolander
Copy link

I've been using Syphon while learning Marionette and I'm absolutely loving it. Recently I started using it for a production setup to collect a post's data before saving it and one of the model fields is an array of category IDs.

I have something like this for that particular field:

<input type="checkbox" id="category_1" value=1 name="categories[]"><label for="category_1">cat1</label>
<input type="checkbox" id="category_2" value=2 name="categories[]"><label for="category_2">cat2</label>
<input type="checkbox" id="category_3" value=3 name="categories[]"><label for="category_3">cat3</label>

I saw the documentation for changing the behavior for checkbox value collecting and so I modified the input reader for checkboxes like so:

Backbone.Syphon.InputReaders.register('checkbox', function(el) {
  if (el.is(":checked")) {
    return el.val();
  }
});

But I've found this is still adding an undefined value when a checkbox is not checked; for example if I select just the last two categories I get something like this

// the data contains this
[ undefined, 2, 3]

which I'm forced to clean after Backbone.Syphon.serialize(this) with a quick loop.

It would be cool to have a way to get only the actual values and not the undefined ones, as in if the value returned is null or undefined, don't add it to the data, unless there are implementation or other reasons behind not doing this.

Anyway, thanks a lot for your time and awesome libs.

@rhubarbselleven
Copy link
Contributor

Thanks for the feedback @poifox !

PRs are welcome if you're keen, otherwise this will go on the queue

@thefotolander
Copy link
Author

Hi @rhubarbselleven, thanks for getting back to me on this.

I'll see if I can do the change myself but I don't promise anything, if I was able to do this, what branch should I merge into, minor?

@rhubarbselleven
Copy link
Contributor

Hi @poifox, yes minor would be fab

@thefotolander
Copy link
Author

Hi @rhubarbselleven

While rummaging through the source code I actually understood how Syphon.KeyAssignmentValidators works and ended up solving my issue by registering a validation function for my checkbox use case in my Marionette view as follows:

// Let's customize the checkbox recollection behavior first.

// This collects the value from the checkbox
Backbone.Syphon.InputReaders.register('checkbox', function(el) {
  return el.val();
});

// And this makes sure that the value gets collected only for checked inputs
Backbone.Syphon.KeyAssignmentValidators.register('checkbox', function($el, key, value) {
  if ($el.prop("checked")) {
    return true;
  }
  return false;
});

// [Marionette view here...]

It was a good experience to look at the source code and realize that all I needed was in there, I just needed to learn how to use it. Maybe this example could be used in the documentation for future reference?

Anyway, I think the issue can be closed now.

Cheers!

@rhubarbselleven
Copy link
Contributor

Thanks for the update @poifox !

I'll link this one to #24 as this is something I'd like to see resolved when we're ready to do a code breaking release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants