Set value using attribute only on initial option render
Tested that you can now type in the middle of a controlled input in Chrome which previously jumped your cursor to the end. Fixes #6445.
Ben Alpert committed
Apr 7, 2016 at 19:02 UTC
5cb4a620e7997d448f3eac73b9ccedd8b5e79237
5 files changed
+40
-29
src/renderers/dom/client/wrappers/ReactDOMOption.js
+10
@@ -12,6 +12,7 @@
12
'use strict';
13
14
var ReactChildren = require('ReactChildren');
15
+var ReactDOMComponentTree = require('ReactDOMComponentTree');
16
var ReactDOMSelect = require('ReactDOMSelect');
17
18
var warning = require('warning');
@@ -57,6 +58,15 @@ var ReactDOMOption = {
58
inst._wrapperState = {selected: selected};
59
},
60
61
+ postMountWrapper: function(inst) {
62
+ // value="" should make a value attribute (#6219)
63
+ var props = inst._currentElement.props;
64
+ if (props.value != null) {
65
+ var node = ReactDOMComponentTree.getNodeFromInstance(inst);
66
+ node.setAttribute('value', props.value);
67
+ }
68
+ },
69
+
70
getNativeProps: function(inst, props) {
71
var nativeProps = Object.assign({selected: undefined, children: undefined}, props);
72
src/renderers/dom/client/wrappers/__tests__/ReactDOMOption-test.js
+11
@@ -71,4 +71,15 @@ describe('ReactDOMOption', function() {
71
var node = ReactDOM.findDOMNode(stub);
72
expect(node.innerHTML).toBe('foobar');
73
});
74
+
75
+ it('should set attribute for empty value', function() {
76
+ var container = document.createElement('div');
77
+ var option = ReactDOM.render(<option value="" />, container);
78
+ expect(option.hasAttribute('value')).toBe(true);
79
+ expect(option.getAttribute('value')).toBe('');
80
+
81
+ ReactDOM.render(<option value="lava" />, container);
82
+ expect(option.hasAttribute('value')).toBe(true);
83
+ expect(option.getAttribute('value')).toBe('lava');
84
+ });
85
});
src/renderers/dom/shared/DOMPropertyOperations.js
+1
-3
@@ -149,10 +149,8 @@ var DOMPropertyOperations = {
149
var propName = propertyInfo.propertyName;
150
// Must explicitly cast values for HAS_SIDE_EFFECTS-properties to the
151
// property type before comparing; only `value` does and is string.
152
- // Must set `value` property if it is not null and not yet set.
152
if (!propertyInfo.hasSideEffects ||
154
- ('' + node[propName]) !== ('' + value) ||
155
- !node.hasAttribute(propertyInfo.attributeName)) {
153
+ ('' + node[propName]) !== ('' + value)) {
154
// Contrary to `setAttribute`, object properties are properly
155
// `toString`ed by IE8/9.
156
node[propName] = value;
src/renderers/dom/shared/ReactDOMComponent.js
+10
@@ -235,6 +235,11 @@ function putListener() {
235
);
236
}
237
238
+function optionPostMount() {
239
+ var inst = this;
240
+ ReactDOMOption.postMountWrapper(inst);
241
+}
242
+
243
// There are so many media events, it makes sense to just
244
// maintain a list rather than create a `trapBubbledEvent` for each
245
var mediaEvents = {
@@ -600,6 +605,11 @@ ReactDOMComponent.Mixin = {
605
);
606
}
607
break;
608
+ case 'option':
609
+ transaction.getReactMountReady().enqueue(
610
+ optionPostMount,
611
+ this
612
+ );
613
}
614
615
return mountImage;
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+8
-26
@@ -15,13 +15,11 @@
15
describe('ReactDOMComponent', function() {
16
var React;
17
var ReactDOM;
18
- var ReactDOMFeatureFlags;
18
var ReactDOMServer;
19
20
beforeEach(function() {
21
jest.resetModuleRegistry();
22
React = require('React');
24
- ReactDOMFeatureFlags = require('ReactDOMFeatureFlags')
23
ReactDOM = require('ReactDOM');
24
ReactDOMServer = require('ReactDOMServer');
25
});
@@ -503,30 +501,14 @@ describe('ReactDOMComponent', function() {
501
expect(nodeValueSetter.mock.calls.length).toBe(expected);
502
}
503
506
- if (ReactDOMFeatureFlags.useCreateElement) {
507
- renderWithValueAndExpect(undefined, 0);
508
- renderWithValueAndExpect('', 1);
509
- renderWithValueAndExpect('foo', 2);
510
- renderWithValueAndExpect('foo', 2);
511
- renderWithValueAndExpect(undefined, 3);
512
- renderWithValueAndExpect(null, 3);
513
- renderWithValueAndExpect('', 4);
514
- renderWithValueAndExpect(undefined, 4);
515
- } else {
516
- renderWithValueAndExpect(undefined, 0);
517
- // This differs because we will have created a node with the value
518
- // attribute set. This means it will hasAttribute, so we won't try to
519
- // set the value.
520
- renderWithValueAndExpect('', 0);
521
- renderWithValueAndExpect('foo', 1);
522
- renderWithValueAndExpect('foo', 1);
523
- renderWithValueAndExpect(undefined, 2);
524
- renderWithValueAndExpect(null, 2);
525
- // Again, much like the initial update case, we will always have the
526
- // attribute set so we won't set the value.
527
- renderWithValueAndExpect('', 2);
528
- renderWithValueAndExpect(undefined, 2);
529
- }
504
+ renderWithValueAndExpect(undefined, 0);
505
+ renderWithValueAndExpect('', 0);
506
+ renderWithValueAndExpect('foo', 1);
507
+ renderWithValueAndExpect('foo', 1);
508
+ renderWithValueAndExpect(undefined, 2);
509
+ renderWithValueAndExpect(null, 2);
510
+ renderWithValueAndExpect('', 2);
511
+ renderWithValueAndExpect(undefined, 2);
512
});
513
514
it('should not incur unnecessary DOM mutations for boolean properties', function() {