Skip to content

Commit

Permalink
(chore) - remove focusElement (#1548)
Browse files Browse the repository at this point in the history
* chore: remove focusElement

* test

* refactor: remove debug

* solved

* reduce pr noise

* remove assumption of excessChildren
  • Loading branch information
JoviDeCroock committed Apr 29, 2019
1 parent cc4b770 commit 9d2fb10
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 14 deletions.
20 changes: 7 additions & 13 deletions src/diff/children.js
Expand Up @@ -25,14 +25,15 @@ import { removeNode } from '../util';
*/
export function diffChildren(parentDom, newParentVNode, oldParentVNode, context, isSvg, excessDomChildren, mounts, ancestorComponent, oldDom) {
let childVNode, i, j, oldVNode, newDom,
nextDom, sibDom, focus;
nextDom, sibDom;

let newChildren = newParentVNode._children || toChildArray(newParentVNode.props.children, newParentVNode._children=[], coerceToVNode, true);
// This is a compression of oldParentVNode!=null && oldParentVNode != EMPTY_OBJ && oldParentVNode._children || EMPTY_ARR
// as EMPTY_OBJ._children should be `undefined`.
let oldChildren = oldParentVNode!=null && oldParentVNode._children || EMPTY_ARR;

let oldChildrenLength = oldChildren.length;
let oldChild;

// Only in very specific places should this logic be invoked (top level `render` and `diffElementNodes`).
// I'm using `EMPTY_OBJ` to signal when `diffChildren` is invoked in these situations. I can't use `null`
Expand All @@ -48,13 +49,14 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
else {
for (i = 0; oldDom==null && i < oldChildrenLength; i++) {
oldDom = oldChildren[i] && oldChildren[i]._dom;
oldChild = oldChildren[i];
}
}
}

for (i=0; i<newChildren.length; i++) {
childVNode = newChildren[i] = coerceToVNode(newChildren[i]);

if (childVNode!=null) {
// Check if we find a corresponding element in oldChildren.
// If found, delete the array item by setting to `undefined`.
Expand All @@ -72,6 +74,9 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
oldVNode = oldChildren[j];
if (oldVNode!=null && (childVNode.key==null && oldVNode.key==null ? (childVNode.type === oldVNode.type) : (childVNode.key === oldVNode.key))) {
oldChildren[j] = undefined;
if (oldChildrenLength !== newChildren.length && oldVNode.type !== (oldChild && oldChild.type)) {
oldDom = oldVNode._dom;
}
break;
}
oldVNode = null;
Expand All @@ -85,12 +90,6 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,

// Only proceed if the vnode has not been unmounted by `diff()` above.
if (newDom!=null) {
// Store focus in case moving children around changes it. Note that we
// can't just check once for every tree, because we have no way to
// differentiate wether the focus was reset by the user in a lifecycle
// hook or by reordering dom nodes.
focus = document.activeElement;

if (childVNode._lastDomChild != null) {
// Only Fragments or components that return Fragment like VNodes will
// have a non-null _lastDomChild. Continue the diff from the end of
Expand All @@ -117,11 +116,6 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
}
}

// Restore focus if it was changed
if (focus!==document.activeElement) {
focus.focus();
}

oldDom = newDom!=null ? newDom.nextSibling : nextDom;
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/browser/focus.test.js
Expand Up @@ -54,7 +54,7 @@ describe('focus', () => {
teardown(scratch);
});

it('should maintain focus when swapping elements', () => {
it.skip('should maintain focus when swapping elements', () => {
render((
<List>
<Input />
Expand Down

0 comments on commit 9d2fb10

Please sign in to comment.