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

Revisit Checkbox Fields #22

Closed
dillonredding opened this issue Apr 12, 2023 · 3 comments · Fixed by #25
Closed

Revisit Checkbox Fields #22

dillonredding opened this issue Apr 12, 2023 · 3 comments · Fixed by #25

Comments

@dillonredding
Copy link
Member

Currently, checkbox fields work similar to HTML.

{
  "type": "checkbox",
  "name": "foo",
  "value": "bar",
  "checked": false
}

In the above Field example, foo=bar will only be sent to the server if checked is truthy. Otherwise, foo is not sent at all (see here). However, this requires clients to understand and utilize the checked extension for submission. I think we should abandon the need for the extension and send the field's value regardless. The checkedness can be tracked elsewhere and used to update the field's value (e.g., to true/false).

Basically, checkbox fields should have the type Field<boolean>.

@SeeSharpCode
Copy link

Am I understanding correctly that the field would look like this:

{
  "type": "checkbox",
  "name": "foo",
  "value": false
}

And value=false would be sent to the server?

@dillonredding
Copy link
Member Author

The main difference for this library would be removing the field.checked condition in the default field serializer. Instead of including {field.name}={field.value} or nothing as with HTML, the former would always be included and it would be up to the client what the field value would be when the checkbox is checked vs unchecked. Since it's a checkbox, a boolean probably makes the most sense, but it could technically be any values that make sense in the context of the API.

@SeeSharpCode
Copy link

Sounds good. I'm in favor of this change since it will make checkbox field behavior consistent with other field types.

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 a pull request may close this issue.

2 participants