From 509cabc421e70cb8a360765e952e8d004bc37fb0 Mon Sep 17 00:00:00 2001 From: Brandon Dail Date: Mon, 14 Aug 2017 12:49:32 -0500 Subject: [PATCH 1/4] Default to first non-disabled option for select Instead of defaulting to the first option for a select element, which could be a disabled option, we find the first non-disabled option available while we looping through the options. If no option matches, set the first non-disabled option as selected. --- src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js index 6a5599b70722..e31b2b5b4893 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js @@ -101,14 +101,18 @@ function updateOptions( // Do not set `select.value` as exact behavior isn't consistent across all // browsers for all cases. let selectedValue = '' + (propValue: string); + let defaultSelected = null; for (let i = 0; i < options.length; i++) { if (options[i].value === selectedValue) { options[i].selected = true; return; } + if (defaultSelected === null && !options[i].disabled) { + defaultSelected = options[i]; + } } - if (options.length) { - options[0].selected = true; + if (defaultSelected !== null) { + defaultSelected.selected = true; } } } From fb99fbe9e94d1f2b9085aaf45b597c6e0c82dfd6 Mon Sep 17 00:00:00 2001 From: Brandon Dail Date: Mon, 14 Aug 2017 13:00:35 -0500 Subject: [PATCH 2/4] Add ReactDOMSelect test for defaulting to non-disabled options --- .../wrappers/__tests__/ReactDOMSelect-test.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js index 95c547b7b9e8..b4dfa586fb7b 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js @@ -128,6 +128,22 @@ describe('ReactDOMSelect', () => { expect(node.value).toEqual('gorilla'); }); + it('should default to the first non-disabled option', () => { + var stub = ( + + ); + var container = document.createElement('div'); + stub = ReactDOM.render(stub, container); + var node = ReactDOM.findDOMNode(stub); + expect(node.options[0].selected).toBe(false); + expect(node.options[2].selected).toBe(true); + }); + it('should allow setting `value` to __proto__', () => { var stub = ( - - - - - - Value: {this.state.value} - -
- Uncontrolled - - -
-
- Controlled in nested subtree -
(this._nestedDOMNode = node)} /> - - This should synchronize in both direction with the one above. - -
- + +
+
+ Controlled + + Value: {this.state.value} +
+
+ Uncontrolled + +
+
+ + + +
  • Open the select
    • +
  • Select "1"
    • +
  • Attempt to reselect "Please select an item"
    • +     + + + The initial picked option should be "Please select an + item", however it should not be a selectable option. + + +
    + +
    +     + + + + The initial picked option value should "0": the first non-disabled option. + + +
    + +
    +     +     ); } } diff --git a/fixtures/dom/src/style.css b/fixtures/dom/src/style.css index cb38d9e0fffa..28e01b611b2f 100644 --- a/fixtures/dom/src/style.css +++ b/fixtures/dom/src/style.css @@ -8,23 +8,20 @@ html { font-size: 10px; } body { - font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif; + font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", + "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", + sans-serif; font-size: 1.4rem; margin: 0; padding: 0; } -select { - width: 12rem; -} - button { margin: 10px; font-size: 18px; padding: 5px; } - .header { background: #222; box-shadow: inset 0 -1px 3px #000; @@ -34,6 +31,10 @@ button { padding: .8rem 1.6rem; } +.header select { + width: 12rem; +} + .header__inner { display: table; margin: 0 auto; @@ -101,7 +102,8 @@ fieldset { overflow: hidden; } -ul, ol { +ul, +ol { margin: 0 0 2rem 0; } @@ -212,3 +214,7 @@ li { background-color: #f4f4f4; border-top: 1px solid #d9d9d9; } + +.field-group { + overflow: hidden; +} From 7e94da163d82172ce2ea6d900f8a44324ec1b729 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Sep 2017 13:32:47 -0700 Subject: [PATCH 4/4] Fix bad merge --- fixtures/dom/src/components/TestCase.js | 4 ++-- .../dom/src/components/fixtures/selects/index.js | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/fixtures/dom/src/components/TestCase.js b/fixtures/dom/src/components/TestCase.js index 11bd2b39eecb..394e61578aa1 100644 --- a/fixtures/dom/src/components/TestCase.js +++ b/fixtures/dom/src/components/TestCase.js @@ -1,5 +1,3 @@ -const React = window.React; - import cn from 'classnames'; import semver from 'semver'; import PropTypes from 'prop-types'; @@ -7,6 +5,8 @@ import IssueList from './IssueList'; import {parse} from 'query-string'; import {semverString} from './propTypes'; +const React = window.React; + const propTypes = { children: PropTypes.node.isRequired, title: PropTypes.node.isRequired, diff --git a/fixtures/dom/src/components/fixtures/selects/index.js b/fixtures/dom/src/components/fixtures/selects/index.js index e4378dd4b7ca..a0b34470bfb9 100644 --- a/fixtures/dom/src/components/fixtures/selects/index.js +++ b/fixtures/dom/src/components/fixtures/selects/index.js @@ -1,9 +1,9 @@ -const React = window.React; -const ReactDOM = window.ReactDOM; - import FixtureSet from '../../FixtureSet'; import TestCase from '../../TestCase'; +const React = window.React; +const ReactDOM = window.ReactDOM; + class SelectFixture extends React.Component { state = {value: ''}; _nestedDOMNode = null; @@ -55,6 +55,13 @@ class SelectFixture extends React.Component { +
    + Controlled in nested subtree +
    (this._nestedDOMNode = node)} /> + + This should synchronize in both direction with the "Controlled". + +