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

Fix issues #474 and #410 - add new items in correct order #640

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
11 changes: 11 additions & 0 deletions src/Transition.js
Expand Up @@ -364,6 +364,7 @@ class Transition extends React.Component {
onExiting: _onExiting,
onExited: _onExited,
nodeRef: _nodeRef,
appendOnReplace: _appendOnReplace,
...childProps
} = this.props

Expand Down Expand Up @@ -561,6 +562,16 @@ Transition.propTypes = {
* @type Function(node: HtmlElement) -> void
*/
onExited: PropTypes.func,

/**
* Hint to tell whether the node should be placed before or after the pending items it replaces.
* Everything with appendOnReplace set to true will be placed after the removed keys,
* while everything else goes before the removed keys.
*
* **Note**: applies when replacing one or more consecutive existing items with one or more consecutive new items.
*
*/
appendOnReplace: PropTypes.bool,
}

// Name the function so it is clearer in the documentation
Expand Down
94 changes: 81 additions & 13 deletions src/utils/ChildMapping.js
Expand Up @@ -18,6 +18,22 @@ export function getChildMapping(children, mapFn) {
return result
}

/**
* Given `this.props.children`, return an object mapping key to child's appendOnReplace prop.
*
* @param {*} children `this.props.children`
* @return {object} Mapping of key to placement hint
*/
function getChildHintMapping(children) {
let result = Object.create(null)
if (children)
Children.map(children, c => c).forEach(child => {
// run the map function here instead so that the key is the computed one
if (isValidElement(child)) result[child.key] = child.props && child.props.appendOnReplace
})
return result
}

/**
* When you're adding or removing children some may be added or removed in the
* same render pass. We want to show *both* since we want to simultaneously
Expand All @@ -32,12 +48,14 @@ export function getChildMapping(children, mapFn) {
* `ReactTransitionChildMapping.getChildMapping()`.
* @param {object} next next children as returned from
* `ReactTransitionChildMapping.getChildMapping()`.
* @param {object} appendHints placement hint mapping as returned from getChildHintMapping().
* @return {object} a key set that contains all keys in `prev` and all keys
* in `next` in a reasonable order.
*/
export function mergeChildMappings(prev, next) {
export function mergeChildMappings(prev, next, appendHints) {
prev = prev || {}
next = next || {}
appendHints = appendHints || {}

function getValueForKey(key) {
return key in next ? next[key] : prev[key]
Expand All @@ -59,23 +77,72 @@ export function mergeChildMappings(prev, next) {
}
}

let i
let pendingNewKeys = []
let prependKeys = []
let appendKeys = []
let childMapping = {}
let i
for (let nextKey in next) {
if (nextKeysPending[nextKey]) {
for (i = 0; i < nextKeysPending[nextKey].length; i++) {
let pendingNextKey = nextKeysPending[nextKey][i]
childMapping[nextKeysPending[nextKey][i]] = getValueForKey(
pendingNextKey
)
if (nextKey in prev) {
if (nextKeysPending[nextKey]) {
prependKeys = []
appendKeys = []
if (pendingNewKeys.length) {
// If there were pending new keys that replaced the nextKeysPending,
// place them before or after the prevKeys based on the value of their appendOnReplace prop
for (i = 0; i < pendingNewKeys.length; i++) {
if (!appendHints[pendingNewKeys[i]]) {
prependKeys.push(pendingNewKeys[i])
} else {
appendKeys.push(pendingNewKeys[i])
}
}
pendingNewKeys = []
}
const combinedPendingKeys = prependKeys.concat(nextKeysPending[nextKey]).concat(appendKeys)
for (i = 0; i < combinedPendingKeys.length; i++) {
let pendingNextKey = combinedPendingKeys[i]
childMapping[pendingNextKey] = getValueForKey(
pendingNextKey
)
}
} else if (pendingNewKeys.length) {
// If there were no pending prevKeys, place the pendingNewKeys before nextKey
for (i = 0; i < pendingNewKeys.length; i++) {
childMapping[pendingNewKeys[i]] = getValueForKey(pendingNewKeys[i])
}
pendingNewKeys = []
}
childMapping[nextKey] = getValueForKey(nextKey)
} else {
// If the key is new, wait to see if they go before or after any pending keys
pendingNewKeys.push(nextKey)
}
}

// For the remaining pending and new keys that didn't appear before any keys in next,
// place the new keys before or after the pending keys based on the value of their appendOnReplace prop
prependKeys = []
appendKeys = []
let finalPendingKeys = []
if (pendingKeys.length && pendingNewKeys.length) {
for (i = 0; i < pendingNewKeys.length; i++) {
if (!appendHints[pendingNewKeys[i]]) {
prependKeys.push(pendingNewKeys[i])
} else {
appendKeys.push(pendingNewKeys[i])
}
}
childMapping[nextKey] = getValueForKey(nextKey)
finalPendingKeys = prependKeys.concat(pendingKeys).concat(appendKeys)
} else if (pendingKeys.length) {
finalPendingKeys = pendingKeys
} else if (pendingNewKeys.length) {
finalPendingKeys = pendingNewKeys
}

// Finally, add the keys which didn't appear before any key in `next`
for (i = 0; i < pendingKeys.length; i++) {
childMapping[pendingKeys[i]] = getValueForKey(pendingKeys[i])
// Finally, add the remaining keys
for (i = 0; i < finalPendingKeys.length; i++) {
childMapping[finalPendingKeys[i]] = getValueForKey(finalPendingKeys[i])
}

return childMapping
Expand All @@ -99,7 +166,8 @@ export function getInitialChildMapping(props, onExited) {

export function getNextChildMapping(nextProps, prevChildMapping, onExited) {
let nextChildMapping = getChildMapping(nextProps.children)
let children = mergeChildMappings(prevChildMapping, nextChildMapping)
let nextChildHintMapping = getChildHintMapping(nextProps.children)
let children = mergeChildMappings(prevChildMapping, nextChildMapping, nextChildHintMapping)

Object.keys(children).forEach(key => {
let child = children[key]
Expand Down
109 changes: 107 additions & 2 deletions test/CSSTransitionGroup-test.js
Expand Up @@ -48,7 +48,7 @@ describe('CSSTransitionGroup', () => {
it('should clean-up silently after the timeout elapses', () => {
render(
<TransitionGroup enter={false}>
<YoloTransition key="one" id="one"/>
<YoloTransition key="one" id="one" />
</TransitionGroup>,
container,
);
Expand All @@ -59,7 +59,7 @@ describe('CSSTransitionGroup', () => {

render(
<TransitionGroup enter={false}>
<YoloTransition key="two" id="two"/>
<YoloTransition key="two" id="two" />
</TransitionGroup>,
container,
);
Expand Down Expand Up @@ -137,6 +137,111 @@ describe('CSSTransitionGroup', () => {
expect(transitionGroupDiv.childNodes[1].id).toBe('two');
});

it('should place new node after the node it replaces when appendOnReplace is true', () => {
render(
<TransitionGroup>
<YoloTransition key="one" id="one" />
</TransitionGroup>,
container,
);

const transitionGroupDiv = container.childNodes[0]

expect(transitionGroupDiv.childNodes.length).toBe(1);

render(
<TransitionGroup>
<YoloTransition key="two" id="two" appendOnReplace />
</TransitionGroup>,
container,
);

expect(transitionGroupDiv.childNodes.length).toBe(2);
expect(transitionGroupDiv.childNodes[0].id).toBe('one');
expect(transitionGroupDiv.childNodes[1].id).toBe('two');
});

it('should place new node after last pending node when appendOnReplace is true', () => {
render(
<TransitionGroup enter={false} leave>
<YoloTransition key="one" id="one" />
<YoloTransition key="two" id="two" />
</TransitionGroup>,
container,
);

const transitionGroupDiv = container.childNodes[0]

render(
<TransitionGroup enter={false} leave>
<YoloTransition key="one" id="one" />
<YoloTransition key="three" id="three" appendOnReplace />
</TransitionGroup>,
container,
);

expect(transitionGroupDiv.childNodes.length).toBe(3);
expect(transitionGroupDiv.childNodes[1].id).toBe('two');
expect(transitionGroupDiv.childNodes[2].id).toBe('three');
});

it('should place new node after the pending node in the middle of the list when appendOnReplace is true', () => {
render(
<TransitionGroup enter={false} leave>
<YoloTransition key="one" id="one" />
<YoloTransition key="two" id="two" />
<YoloTransition key="three" id="three" />
</TransitionGroup>,
container,
);

const transitionGroupDiv = container.childNodes[0]

render(
<TransitionGroup enter={false} leave>
<YoloTransition key="one" id="one" />
<YoloTransition key="four" id="four" appendOnReplace />
<YoloTransition key="three" id="three" />
</TransitionGroup>,
container,
);

expect(transitionGroupDiv.childNodes.length).toBe(4);
expect(transitionGroupDiv.childNodes[0].id).toBe('one');
expect(transitionGroupDiv.childNodes[1].id).toBe('two');
expect(transitionGroupDiv.childNodes[2].id).toBe('four');
expect(transitionGroupDiv.childNodes[3].id).toBe('three');
});

it('should place pending nodes without appendOnReplace first, then new nodes, then pending nodes with appendOnReplace last', () => {
render(
<TransitionGroup enter={false} leave>
<YoloTransition key="one" id="one" />
<YoloTransition key="two" id="two" />
<YoloTransition key="three" id="three" />
</TransitionGroup>,
container,
);

const transitionGroupDiv = container.childNodes[0]

render(
<TransitionGroup enter={false} leave>
<YoloTransition key="one" id="one" />
<YoloTransition key="four" id="four" />
<YoloTransition key="five" id="five" appendOnReplace/>
<YoloTransition key="three" id="three" />
</TransitionGroup>,
container,
);

expect(transitionGroupDiv.childNodes.length).toBe(5);
expect(transitionGroupDiv.childNodes[0].id).toBe('one');
expect(transitionGroupDiv.childNodes[1].id).toBe('four');
expect(transitionGroupDiv.childNodes[2].id).toBe('two');
expect(transitionGroupDiv.childNodes[3].id).toBe('five');
expect(transitionGroupDiv.childNodes[4].id).toBe('three');
});

it('should work with a null child', () => {
render(
Expand Down