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

Support multi checkbox group #13

Closed
brunolemos opened this issue Aug 6, 2018 · 9 comments · Fixed by #28
Closed

Support multi checkbox group #13

brunolemos opened this issue Aug 6, 2018 · 9 comments · Fixed by #28

Comments

@brunolemos
Copy link

brunolemos commented Aug 6, 2018

<input type="checkbox" name="enabled_features[]" value="featureA" checked />Feature A<br />
<input type="checkbox" name="enabled_features[]" value="featureB" />Feature B<br />
<input type="checkbox" name="enabled_features[]" value="featureC" />Feature C<br />

Expected Result

{
  "enabledFeatures": ["featureA"]
}

or

{
  "enabledFeatures": {
    "featureA": true,
    "featureB": false,
    "featureC": false
  }
}

Current Result

{
  "enabledFeatures[]": true
}

This is common at least when I was in the php world a few years ago:
https://gist.github.com/southern-hu/5424068
http://www.wastedpotential.com/html-multi-checkbox-set-the-correct-way-to-group-checkboxes/

@fregante
Copy link
Owner

fregante commented Aug 8, 2018

That sounds like a good idea. PR welcome to implement it as an array:

{
  "enabledFeatures": ["featureA"]
}

@AndersDJohnson
Copy link

We could consider using something like form-serialize.

@fregante
Copy link
Owner

That's not a terrible idea, it would free webext-options-sync from doing any form parsing, however we still need an unserialize version to apply the data back to the form.

@AndersDJohnson
Copy link

AndersDJohnson commented May 25, 2019

@bfred-it Good point. Looks like there is a fork of it (form-serialization) that supports deserialization, though I found a more widely used one that supports both serialize and deserialize: dom-form-serializer.

@fregante
Copy link
Owner

Amazing, thanks! Could you send a PR for that?

@fregante fregante assigned fregante and unassigned fregante May 30, 2019
@fregante
Copy link
Owner

A build step is coming in #20 so it will be easier to integrate this.

@AndersDJohnson
Copy link

@bfred-it Thanks, let me know when that's in?

@fregante
Copy link
Owner

It's in!

@fregante
Copy link
Owner

Can you test #28 out with:

npm install webext-options-sync@next

You can then verify the stored data with chrome.storage.sync.get(console.log). You'll also see that default's aren't included (except the arrays/object that are now possible thanks to that module)

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