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

Allow non-string values to be used as ownValue #26

Merged
merged 6 commits into from Mar 1, 2019
Merged

Conversation

wsmd
Copy link
Owner

@wsmd wsmd commented Feb 27, 2019

Fixes #14

<input {...input.checkbox('options', 'value')} />;
<input {...input.checkbox('options', true)} />;
<input {...input.checkbox('options', 123)} />;
<input {...input.checkbox('options', ['a', 'b'])} />;

{
  "values": {
    "options": [
      "value",
      "123",
      "a,b",
      "true"
    ]
  }
}
<input {...input.radio('option', '1')} />;
<input {...input.radio('option', 1)} />;
<input {...input.radio('option', ['1'])} />;

{
  "values": {
    "option": "1"
  }
}

@coveralls
Copy link

coveralls commented Feb 27, 2019

Pull Request Test Coverage Report for Build 135

  • 7 of 7 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 122: 0.0%
Covered Lines: 84
Relevant Lines: 84

💛 - Coveralls

@@ -105,7 +108,7 @@ export default function useFormState(initialState, options) {
* returning the value of input from the current form state is illogical
*/
if (isCheckbox || isRadio) {
return ownValue;
return toString(ownValue);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per spec:

the user agent must set the value of the element to the value of the value content attribute, if there is one, or the empty string otherwise

Therefore, if ownValue isn't specified by the user, it should return an empty string. Previously, it was returning undefined. This should not result in any breaking changes.

@wsmd wsmd force-pushed the stringify-own-values branch 2 times, most recently from a8ab950 to fcc3414 Compare March 1, 2019 13:06
@wsmd wsmd merged commit 72d981f into master Mar 1, 2019
@wsmd wsmd deleted the stringify-own-values branch March 1, 2019 13:14
@wsmd wsmd mentioned this pull request Mar 3, 2019
@wsmd wsmd added the enhancement Improve existing functionality label Mar 9, 2019
gcampes pushed a commit to gcampes/react-use-form-state that referenced this pull request Mar 18, 2019
* allow non-string values to be used as ownValue

* always stringify input value of checkbox and radio

* add test

* cleanup

* update README.md

* remove eslint-disable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants