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.

Sign up for GitHub

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 dynamic insertion of wrapped or redistributing content. #1816

Merged
merged 4 commits into from
Jun 12, 2015
Merged

Fix dynamic insertion of wrapped or redistributing content. #1816

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49ed47c
Fix dynamic insertion of wrapped or redistributing content.
kevinpschaaf Jun 10, 2015
17ad90f
Inch closer to correct.
kevinpschaaf Jun 11, 2015
05b8579
Add dynamic content tests, fix logic.
kevinpschaaf Jun 12, 2015
733e84d
Remove unused test setup.
kevinpschaaf Jun 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 64 additions & 66 deletions src/lib/dom-api.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,15 @@
// container to container.host.
// 3. node is <content> (host of container needs distribution)
appendChild: function(node) {
var distributed;
var handled;
this._removeNodeFromHost(node);
if (this._nodeIsInLogicalTree(this.node)) {
var host = this._hostForNode(this.node);
this._addLogicalInfo(node, this.node, host && host.shadyRoot);
this._addLogicalInfo(node, this.node);
this._addNodeToHost(node);
if (host) {
distributed = this._maybeDistribute(node, this.node, host);
}
handled = this._maybeDistribute(node, this.node);
}
// if not distributing and not adding to host, do a fast path addition
if (!distributed && !this._tryRemoveUndistributedNode(node)) {
if (!handled && !this._tryRemoveUndistributedNode(node)) {
// if adding to a shadyRoot, add to host instead
var container = this.node._isShadyRoot ? this.node.host : this.node;
nativeAppendChild.call(container, node);
Expand All @@ -82,7 +79,7 @@
if (!ref_node) {
return this.appendChild(node);
}
var distributed;
var handled;
this._removeNodeFromHost(node);
if (this._nodeIsInLogicalTree(this.node)) {
saveLightChildrenIfNeeded(this.node);
Expand All @@ -92,15 +89,12 @@
throw Error('The ref_node to be inserted before is not a child ' +
'of this node');
}
var host = this._hostForNode(this.node);
this._addLogicalInfo(node, this.node, host && host.shadyRoot, index);
this._addLogicalInfo(node, this.node, index);
this._addNodeToHost(node);
if (host) {
distributed = this._maybeDistribute(node, this.node, host);
}
handled = this._maybeDistribute(node, this.node);
}
// if not distributing and not adding to host, do a fast path addition
if (!distributed && !this._tryRemoveUndistributedNode(node)) {
if (!handled && !this._tryRemoveUndistributedNode(node)) {
// if ref_node is <content> replace with first distributed node
ref_node = ref_node.localName === CONTENT ?
this._firstComposedNode(ref_node) : ref_node;
Expand All @@ -118,19 +112,18 @@
*/
removeChild: function(node) {
if (factory(node).parentNode !== this.node) {
console.warn('The node to be removed is not a child of this node',
console.warn('The node to be removed is not a child of this node',
node);
}
var distributed;
var handled;
if (this._nodeIsInLogicalTree(this.node)) {
var host = this._hostForNode(this.node);
distributed = this._maybeDistribute(node, this.node, host);
handled = this._maybeDistribute(node, this.node);
this._removeNodeFromHost(node);
}
if (!distributed) {
if (!handled) {
// if removing from a shadyRoot, remove form host instead
var container = this.node._isShadyRoot ? this.node.host : this.node;
// not guaranteed to physically be in container; e.g.
// not guaranteed to physically be in container; e.g.
// undistributed nodes.
if (container === node.parentNode) {
nativeRemoveChild.call(container, node);
Expand Down Expand Up @@ -173,17 +166,39 @@

},

_maybeDistribute: function(node, parent, host) {
var nodeNeedsDistribute = this._nodeNeedsDistribution(node);
var distribute = this._parentNeedsDistribution(parent) ||
nodeNeedsDistribute;
if (nodeNeedsDistribute) {
_maybeDistribute: function(node, parent) {
// TODO(sorvell): technically we should check non-fragment nodes for
// <content> children but since this case is assumed to be exceedingly
// rare, we avoid the cost and will address with some specific api
// when the need arises.
var isContent = (node.localName === CONTENT);
var hasContent = (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) &&
node.querySelector(CONTENT);
// There are 2 possible cases where a host may need distribution:
// 1. <content> being inserted (the host of the shady root where
// content is inserted needs distribution)
// 2. children being inserted into parent with a shady root (parent
// needs distribution)
// Note that when 1 & 2 are both true (re-distribution), distributing the
// <content>'s host (1) will cause the parent to distribute as well (2)
var host;
if (isContent || hasContent) {
host = this._ownerShadyRootForNode(parent).host;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's possible we don't have an owner root here isn't it?

this._updateInsertionPoints(host);
} else if (this._parentNeedsDistribution(parent)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is always needed, remove the else.

host = parent;
}
if (distribute) {
if (host) {
this._lazyDistribute(host);
}
return distribute;
// Return true when distribution will fully handle the composition
// Note that if a content was being inserted that was wrapped by a node,
// and the parent does not need distribution, return false to allow
// the nodes to be added directly, after which children may be
// distributed and composed into the wrapping node(s)
var wrappedContent = hasContent &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasContent seems like a Boolean, let's make the name more explicit

(hasContent.parentNode.nodeType !== Node.DOCUMENT_FRAGMENT_NODE);
return host && (!wrappedContent || host == parent);
},

_tryRemoveUndistributedNode: function(node) {
Expand All @@ -200,35 +215,18 @@
factory(host.shadyRoot).querySelectorAll(CONTENT);
},

// a node is in a shadyRoot, is a shadyRoot,
// a node is in a shadyRoot, is a shadyRoot,
// or has a lightParent
_nodeIsInLogicalTree: function(node) {
return Boolean(node._lightParent || node._isShadyRoot ||
this._ownerShadyRootForNode(node) ||
node.shadyRoot);
},

// note: a node is its own host
_hostForNode: function(node) {
var root = node.shadyRoot || (node._isShadyRoot ?
node : this._ownerShadyRootForNode(node));
return root && root.host;
},

_parentNeedsDistribution: function(parent) {
return parent && parent.shadyRoot && hasInsertionPoint(parent.shadyRoot);
},

// TODO(sorvell): technically we should check non-fragment nodes for
// <content> children but since this case is assumed to be exceedingly
// rare, we avoid the cost and will address with some specific api
// when the need arises.
_nodeNeedsDistribution: function(node) {
return (node.localName === CONTENT) ||
((node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) &&
node.querySelector(CONTENT));
},

_removeNodeFromHost: function(node) {
if (node._lightParent) {
var root = this._ownerShadyRootForNode(node);
Expand All @@ -249,7 +247,7 @@
}
},

_addLogicalInfo: function(node, container, root, index) {
_addLogicalInfo: function(node, container, index) {
saveLightChildrenIfNeeded(container);
var children = factory(container).childNodes;
index = index === undefined ? children.length : index;
Expand Down Expand Up @@ -453,31 +451,31 @@
return (n.nodeType === Node.ELEMENT_NODE);
});
},
configurable: true
configurable: true
},

parentNode: {
get: function() {
return this.node._lightParent ||
return this.node._lightParent ||
(this.node.__patched ? this.node._composedParent :
this.node.parentNode);
},
configurable: true
configurable: true
},

firstChild: {
get: function() {
return this.childNodes[0];
},
configurable: true
configurable: true
},

lastChild: {
get: function() {
var c$ = this.childNodes;
return c$[c$.length-1];
},
configurable: true
configurable: true
},

nextSibling: {
Expand All @@ -487,7 +485,7 @@
return c$[Array.prototype.indexOf.call(c$, this.node) + 1];
}
},
configurable: true
configurable: true
},

previousSibling: {
Expand All @@ -497,22 +495,22 @@
return c$[Array.prototype.indexOf.call(c$, this.node) - 1];
}
},
configurable: true
configurable: true
},

firstElementChild: {
get: function() {
return this.children[0];
},
configurable: true
configurable: true
},

lastElementChild: {
get: function() {
var c$ = this.children;
return c$[c$.length-1];
},
configurable: true
configurable: true
},

nextElementSibling: {
Expand All @@ -522,7 +520,7 @@
return c$[Array.prototype.indexOf.call(c$, this.node) + 1];
}
},
configurable: true
configurable: true
},

previousElementSibling: {
Expand All @@ -532,7 +530,7 @@
return c$[Array.prototype.indexOf.call(c$, this.node) - 1];
}
},
configurable: true
configurable: true
},

// textContent / innerHTML
Expand All @@ -552,7 +550,7 @@
this.appendChild(document.createTextNode(text));
}
},
configurable: true
configurable: true
},

innerHTML: {
Expand All @@ -573,7 +571,7 @@
}
}
},
configurable: true
configurable: true
}

});
Expand Down Expand Up @@ -624,7 +622,7 @@
get: function() {
return Array.prototype.slice.call(this.node.children);
},
configurable: true
configurable: true
},

// textContent / innerHTML
Expand All @@ -635,7 +633,7 @@
set: function(value) {
return this.node.textContent = value;
},
configurable: true
configurable: true
},

innerHTML: {
Expand All @@ -645,21 +643,21 @@
set: function(value) {
return this.node.innerHTML = value;
},
configurable: true
configurable: true
}

});

var forwards = ['parentNode', 'firstChild', 'lastChild', 'nextSibling',
'previousSibling', 'firstElementChild', 'lastElementChild',
var forwards = ['parentNode', 'firstChild', 'lastChild', 'nextSibling',
'previousSibling', 'firstElementChild', 'lastElementChild',
'nextElementSibling', 'previousElementSibling'];

forwards.forEach(function(name) {
Object.defineProperty(DomApi.prototype, name, {
get: function() {
return this.node[name];
},
configurable: true
configurable: true
});
});

Expand Down Expand Up @@ -709,7 +707,7 @@
} else {
addNodeToComposedChildren(node, parent, children, i);
}

}

function addNodeToComposedChildren(node, parent, children, i) {
Expand Down
12 changes: 7 additions & 5 deletions src/mini/shady.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,13 @@

function clearDistributedDestinationInsertionPoints(content) {
var e$ = content._distributedNodes;
for (var i=0; i < e$.length; i++) {
var d = e$[i]._destinationInsertionPoints;
if (d) {
// this is +1 because these insertion points are *not* in this scope
d.splice(d.indexOf(content)+1, d.length);
if (e$) {
for (var i=0; i < e$.length; i++) {
var d = e$[i]._destinationInsertionPoints;
if (d) {
// this is +1 because these insertion points are *not* in this scope
d.splice(d.indexOf(content)+1, d.length);
}
}
}
}
Expand Down
36 changes: 35 additions & 1 deletion test/unit/polymer-dom-elements.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@

<script>
Polymer({
is: "x-distribute"
is: "x-distribute"
});
</script>

Expand Down Expand Up @@ -178,4 +178,38 @@
<script>Polymer({is: 'x-compose-select-attr'});</script>
</dom-module>

<dom-module id="x-dynamic-content">
<template>
<div id="container">
<template is="dom-if" id="domif">
<content select=".insert" id="dynamicContent"></content>
</template>
</div>
</template>
<script>Polymer({is: 'x-dynamic-content'});</script>
</dom-module>

<dom-module id="x-dynamic-content-wrapped">
<template>
<div>
<template is="dom-if" id="domif">
<div id="container">
<content select=".insert"></content>
</div>
</template>
</div>
</template>
<script>Polymer({is: 'x-dynamic-content-wrapped'});</script>
</dom-module>

<dom-module id="x-dynamic-content-redist">
<template>
<x-dynamic-content id="redistContainer">
<template is="dom-if" id="redistDomif">
<content select=".insert" id="redistContent"></content>
</template>
</x-dynamic-content>
</template>
<script>Polymer({is: 'x-dynamic-content-redist'});</script>
</dom-module>

Loading