Skip to content

Commit

Permalink
Rewrite attach by @id and template-in-template features (#3098)
Browse files Browse the repository at this point in the history 
* Remove obsolete features and reimplement "attach" on the server side

* Update RPC handler logic.

* Remove VirtualChildrenList feature

* Rename NewVirtualChildrenList to VirtualChildrenList

* Correct logic for template-in-template case

* Reimplement "attach by id" on the client-side.

* Improve/correct the client-side implementation.

* Correct model properties population.

* Correct JS execution code for attached elements.

* Add javadocs to the client side class.

* Correct unit test for RPC response from the client-side.

* Change JS execution logic and add IT test for injection inside template-in-template

* Update JUnit server side tests for new implementation.

* Correct GWT unit tests to make them compilable.

* Merge branch 'master' into 3057-attach-by-id

* Add IT test

* The client side code corrections: javadocs are updated/added, unit tests.

* Extract error message prefix as a constant

* Java Unit test for execute JS processor (virtual child awaiting init).

* Correct validation code and make GWT unit test for it.

* More GWT unit tests for unhappy path

* Correct shadow root handling. GWT unit test for the postponed attach by id.

* GWT unit test for postponed attach by indices path.

* Code review corrections.
  • Loading branch information
Denis authored Dec 7, 2017
1 parent a4fbf46 commit 440418f
Show file tree
Hide file tree
Showing 41 changed files with 1,409 additions and 1,421 deletions.
178 changes: 9 additions & 169 deletions flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java
View file
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
34 changes: 0 additions & 34 deletions flow-client/src/main/java/com/vaadin/client/ExistingElementMap.java
View file
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

0 comments on commit 440418f

Please sign in to comment.