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

The selected attribute for <option> tags are not rendered correctly #5255

Closed
tnlogy opened this issue Oct 23, 2015 · 10 comments
Closed

The selected attribute for <option> tags are not rendered correctly #5255

tnlogy opened this issue Oct 23, 2015 · 10 comments

Comments

@tnlogy
Copy link

tnlogy commented Oct 23, 2015

This will cause Selenium tests to fail when using getFirstSelectedOption() to find the selected item with the Java API. This seems to affect at least 0.12 and forward. This example should set the selected attribute on the selected option-tag.

import React from 'react';
import ReactDOM from 'react-dom';

var App = React.createClass({
    getInitialState: function() {
        return {
            value: "a"
        };
    },
    onChange: function (event) {
        this.setState({value: event.target.value});
    },
    render: function () {
        return <div>
            <select value={this.state.value} onChange={this.onChange}>
                <option value="a">a</option>
                <option value="b">b</option>
            </select>
        </div>;
    }
});

ReactDOM.render(<App/>, document.getElementById("app"));
@tnlogy
Copy link
Author

tnlogy commented Oct 23, 2015

This is a naive change to use setAttribute instead, but I guess there are browser differences that might create problems, and also performance differences?

master...tnlogy:patch-1

@syranide
Copy link
Contributor

Correct behavior is to change the property as it does, not the attribute. The attribute only indicates the initially selected option. The attribute is also correctly set for the initially selected option. This sounds like a bug with getFirstSelectedOption unless I'm misunderstanding something.

@StJohn3D
Copy link

I'm having the same issue. On change will get the correct event information but the select element on the dom still reflects the initial value. Only on the second update (with the same selection - no change) will the select element reflect the true current selection.

The only way I was able to fix this was to use the selected property on <option> elements during my mapping function...
<option> selected={opVal === props.selectedVal} key={index}>{opVal}</option>

This of course gets me React warnings saying to use the value prop on <Select> but since that's not working I'm just living with the warnings.

Please fix this

@gaearon
Copy link
Collaborator

gaearon commented Apr 30, 2016

@StJohn3D Please try 15.0.2, it might contain the fix you need. If not, please create an example reproducing the issue.

@StJohn3D
Copy link

StJohn3D commented May 2, 2016

Made the upgrade and the behavior is the same.
I'm using a redux model, to emit changes to the store. Here is the component code.

import React, { Component, PropTypes } from 'react'
import { connect } from 'react-redux'
import { toolSelected } from '../../actions/ui-actions'

class Tool extends Component {
    handleToolChange(e) {
        const { dispatch, panelID } = this.props
        const selectedIndex = e.target.selectedIndex
        dispatch(toolSelected({
            panelID,
            selectedIndex
            })
        )
    }
    render() {
        const { tools, panels, panelID } = this.props
        if ( tools.length ) {
            const tool = tools[panels[panelID].selectedToolIndex]
            const value = tool.props.name || tool.type.niceName || tool.type.displayName
----------> const selector = <select value={value} onChange={this.handleToolChange.bind(this)}>
                {tools.map((tool, index) => {
                    const opValue = tool.props.name || tool.type.niceName || tool.type.displayName
------------------> return <option key={index}>{opValue}</option>
                })}
            </select>
            return (
                <tool.type toolSelector={selector} {...tool.props}/>
            )
        } else return null
    }
}

const mapStateToProps = state => ({
    tools : state.timberUI.tools,
    panels: state.timberUI.panels
})


export default connect(mapStateToProps)(Tool)

The behavior is that the correct tool (React component) will show up in the dom every time the change is made and the Redux store's state is accurate. But the select will still show the last selected tool as it's value until the Redux store is updated a second time. However this small change using selected in the render function solves the issue. Albeit with warnings suggesting to use value on select components instead.

Working render function using selected

...
render() {
        const { tools, panels, panelID } = this.props
        if ( tools.length ) {
            const tool = tools[panels[panelID].selectedToolIndex]
            const value = tool.props.name || tool.type.niceName || tool.type.displayName
----------> const selector = <select onChange={this.handleToolChange.bind(this)}>
                {tools.map((tool, index) => {
                    const opValue = tool.props.name || tool.type.niceName || tool.type.displayName
------------------> return <option selected={opValue === value} key={index}>{opValue}</option>
                })}
            </select>
            return (
                <tool.type toolSelector={selector} {...tool.props}/>
            )
        } else return null
    }
...

@gaearon
Copy link
Collaborator

gaearon commented May 2, 2016

Your first example is missing the value attribute on the option. From the documentation, this is the correct usage:

  <select value="B">
    <option value="A">Apple</option>
    <option value="B">Banana</option>
    <option value="C">Cranberry</option>
  </select>

Otherwise React thinks none of them have values, so it can’t match your select’s value to any of the options.

@gaearon
Copy link
Collaborator

gaearon commented May 3, 2016

Hmm, or maybe I’m wrong. Perhaps #5362 should fix it?

@StJohn3D
Copy link

StJohn3D commented May 3, 2016

It worked for me! I put value in both the select and the options and now it works without warnings. Many thanks! @gaearon

@gaearon
Copy link
Collaborator

gaearon commented May 3, 2016

This is still a bug probably so let’s keep it open until #5362 gets merged.

@aweary
Copy link
Contributor

aweary commented Sep 20, 2017

The original issue reported by @tnlogy is expected behavior since we're updating the property not the attribute. And it sounds like the other issues mentioned were resolved by #5362 so I'm going to close this out 👋

@aweary aweary closed this as completed Sep 20, 2017
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

No branches or pull requests

5 participants