Skip to content
This repository was archived by the owner on Jul 19, 2019. It is now read-only.

Commit 1a65d9d

Browse files
committed
Merge pull request #82 from spicyj/mc
Remove now-unnecessary mostRecentlyPlacedChild
2 parents bd3a51f + 820150f commit 1a65d9d

File tree

2 files changed

+48
-41
lines changed

2 files changed

+48
-41
lines changed

src/ReactART.js

Lines changed: 20 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ function createComponent(name) {
5151
this.listeners = null;
5252
this._mountImage = null;
5353
this._renderedChildren = null;
54-
this._mostRecentlyPlacedChild = null;
5554
};
5655
ReactARTComponent.displayName = name;
5756
for (var i = 1, l = arguments.length; i < l; i++) {
@@ -63,6 +62,22 @@ function createComponent(name) {
6362

6463
// ContainerMixin for components that can hold ART nodes
6564

65+
function injectAfter(parentNode, referenceNode, node) {
66+
var beforeNode;
67+
if (referenceNode == null) {
68+
// node is supposed to be first.
69+
beforeNode = parentNode.firstChild;
70+
} else {
71+
// node is supposed to be after referenceNode.
72+
beforeNode = referenceNode.nextSibling;
73+
}
74+
if (beforeNode) {
75+
node.injectBefore(beforeNode);
76+
} else {
77+
node.inject(parentNode);
78+
}
79+
}
80+
6681
var ContainerMixin = assign({}, ReactMultiChild.Mixin, {
6782

6883
/**
@@ -72,29 +87,9 @@ var ContainerMixin = assign({}, ReactMultiChild.Mixin, {
7287
* @param {number} toIndex Destination index of the element.
7388
* @protected
7489
*/
75-
moveChild: function(child, toIndex) {
90+
moveChild: function(child, afterNode, toIndex, lastIndex) {
7691
var childNode = child._mountImage;
77-
var mostRecentlyPlacedChild = this._mostRecentlyPlacedChild;
78-
if (mostRecentlyPlacedChild == null) {
79-
// I'm supposed to be first.
80-
if (childNode.previousSibling) {
81-
if (this.node.firstChild) {
82-
childNode.injectBefore(this.node.firstChild);
83-
} else {
84-
childNode.inject(this.node);
85-
}
86-
}
87-
} else {
88-
// I'm supposed to be after the previous one.
89-
if (mostRecentlyPlacedChild.nextSibling !== childNode) {
90-
if (mostRecentlyPlacedChild.nextSibling) {
91-
childNode.injectBefore(mostRecentlyPlacedChild.nextSibling);
92-
} else {
93-
childNode.inject(this.node);
94-
}
95-
}
96-
}
97-
this._mostRecentlyPlacedChild = childNode;
92+
injectAfter(this.node, afterNode, childNode);
9893
},
9994

10095
/**
@@ -107,23 +102,7 @@ var ContainerMixin = assign({}, ReactMultiChild.Mixin, {
107102
*/
108103
createChild: function(child, afterNode, childNode) {
109104
child._mountImage = childNode;
110-
var mostRecentlyPlacedChild = this._mostRecentlyPlacedChild;
111-
if (mostRecentlyPlacedChild == null) {
112-
// I'm supposed to be first.
113-
if (this.node.firstChild) {
114-
childNode.injectBefore(this.node.firstChild);
115-
} else {
116-
childNode.inject(this.node);
117-
}
118-
} else {
119-
// I'm supposed to be after the previous one.
120-
if (mostRecentlyPlacedChild.nextSibling) {
121-
childNode.injectBefore(mostRecentlyPlacedChild.nextSibling);
122-
} else {
123-
childNode.inject(this.node);
124-
}
125-
}
126-
this._mostRecentlyPlacedChild = childNode;
105+
injectAfter(this.node, afterNode, childNode);
127106
},
128107

129108
/**
@@ -154,7 +133,6 @@ var ContainerMixin = assign({}, ReactMultiChild.Mixin, {
154133
* @override {ReactMultiChild.Mixin.updateChildren}
155134
*/
156135
updateChildren: function(nextChildren, transaction, context) {
157-
this._mostRecentlyPlacedChild = null;
158136
this._updateChildren(nextChildren, transaction, context);
159137
},
160138

@@ -273,6 +251,7 @@ var NodeMixin = {
273251
},
274252

275253
getNativeNode: function() {
254+
return this.node;
276255
},
277256

278257
getPublicInstance: function() {

src/__tests__/ReactART-test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,34 @@ describe('ReactART', function() {
182182
testDOMNodeStructure(realNode, expectedNewStructure);
183183
});
184184

185+
it('should be able to reorder many components', function() {
186+
var container = document.createElement('div');
187+
188+
var Component = React.createClass({
189+
render: function() {
190+
var chars = this.props.chars.split('');
191+
return (
192+
<Surface>
193+
{chars.map((text) => <Shape key={text} title={text} />)}
194+
</Surface>
195+
);
196+
},
197+
});
198+
199+
// Mini multi-child stress test: lots of reorders, some adds, some removes.
200+
var before = 'abcdefghijklmnopqrst';
201+
var after = 'mxhpgwfralkeoivcstzy';
202+
203+
var instance = ReactDOM.render(<Component chars={before} />, container);
204+
var realNode = ReactDOM.findDOMNode(instance);
205+
expect(realNode.textContent).toBe(before);
206+
207+
var instance = ReactDOM.render(<Component chars={after} />, container);
208+
expect(realNode.textContent).toBe(after);
209+
210+
ReactDOM.unmountComponentAtNode(container);
211+
});
212+
185213
it('renders composite with lifecycle inside group', function() {
186214
var mounted = false;
187215
var CustomShape = React.createClass({

0 commit comments

Comments
 (0)