From 8b3ea20b30604dd5640e6e302d2bd137c14357a7 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Wed, 6 Dec 2017 12:16:30 +0300 Subject: [PATCH] The client side code corrections: javadocs are updated/added, unit tests. --- .../client/ExecuteJavaScriptElementUtils.java | 4 - .../java/com/vaadin/client/PolymerUtils.java | 101 ++++++-- .../client/communication/ServerConnector.java | 2 - .../com/vaadin/client/flow/StateNode.java | 11 + .../binding/SimpleElementBindingStrategy.java | 28 ++- .../flow/GwtBasicElementBinderTest.java | 228 ++++++++++-------- .../flow/GwtPropertyElementBinderTest.java | 13 + 7 files changed, 256 insertions(+), 131 deletions(-) diff --git a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java index 31bef418d5d..a9d346f2ece 100644 --- a/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java +++ b/flow-client/src/main/java/com/vaadin/client/ExecuteJavaScriptElementUtils.java @@ -15,10 +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; diff --git a/flow-client/src/main/java/com/vaadin/client/PolymerUtils.java b/flow-client/src/main/java/com/vaadin/client/PolymerUtils.java index 0c38cd42385..694fd9712b9 100644 --- a/flow-client/src/main/java/com/vaadin/client/PolymerUtils.java +++ b/flow-client/src/main/java/com/vaadin/client/PolymerUtils.java @@ -219,17 +219,47 @@ public static native Node getElementInShadowRootById(ShadowRoot shadowRoot, return shadowRoot.getElementById(id); }-*/; + /** + * Find the DOM element inside shadow root of the {@code shadowRootParent}. + * + * @param shadowRootParent + * the parent whose shadow root contains the element with the + * {@code id} + * @param id + * the identifier of the element to search for + * @return the element with the given {@code id} inside the shadow root of + * the parent + */ public static native Element getDomElementById(Node shadowRootParent, String id) /*-{ return shadowRootParent.$[id]; }-*/; + /** + * Checks whether the {@code node} has required {@code tag}. + * + * @param node + * the node to check + * @param tag + * the required tag name + * @return {@code true} if the node has required tag name + */ public static boolean hasTag(Node node, String tag) { return node instanceof Element && tag.equalsIgnoreCase(((Element) node).getTagName()); } + /** + * Gets the custom element using {@code path} of indices starting from the + * {@code root}. + * + * @param root + * the root element to start from + * @param path + * the indices path identifying the custom element. + * @return the element inside the {@code root} by the path of indices + */ public static Element getCustomElement(Node root, JsonArray path) { Node current = root; for (int i = 0; i < path.length(); i++) { @@ -248,30 +278,27 @@ public static Element getCustomElement(Node root, JsonArray path) { return null; } - public 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; - } - + /** + * Returns the shadow root of the {@code templateElement}. + * + * @param templateElement + * the owner of the shadow root + * @return the shadow root of the element + */ public static native Element getDomRoot(Node templateElement) /*-{ return templateElement.root; }-*/; + /** + * Invokes the {@code runnable} when the custom element with the given + * {@code tagName} is initialized (its DOM structure becomes available). + * + * @param tagName + * the name of the custom element + * @param runnable + * the command to run when the element if initialized + */ public static native void invokeWhenDefined(String tagName, Runnable runnable) /*-{ @@ -281,12 +308,48 @@ public static native void invokeWhenDefined(String tagName, }); }-*/; + /** + * Invokes the {@code runnable} when the custom element with the tag name of + * the {@code node} is initialized (its DOM structure becomes available). + * + * @param node + * the node whose tag name is awaiting for + * @param runnable + * the command to run when the element if initialized + */ public static void invokeWhenDefined(Node node, Runnable runnable) { invokeWhenDefined(node.getLocalName(), runnable); } + /** + * Gets the tag name of the {@code node}. + * + * @param node + * the node to get the tag name from + * @return the tag name of the node + */ public static String getTag(StateNode node) { return (String) node.getMap(NodeFeatures.ELEMENT_DATA) .getProperty(NodeProperties.TAG).getValue(); } + + 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; + } + } diff --git a/flow-client/src/main/java/com/vaadin/client/communication/ServerConnector.java b/flow-client/src/main/java/com/vaadin/client/communication/ServerConnector.java index bc583bdd355..6fb61d52886 100644 --- a/flow-client/src/main/java/com/vaadin/client/communication/ServerConnector.java +++ b/flow-client/src/main/java/com/vaadin/client/communication/ServerConnector.java @@ -196,8 +196,6 @@ public void sendExistingElementAttachToServer(StateNode parent, * @param assignedId * identifier which should be used on the server side for the * element (instead of requestedId) - * @param tagName - * the requested tagName * @param id * id of requested element */ diff --git a/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java b/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java index 4b6a15dbd9a..577f356dbd6 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/StateNode.java @@ -246,6 +246,17 @@ public void setDomNode(Node node) { }); } + /** + * Adds a listener to get a notification when the DOM Node is set for this + * {@link StateNode}. + *

+ * The listener return value is used to decide whether the listener should + * be removed immediately if it returns {@code true}. + * + * @param listener + * listener to add + * @return an event remover that can be used for removing the added listener + */ public EventRemover addDomNodeSetListener( Function listener) { domNodeSetListeners.add(listener); diff --git a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java index a630a448e2b..d742a6781d5 100644 --- a/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java +++ b/flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java @@ -22,6 +22,7 @@ import com.google.gwt.core.client.JavaScriptObject; import com.vaadin.client.Command; import com.vaadin.client.Console; +import com.vaadin.client.ExistingElementMap; import com.vaadin.client.PolymerUtils; import com.vaadin.client.WidgetUtil; import com.vaadin.client.flow.ConstantPool; @@ -545,8 +546,17 @@ private EventRemover bindChildren(BindingContext context) { for (int i = 0; i < children.length(); i++) { StateNode childNode = (StateNode) children.get(i); - Node child = context.binderContext.createAndBind(childNode); - DomApi.wrap(context.htmlNode).appendChild(child); + ExistingElementMap existingElementMap = childNode.getTree() + .getRegistry().getExistingElementMap(); + Node child = existingElementMap.getElement(childNode.getId()); + if (child != null) { + existingElementMap.remove(childNode.getId()); + childNode.setDomNode(child); + context.binderContext.createAndBind(childNode); + } else { + child = context.binderContext.createAndBind(childNode); + DomApi.wrap(context.htmlNode).appendChild(child); + } } return children.addSpliceListener(e -> { @@ -811,9 +821,19 @@ private void addChildren(int index, BindingContext context, Object newChildObject = add.get(i); StateNode newChild = (StateNode) newChildObject; - Node childNode = context.binderContext.createAndBind(newChild); + ExistingElementMap existingElementMap = newChild.getTree() + .getRegistry().getExistingElementMap(); + Node childNode = existingElementMap.getElement(newChild.getId()); + if (childNode != null) { + existingElementMap.remove(newChild.getId()); + newChild.setDomNode(childNode); + context.binderContext.createAndBind(newChild); + } else { + childNode = context.binderContext.createAndBind(newChild); - DomApi.wrap(context.htmlNode).insertBefore(childNode, beforeRef); + DomApi.wrap(context.htmlNode).insertBefore(childNode, + beforeRef); + } beforeRef = DomApi.wrap(childNode).getNextSibling(); } diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java index 84a2ac4351e..0ac30d45e5a 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java @@ -756,7 +756,7 @@ public void testAttachExistingElement() { Reactive.flush(); // nothing has changed: no new child - assertEquals(element.getChildElementCount(), 1); + assertEquals(1, element.getChildElementCount()); Element childElement = element.getFirstElementChild(); @@ -777,149 +777,137 @@ public void testPropertyValueHasPrototypeMethods() { assertEquals("[object Object]", toString); } - public void testBindVirtualElement() { - addShadowRootElement(element); - StateNode shadowRootNode = createAndAttachShadowRootNode(); - - // start binder test - Binder.bind(node, element); - - StateNode childNode = createChildNode("childElement"); + public void testBindChild_wrongTag_searchById() { + Element shadowRootElement = addShadowRootElement(element); - String tag = (String) childNode.getMap(NodeFeatures.ELEMENT_DATA) - .getProperty(NodeProperties.TAG).getValue(); + String childId = "childElement"; + createAndAttachShadowRootNode(); - ExistingElementMap existingElementMap = node.getTree().getRegistry() - .getExistingElementMap(); + StateNode child = createChildNode(childId, "a"); + addVirtualChild(node, child, NodeProperties.INJECT_BY_ID, + Json.create(childId)); - // create and add an existing element - Element span = Browser.getDocument().createElement(tag); - element.appendChild(span); - - existingElementMap.add(childNode.getId(), span); + Element elementWithDifferentId = createAndAppendElementToShadowRoot( + shadowRootElement, "otherId", "div"); + assertNotSame( + "Element added to shadow root should not have same id as virtual child node", + childId, elementWithDifferentId.getId()); - addVirtualChild(shadowRootNode, childNode); + Binder.bind(node, element); Reactive.flush(); - // nothing has changed: no new child - assertEquals(element.getChildElementCount(), 1); - - assertNull(existingElementMap.getElement(childNode.getId())); - - Element childElement = element.getFirstElementChild(); - - assertEquals(tag, - childElement.getTagName().toLowerCase(Locale.ENGLISH)); - assertSame(span, childElement); - assertEquals("childElement", childElement.getId()); - assertNull(existingElementMap.getElement(childNode.getId())); - } - - public void testBindChild_noTagAndId() { - addShadowRootElement(element); - addVirtualChild(createAndAttachShadowRootNode(), - createChildNode(null, null)); - - try { - Binder.bind(node, element); - fail("Appending child state node with no tag and id should cause an exception"); - } catch (IllegalStateException ignored) { - // expected - } + assertEquals(4, tree.existingElementRpcArgs.size()); + assertEquals(node, tree.existingElementRpcArgs.get(0)); + assertEquals(child.getId(), tree.existingElementRpcArgs.get(1)); + assertEquals(-1, tree.existingElementRpcArgs.get(2)); + assertEquals(childId, tree.existingElementRpcArgs.get(3)); } public void testBindChild_noCorrespondingElementInShadowRoot_searchById() { Element shadowRootElement = addShadowRootElement(element); String childId = "childElement"; - addVirtualChild(createAndAttachShadowRootNode(), - createChildNode(childId, null)); + createAndAttachShadowRootNode(); - Binder.bind(node, element); + String tag = "a"; + + StateNode child = createChildNode(childId, tag); + addVirtualChild(node, child, NodeProperties.INJECT_BY_ID, + Json.create(childId)); Element elementWithDifferentId = createAndAppendElementToShadowRoot( - shadowRootElement, "otherId", null); + shadowRootElement, "otherId", tag); assertNotSame( "Element added to shadow root should not have same id as virtual child node", childId, elementWithDifferentId.getId()); - try { - Reactive.flush(); - fail("Appending state node for element with no corresponding element in shadow root should cause an exception"); - } catch (IllegalStateException e) { - assertTrue( - "Exception message '" + e.getMessage() - + "' should contain id '" + childId + '\'', - e.getMessage().contains(childId)); - } + Binder.bind(node, element); + + Reactive.flush(); + + assertEquals(4, tree.existingElementRpcArgs.size()); + assertEquals(node, tree.existingElementRpcArgs.get(0)); + assertEquals(child.getId(), tree.existingElementRpcArgs.get(1)); + assertEquals(-1, tree.existingElementRpcArgs.get(2)); + assertEquals(childId, tree.existingElementRpcArgs.get(3)); } - public void testBindChild_noCorrespondingElementInShadowRoot_searchByTag() { + public void testBindChild_wrongTag_searchByIndicesPath() { Element shadowRootElement = addShadowRootElement(element); String childTagName = "span"; - addVirtualChild(createAndAttachShadowRootNode(), - createChildNode(null, childTagName)); + + createAndAttachShadowRootNode(); + + StateNode child = createChildNode(null, childTagName); Binder.bind(node, element); + JsonArray path = Json.createArray(); + path.set(0, 0); + addVirtualChild(node, child, NodeProperties.TEMPLATE_IN_TEMPLATE, path); + Element elementWithDifferentTag = createAndAppendElementToShadowRoot( shadowRootElement, null, "div"); assertNotSame( "Element added to shadow root should not have same tag name as virtual child node", childTagName, elementWithDifferentTag.getTagName()); - try { - Reactive.flush(); - fail("Appending state node for element with no corresponding element in shadow root should cause an exception"); - } catch (IllegalStateException e) { - assertTrue("Exception message '" + e.getMessage() - + "' should contain tag name '" + childTagName + '\'', - e.getMessage().contains(childTagName)); - } + Reactive.flush(); + + assertEquals(4, tree.existingElementRpcArgs.size()); + assertEquals(node, tree.existingElementRpcArgs.get(0)); + assertEquals(child.getId(), tree.existingElementRpcArgs.get(1)); + assertEquals(-1, tree.existingElementRpcArgs.get(2)); + assertEquals(null, tree.existingElementRpcArgs.get(3)); } - public void testBindChild_withCorrespondingElementInShadowRoot_byTagName() { + public void testBindChild_noCorrespondingElementInShadowRoot_searchByIndicesPath() { Element shadowRootElement = addShadowRootElement(element); - StateNode childNode = createChildNode(null, element.getTagName()); - addVirtualChild(createAndAttachShadowRootNode(), childNode); + + String childTagName = "span"; + + createAndAttachShadowRootNode(); + + StateNode child = createChildNode(null, childTagName); Binder.bind(node, element); - createAndAppendElementToShadowRoot(shadowRootElement, null, - element.getTagName()); - List expectedAfterBindingFeatures = Arrays.asList( - NodeFeatures.POLYMER_SERVER_EVENT_HANDLERS, - NodeFeatures.ELEMENT_CHILDREN, - NodeFeatures.SYNCHRONIZED_PROPERTY_EVENTS); + JsonArray path = Json.createArray(); + path.set(0, 1); + addVirtualChild(node, child, NodeProperties.TEMPLATE_IN_TEMPLATE, path); - expectedAfterBindingFeatures.forEach(notExpectedFeature -> assertFalse( - "Child node should not have any features from list " - + expectedAfterBindingFeatures - + " before binding, but got feature " - + notExpectedFeature, - childNode.hasFeature(notExpectedFeature))); + Element elementWithDifferentTag = createAndAppendElementToShadowRoot( + shadowRootElement, null, childTagName); + assertNotSame( + "Element added to shadow root should not have same tag name as virtual child node", + childTagName, elementWithDifferentTag.getTagName()); Reactive.flush(); - expectedAfterBindingFeatures.forEach(expectedFeature -> assertTrue( - "Child node should have all features from list " - + expectedAfterBindingFeatures - + " before binding, but missing feature " - + expectedFeature, - childNode.hasFeature(expectedFeature))); + assertEquals(4, tree.existingElementRpcArgs.size()); + assertEquals(node, tree.existingElementRpcArgs.get(0)); + assertEquals(child.getId(), tree.existingElementRpcArgs.get(1)); + assertEquals(-1, tree.existingElementRpcArgs.get(2)); + assertEquals(null, tree.existingElementRpcArgs.get(3)); } - public void testBindChild_withCorrespondingElementInShadowRoot_byId() { + public void testBindChild_withCorrespondingElementInShadowRoot_byTagNameAndIndicesPath() { Element shadowRootElement = addShadowRootElement(element); - String childId = "childElement"; - StateNode childNode = createChildNode(childId, element.getTagName()); - addVirtualChild(createAndAttachShadowRootNode(), childNode); + StateNode childNode = createChildNode(null, element.getTagName()); + JsonArray path = Json.createArray(); + path.set(0, 0); + + createAndAttachShadowRootNode(); Binder.bind(node, element); - createAndAppendElementToShadowRoot(shadowRootElement, childId, - element.getTagName()); + + addVirtualChild(node, childNode, NodeProperties.TEMPLATE_IN_TEMPLATE, + path); + + Element addressedElement = createAndAppendElementToShadowRoot( + shadowRootElement, null, element.getTagName()); List expectedAfterBindingFeatures = Arrays.asList( NodeFeatures.POLYMER_SERVER_EVENT_HANDLERS, @@ -941,15 +929,30 @@ public void testBindChild_withCorrespondingElementInShadowRoot_byId() { + " before binding, but missing feature " + expectedFeature, childNode.hasFeature(expectedFeature))); + + // nothing has changed: no new child + assertEquals(0, element.getChildElementCount()); + assertEquals(1, shadowRootElement.getChildElementCount()); + + Element childElement = shadowRootElement.getFirstElementChild(); + + assertSame(addressedElement, childElement); } - public void testBindChild_withAlreadyInitializedElement() { + public void testBindChild_withCorrespondingElementInShadowRoot_byId() { Element shadowRootElement = addShadowRootElement(element); String childId = "childElement"; StateNode childNode = createChildNode(childId, element.getTagName()); - addVirtualChild(createAndAttachShadowRootNode(), childNode); - childNode.setDomNode(createAndAppendElementToShadowRoot( - shadowRootElement, childId, element.getTagName())); + + createAndAttachShadowRootNode(); + + Binder.bind(node, element); + + addVirtualChild(node, childNode, NodeProperties.INJECT_BY_ID, + Json.create(childId)); + + Element addressedElement = createAndAppendElementToShadowRoot( + shadowRootElement, childId, element.getTagName()); List expectedAfterBindingFeatures = Arrays.asList( NodeFeatures.POLYMER_SERVER_EVENT_HANDLERS, @@ -963,7 +966,7 @@ public void testBindChild_withAlreadyInitializedElement() { + notExpectedFeature, childNode.hasFeature(notExpectedFeature))); - Binder.bind(node, element); + Reactive.flush(); expectedAfterBindingFeatures.forEach(expectedFeature -> assertTrue( "Child node should have all features from list " @@ -971,6 +974,14 @@ public void testBindChild_withAlreadyInitializedElement() { + " before binding, but missing feature " + expectedFeature, childNode.hasFeature(expectedFeature))); + + // nothing has changed: no new child + assertEquals(0, element.getChildElementCount()); + assertEquals(1, shadowRootElement.getChildElementCount()); + + Element childElement = shadowRootElement.getFirstElementChild(); + + assertSame(addressedElement, childElement); } private Element createAndAppendElementToShadowRoot(Element shadowRoot, @@ -979,13 +990,25 @@ private Element createAndAppendElementToShadowRoot(Element shadowRoot, .createElement(tagName); childShadowRootElement.setId(id); shadowRoot.appendChild(childShadowRootElement); + + if (id != null) { + JsonObject obj = Json.createObject(); + WidgetUtil.setJsProperty(obj, id.toString(), + childShadowRootElement); + WidgetUtil.setJsProperty(element, "$", obj); + } return childShadowRootElement; } - private void addVirtualChild(StateNode shadowRootNode, - StateNode childNode) { + private void addVirtualChild(StateNode shadowRootNode, StateNode childNode, + String type, JsonValue payload) { NodeList virtualChildren = shadowRootNode .getList(NodeFeatures.VIRTUAL_CHILDREN); + JsonObject object = Json.createObject(); + childNode.getMap(NodeFeatures.ELEMENT_DATA) + .getProperty(NodeProperties.PAYLOAD).setValue(object); + object.put(NodeProperties.TYPE, type); + object.put(NodeProperties.PAYLOAD, payload); virtualChildren.add(virtualChildren.length(), childNode); } @@ -1005,6 +1028,7 @@ private native Element addShadowRootElement(Element element) return this.querySelector('#' + id); }; element.shadowRoot = shadowRoot; + element.root = shadowRoot; return shadowRoot; }-*/; diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPropertyElementBinderTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPropertyElementBinderTest.java index a937c685f9f..1c0e2a7b668 100644 --- a/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPropertyElementBinderTest.java +++ b/flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPropertyElementBinderTest.java @@ -15,6 +15,9 @@ */ package com.vaadin.client.flow; +import java.util.ArrayList; +import java.util.List; + import com.vaadin.client.ClientEngineTestBase; import com.vaadin.client.ExistingElementMap; import com.vaadin.client.Registry; @@ -45,6 +48,7 @@ protected static class CollectingStateTree extends StateTree { JsArray collectedEventData = JsCollections.array(); JsMap> synchronizedProperties = JsCollections .map(); + List existingElementRpcArgs = new ArrayList<>(); public CollectingStateTree(ConstantPool constantPool, ExistingElementMap existingElementMap) { @@ -82,6 +86,15 @@ public void sendNodePropertySyncToServer(MapProperty property) { nodeMap.set(propertyName, value); } + @Override + public void sendExistingElementWithIdAttachToServer(StateNode parent, + int requestedId, int assignedId, String id) { + existingElementRpcArgs.add(parent); + existingElementRpcArgs.add(requestedId); + existingElementRpcArgs.add(assignedId); + existingElementRpcArgs.add(id); + } + public void clearSynchronizedProperties() { synchronizedProperties.clear(); }