-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
'event.target.value' are 'undefined' of 'onChange(event)' for 'Checkbox', 'Radio', and 'Dropdown' #638
Comments
This is a known limitation. The Because of this, there are no native browser events available for certain callbacks. This is why all change events in Semantic-UI-React provide the event first (when available) but also a second argument which contains the data you need. You should never have to access the native browser event for most tasks. You can see examples of how to retrieve values from the second argument in the docs. Such as the Radio Group example. Going to close this issue as this is not a bug. Feel free to open another issue if you feel any components are missing helpful data in the second argument and we can add more callback data. |
@twang2218 - to add to Levi's response, in the @levithomason - for consistency, what do you think about returning handleDropdownChange (e, result) {
const { name, value } = result
// ...
}
handleInputChange (e) {
const { name, value } = e.target
// ...
} which isn't ideal. I guess this would actually be addressed if we decided to pass all props back to handlers. |
To keep things as simple and predictable as possible I think I'd like to push usage toward: handleDropdownChange (e, data) {
const { name, value } = data
// ...
}
handleInputChange (e, data) {
const { name, value } = data
// ...
} Where |
I'm almost wondering then if it makes sense to pass the data first. This way the second parameter is the sometimes available event. handleDropdownChange (props, e) {
const { name, value } = props
// ...
}
handleInputChange (props, e) {
const { name, value } = props
// ...
} Thoughts? |
The problem came from using Currently, I just wrap the As you suggested, I tried the second argument of Another problem is that, although |
To be clear, this is not our design. It is how React handles Forms: https://facebook.github.io/react/docs/forms.html. It is also worth noting that this is how HTML handles checkboxes vs other inputs as well: <input type='checkbox' value='terms' checked /> I think it is best to stick with the existing HTML and React conventions.
The |
@levithomason what is the status on 2 parameters for all input types for consistency? Right now Checkbox action returns a javascript object and Input action returns a DOM Node with properties. |
I've tried implementing these suggestions and I still cannot get "data" returned from a Form element in a react application. Additionally, the linked documentation above that @levithomason posted above gets redirected to the Introduction section for semantic UI. Can you link to current documentation for a solution? |
Details
Version:
"semantic-ui-react": "^0.54.2"
To reproduce the issue, here is the
onChange()
function:and use the following JSX for the testing:
Here is the sample code link for the test.
http://codepen.io/twang/pen/dpdowZ
Just open the Developer Tools, and check the Console, and then change each components' value.
For the
<Input>
, theevent.target.value
returns correct value of the<input ...>
, however, for the<Checkbox>
,<Radio>
and<Dropdown>
, theevent.target.value
areundefined
.The text was updated successfully, but these errors were encountered: