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

Rewrite attach by @Id and template-in-template features #3098

Merged
merged 24 commits into from
Dec 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
318fcf4
Remove obsolete features and reimplement "attach" on the server side
Nov 30, 2017
6706877
Update RPC handler logic.
Nov 30, 2017
29d4d9c
Remove VirtualChildrenList feature
Nov 30, 2017
bc40993
Rename NewVirtualChildrenList to VirtualChildrenList
Nov 30, 2017
4458bd5
Correct logic for template-in-template case
Nov 30, 2017
21761bd
Reimplement "attach by id" on the client-side.
Nov 30, 2017
0163fb2
Improve/correct the client-side implementation.
Dec 1, 2017
0893a48
Correct model properties population.
Dec 1, 2017
4fa857e
Correct JS execution code for attached elements.
Dec 4, 2017
3fa4db0
Add javadocs to the client side class.
Dec 4, 2017
22e5e8d
Correct unit test for RPC response from the client-side.
Dec 4, 2017
5655074
Change JS execution logic and add IT test for injection inside templa…
Dec 5, 2017
7aab292
Update JUnit server side tests for new implementation.
Dec 5, 2017
1cea656
Correct GWT unit tests to make them compilable.
Dec 5, 2017
25e788e
Merge branch 'master' into 3057-attach-by-id
Dec 5, 2017
fcefbdc
Add IT test
Dec 5, 2017
8d3b525
The client side code corrections: javadocs are updated/added, unit te…
Dec 6, 2017
8bb7714
Extract error message prefix as a constant
Dec 6, 2017
0cb2ae8
Java Unit test for execute JS processor (virtual child awaiting init).
Dec 6, 2017
1eba7e6
Correct validation code and make GWT unit test for it.
Dec 6, 2017
36c6261
More GWT unit tests for unhappy path
Dec 7, 2017
9d6f6a8
Correct shadow root handling. GWT unit test for the postponed attach …
Dec 7, 2017
cc3cc5a
GWT unit test for postponed attach by indices path.
Dec 7, 2017
1bf13b4
Code review corrections.
Dec 7, 2017
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package com.vaadin.client;

import com.google.gwt.core.client.Scheduler;

import com.vaadin.client.flow.ExecuteJavaScriptProcessor;
import com.vaadin.client.flow.StateNode;
import com.vaadin.client.flow.collection.JsArray;
Expand All @@ -26,13 +24,9 @@
import com.vaadin.client.flow.nodefeature.NodeMap;
import com.vaadin.client.flow.reactive.Reactive;
import com.vaadin.flow.nodefeature.NodeFeatures;
import com.vaadin.flow.nodefeature.NodeProperties;

import elemental.dom.Element;
import elemental.dom.Node;
import elemental.html.HTMLCollection;
import elemental.json.JsonArray;
import elemental.json.JsonValue;

/**
* Utility class which handles javascript execution context (see
Expand Down Expand Up @@ -118,79 +112,6 @@ private static boolean hasTag(Node node, String tag) {
&& tag.equalsIgnoreCase(((Element) node).getTagName());
}

/**
* Find element for given id and collect data required for server side
* callback to attach existing element and send it to the server.
*
* @param parent
* the parent node containing the shadow root containing the
* element requested to attach
* @param tagName
* the tag name of the element requested to attach
* @param serverSideId
* the identifier of the server side node which is requested to
* be a counterpart of the client side element
* @param id
* the id attribute of the element to wire to
*/
public static void attachExistingElementById(StateNode parent,
String tagName, int serverSideId, String id) {
if (parent.getDomNode() == null) {
Reactive.addPostFlushListener(() -> Scheduler.get()
.scheduleDeferred(() -> attachExistingElementById(parent,
tagName, serverSideId, id)));
} else if (getDomRoot(parent.getDomNode()) == null) {
invokeWhenDefined(parent.getDomNode(),
() -> attachExistingElementById(parent, tagName,
serverSideId, id));
return;
} else {
Element existingElement = getDomElementById(
(Element) parent.getDomNode(), id);

respondExistingElement(parent, tagName, serverSideId, id,
existingElement);
}
}

/**
* Find element by the given {@code path} in the {@code parent} and collect
* data required for server side callback to attach existing element and
* send it to the server.
*
* @param parent
* the parent node containing the shadow root containing the
* element requested to attach
* @param tagName
* the tag name of the element requested to attach
* @param serverSideId
* the identifier of the server side node which is requested to
* be a counterpart of the client side element
* @param path
* the path from the {@code parent} template element to the
* element to wire to (consist of indices)
*/
public static void attachCustomElement(StateNode parent, String tagName,
int serverSideId, JsonArray path) {
if (getDomRoot(parent.getDomNode()) == null) {
invokeWhenDefined(parent.getDomNode(),
() -> attachCustomElement(parent, tagName, serverSideId,
path));
return;
}
Element customElement = getCustomElement(
getDomRoot(parent.getDomNode()), path);
if (customElement != null
&& !tagName.equalsIgnoreCase(customElement.getTagName())) {
Console.warn("Custom element addressed by the path '" + path
+ "' has wrong tag name '" + customElement.getTagName()
+ "', required tag '" + tagName + "'");
}
respondExistingElement(parent, tagName, serverSideId, null,
customElement);

}

/**
* Populate model {@code properties}: add them into
* {@literal NodeFeatures.ELEMENT_PROPERTIES} {@link NodeMap} if they are
Expand All @@ -205,98 +126,25 @@ public static void attachCustomElement(StateNode parent, String tagName,
public static void populateModelProperties(StateNode node,
JsArray<String> properties) {
NodeMap map = node.getMap(NodeFeatures.ELEMENT_PROPERTIES);
if (node.getDomNode() == null) {
PolymerUtils.invokeWhenDefined(PolymerUtils.getTag(node),
() -> Reactive.addPostFlushListener(
() -> populateModelProperties(node, properties)));
return;
}
for (int i = 0; i < properties.length(); i++) {
String property = properties.get(i);
if (!isPropertyDefined(node.getDomNode(), property)) {
map.getProperty(property).setValue(null);
if (!map.hasPropertyValue(property)) {
map.getProperty(property).setValue(null);
}
} else {
map.getProperty(property).syncToServer(
WidgetUtil.getJsProperty(node.getDomNode(), property));
}
}
}

private static void respondExistingElement(StateNode parent, String tagName,
int serverSideId, String id, Element existingElement) {
if (existingElement != null && hasTag(existingElement, tagName)) {
NodeMap map = parent.getMap(NodeFeatures.SHADOW_ROOT_DATA);
StateNode shadowRootNode = (StateNode) map
.getProperty(NodeProperties.SHADOW_ROOT).getValue();
NodeList list = shadowRootNode
.getList(NodeFeatures.ELEMENT_CHILDREN);
Integer existingId = null;

for (int i = 0; i < list.length(); i++) {
StateNode stateNode = (StateNode) list.get(i);
Node domNode = stateNode.getDomNode();

if (domNode.equals(existingElement)) {
existingId = stateNode.getId();
break;
}
}

existingId = getExistingIdOrUpdate(shadowRootNode, serverSideId,
existingElement, existingId);

// Return this as attach to parent which will delegate it to the
// underlying shadowRoot as a virtual child.
parent.getTree().sendExistingElementWithIdAttachToServer(parent,
serverSideId, existingId, existingElement.getTagName(), id);
} else {
parent.getTree().sendExistingElementWithIdAttachToServer(parent,
serverSideId, -1, tagName, id);
}
}

private static Element getCustomElement(Node root, JsonArray path) {
Node current = root;
for (int i = 0; i < path.length(); i++) {
JsonValue value = path.get(i);
current = getChildIgnoringStyles(current, (int) value.asNumber());
}
if (current instanceof Element) {
return (Element) current;
} else if (current == null) {
Console.warn(
"There is no element addressed by the path '" + path + "'");
} else {
Console.warn("The node addressed by path " + path
+ " is not an Element");
}
return null;
}

private static Node getChildIgnoringStyles(Node parent, int index) {
HTMLCollection children = DomApi.wrap(parent).getChildren();
int filteredIndex = -1;
for (int i = 0; i < children.getLength(); i++) {
Node next = children.item(i);
assert next instanceof Element : "Unexpected element type in the collection of children. "
+ "DomElement::getChildren is supposed to return Element chidren only, but got "
+ next.getClass();
Element element = (Element) next;
if (!"style".equalsIgnoreCase(element.getTagName())) {
filteredIndex++;
}
if (filteredIndex == index) {
return next;
}
}
return null;
}

private static native Element getDomElementById(Element shadowRootParent,
String id)
/*-{
return shadowRootParent.$[id];
}-*/;

private static native Element getDomRoot(Node templateElement)
/*-{
return templateElement.root;
}-*/;

private static Integer getExistingIdOrUpdate(StateNode parent,
int serverSideId, Element existingElement, Integer existingId) {
if (existingId == null) {
Expand All @@ -312,14 +160,6 @@ private static Integer getExistingIdOrUpdate(StateNode parent,
return existingId;
}

private static native void invokeWhenDefined(Node node, Runnable runnable)
/*-{
$wnd.customElements.whenDefined(node.localName).then(
function () {
runnable.@java.lang.Runnable::run(*)();
});
}-*/;

private static native boolean isPropertyDefined(Node node, String property)
/*-{
return !!(node["constructor"] && node["constructor"]["properties"] &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@
*/
package com.vaadin.client;

import java.util.function.Function;

import com.vaadin.client.flow.collection.JsArray;
import com.vaadin.client.flow.collection.JsCollections;
import com.vaadin.client.flow.collection.JsMap;
import com.vaadin.client.flow.collection.JsSet;

import elemental.dom.Element;
import elemental.events.EventRemover;

/**
* Mapping between a server side node identifier which has been requested to
Expand All @@ -34,9 +30,6 @@
*/
public class ExistingElementMap {

private final JsSet<Function<Integer, Boolean>> listeners = JsCollections
.set();

private final JsMap<Element, Integer> elementToId = JsCollections.map();
// JsArray is used as a Map<Integer,Element> here. So this is a map between
// an id and an Element.
Expand Down Expand Up @@ -79,15 +72,6 @@ public void remove(int id) {
if (element != null) {
idToElement.set(id, null);
elementToId.delete(element);

JsSet<Function<Integer, Boolean>> copy = JsCollections
.set(listeners);

copy.forEach(listener -> {
if (listener.apply(id)) {
listeners.delete(listener);
}
});
}
}

Expand All @@ -104,22 +88,4 @@ public void add(int id, Element element) {
elementToId.set(element, id);
}

/**
* Add remove listener for the identifier of the node.
* <p>
* Listener interface is a function that accepts the identifier of removed
* node and returns {@code true} if the listener should be removed once the
* node is removed. If it returns {@code false} then it's preserved in the
* listeners list.
*
* @param listener
* the node remove listener to add
* @return an event remover that can be used to remove the listener
*/
public EventRemover addNodeRemoveListener(
Function<Integer, Boolean> listener) {
listeners.add(listener);
return () -> listeners.delete(listener);
}

}
Loading