Skip to content

Commit

Permalink
Use InitialPropertiesHandler for injected elements
Browse files Browse the repository at this point in the history 
Fixes #4304
  • Loading branch information
Denis Anisimov committed Jul 31, 2018
1 parent aff7bb0 commit b7cae0e
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 76 deletions.
43 changes: 27 additions & 16 deletions flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.vaadin.client.Command;
import com.vaadin.client.Console;
import com.vaadin.client.ExistingElementMap;
import com.vaadin.client.InitialPropertiesHandler;
import com.vaadin.client.PolymerUtils;
import com.vaadin.client.WidgetUtil;
import com.vaadin.client.flow.ConstantPool;
Expand Down Expand Up @@ -61,7 +62,6 @@
import elemental.json.JsonObject;
import elemental.json.JsonType;
import elemental.json.JsonValue;

import jsinterop.annotations.JsFunction;

/**
Expand Down Expand Up @@ -284,9 +284,9 @@ private native void bindPolymerModelProperties(StateNode node,
private native void hookUpPolymerElement(StateNode node, Element element)
/*-{
var self = this;
var originalPropertiesChanged = element._propertiesChanged;
if (originalPropertiesChanged) {
element._propertiesChanged = function (currentProps, changedProps, oldProps) {
$entry(function () {
Expand All @@ -295,16 +295,16 @@ private native void hookUpPolymerElement(StateNode node, Element element)
originalPropertiesChanged.apply(this, arguments);
};
}
var tree = [email protected]::getTree()();
var originalReady = element.ready;
element.ready = function (){
originalReady.apply(this, arguments);
@com.vaadin.client.PolymerUtils::fireReadyEvent(*)(element);
// The _propertiesChanged method which is replaced above for the element
// doesn't do anything for items in dom-repeat.
// Instead it's called with some meaningful info for the <code>dom-repeat</code> element.
Expand All @@ -313,7 +313,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
// which changes this method for any dom-repeat instance.
var replaceDomRepeatPropertyChange = function(){
var domRepeat = element.root.querySelector('dom-repeat');
if ( domRepeat ){
// If the <code>dom-repeat</code> element is in the DOM then
// this method should not be executed anymore. The logic below will replace
Expand All @@ -327,12 +327,12 @@ private native void hookUpPolymerElement(StateNode node, Element element)
// if dom-repeat is found => replace _propertiesChanged method in the prototype and mark it as replaced.
if ( !domRepeat.constructor.prototype.$propChangedModified){
domRepeat.constructor.prototype.$propChangedModified = true;
var changed = domRepeat.constructor.prototype._propertiesChanged;
domRepeat.constructor.prototype._propertiesChanged = function(currentProps, changedProps, oldProps){
changed.apply(this, arguments);
var props = Object.getOwnPropertyNames(changedProps);
var items = "items.";
for(i=0; i<props.length; i++){
Expand All @@ -352,7 +352,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
if( currentPropsItem && currentPropsItem.nodeId ){
var nodeId = currentPropsItem.nodeId;
var value = currentPropsItem[propertyName];
// this is an attempt to find the template element
// which is not available as a context in the protype method
var host = this.__dataHost;
Expand All @@ -363,7 +363,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
while( !host.localName || host.__dataHost ){
host = host.__dataHost;
}
$entry(function () {
@SimpleElementBindingStrategy::handleListItemPropertyChange(*)(nodeId, host, propertyName, value, tree);
})();
Expand All @@ -374,7 +374,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
};
}
};
// dom-repeat doesn't have to be in DOM even if template has it
// such situation happens if there is dom-if e.g. which evaluates to <code>false</code> initially.
// in this case dom-repeat is not yet in the DOM tree until dom-if becomes <code>true</code>
Expand All @@ -389,7 +389,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
element.addEventListener('dom-change',replaceDomRepeatPropertyChange);
}
}
}-*/;

private static void handleListItemPropertyChange(double nodeId,
Expand Down Expand Up @@ -846,6 +846,9 @@ private void appendVirtualChild(BindingContext context, StateNode node,
return;
}

InitialPropertiesHandler initialPropertiesHandler = node.getTree()
.getRegistry().getInitialPropertiesHandler();

assert context.htmlNode instanceof Element : "Unexpected html node. The node is supposed to be a custom element";
if (NodeProperties.INJECT_BY_ID.equals(type)) {
String id = object.getString(NodeProperties.PAYLOAD);
Expand All @@ -864,6 +867,10 @@ private void appendVirtualChild(BindingContext context, StateNode node,
.getDomElementById(context.htmlNode, id);
if (verifyAttachedElement(existingElement, node, id, address,
context)) {
if (!reactivePhase) {
initialPropertiesHandler.nodeRegistered(node);
initialPropertiesHandler.flushPropertyUpdates();
}
node.setDomNode(existingElement);
context.binderContext.createAndBind(node);
}
Expand All @@ -886,6 +893,10 @@ private void appendVirtualChild(BindingContext context, StateNode node,

if (verifyAttachedElement(customElement, node, null, address,
context)) {
if (!reactivePhase) {
initialPropertiesHandler.nodeRegistered(node);
initialPropertiesHandler.flushPropertyUpdates();
}
node.setDomNode(customElement);
context.binderContext.createAndBind(node);
}
Expand Down
41 changes: 38 additions & 3 deletions flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected void gwtSetUp() throws Exception {
titleProperty = properties.getProperty("title");
idAttribute = attributes.getProperty("id");

nextId = node.getId() + 1;
nextId = node.getId() + 2;

element = Browser.getDocument().createElement("div");
}
Expand Down Expand Up @@ -246,6 +246,7 @@ public void testRemoveAttribute() {

private StateNode createChildNode(String id, String tag) {
StateNode childNode = new StateNode(nextId++, node.getTree());
node.getTree().registerNode(childNode);

childNode.getMap(NodeFeatures.ELEMENT_DATA)
.getProperty(NodeProperties.TAG).setValue(tag);
Expand Down Expand Up @@ -1278,6 +1279,10 @@ public void testBindVirtualChild_withCorrespondingElementInShadowRoot_byId() {
String childId = "childElement";
StateNode childNode = createChildNode(childId, element.getTagName());

NodeMap properties = childNode.getMap(NodeFeatures.ELEMENT_PROPERTIES);
MapProperty fooProperty = properties.getProperty("foo");
fooProperty.setValue("bar");

Binder.bind(node, element);

addVirtualChild(node, childNode, NodeProperties.INJECT_BY_ID,
Expand Down Expand Up @@ -1326,6 +1331,10 @@ public void testBindVirtualChild_withDeferredElementInShadowRoot_byId() {
String tag = element.getTagName();
StateNode childNode = createChildNode(childId, tag);

NodeMap properties = childNode.getMap(NodeFeatures.ELEMENT_PROPERTIES);
MapProperty fooProperty = properties.getProperty("foo");
fooProperty.setValue("bar");

addVirtualChild(node, childNode, NodeProperties.INJECT_BY_ID,
Json.create(childId));

Expand All @@ -1352,9 +1361,20 @@ public void testBindVirtualChild_withDeferredElementInShadowRoot_byId() {
Element addressedElement = createAndAppendElementToShadowRoot(
shadowRoot, childId, tag);

// add flush listener which register the property to revert its initial
// value back if it has been changed during binding "from the client
// side" and do update the property emulating client side update
// The property value should be reverted back in the end
Reactive.addFlushListener(() -> {
tree.getRegistry().getInitialPropertiesHandler()
.handlePropertyUpdate(fooProperty);
fooProperty.setValue("baz");
});

PolymerUtils.fireReadyEvent(element);

Reactive.flush();
// the property value should be the same as initially
assertEquals("bar", fooProperty.getValue());

expectedAfterBindingFeatures.forEach(expectedFeature -> assertTrue(
"Child node should have all features from list "
Expand All @@ -1381,6 +1401,10 @@ public void testBindVirtualChild_withDeferredElementInShadowRoot_byIndicesPath()
String childId = "childElement";
StateNode childNode = createChildNode(childId, element.getTagName());

NodeMap properties = childNode.getMap(NodeFeatures.ELEMENT_PROPERTIES);
MapProperty fooProperty = properties.getProperty("foo");
fooProperty.setValue("bar");

WidgetUtil.setJsProperty(element, "ready", NativeFunction.create(""));

Binder.bind(node, element);
Expand Down Expand Up @@ -1411,9 +1435,20 @@ public void testBindVirtualChild_withDeferredElementInShadowRoot_byIndicesPath()
Element addressedElement = createAndAppendElementToShadowRoot(
shadowRoot, childId, element.getTagName());

// add flush listener which register the property to revert its initial
// value back if it has been changed during binding "from the client
// side" and do update the property emulating client side update
// The property value should be reverted back in the end
Reactive.addFlushListener(() -> {
tree.getRegistry().getInitialPropertiesHandler()
.handlePropertyUpdate(fooProperty);
fooProperty.setValue("baz");
});

PolymerUtils.fireReadyEvent(element);

Reactive.flush();
// the property value should be the same as initially
assertEquals("bar", fooProperty.getValue());

expectedAfterBindingFeatures.forEach(expectedFeature -> assertTrue(
"Child node should have all features from list "
Expand Down
47 changes: 36 additions & 11 deletions flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtPropertyElementBinderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.vaadin.client.ClientEngineTestBase;
import com.vaadin.client.ExistingElementMap;
import com.vaadin.client.InitialPropertiesHandler;
import com.vaadin.client.Registry;
import com.vaadin.client.flow.binding.Binder;
import com.vaadin.client.flow.collection.JsArray;
Expand All @@ -44,6 +45,39 @@
public abstract class GwtPropertyElementBinderTest
extends ClientEngineTestBase {

private static class TestRegistry extends Registry {
private InitialPropertiesHandler handler = new InitialPropertiesHandler(
this);

private ConstantPool constantPool;
private ExistingElementMap existingElementMap;

TestRegistry(ConstantPool constantPool,
ExistingElementMap existingElementMap) {
this.constantPool = constantPool;
this.existingElementMap = existingElementMap;
}

@Override
public ConstantPool getConstantPool() {
return constantPool;
}

@Override
public ExistingElementMap getExistingElementMap() {
return existingElementMap;
}

@Override
public InitialPropertiesHandler getInitialPropertiesHandler() {
return handler;
}

private void setTree(StateTree tree) {
set(StateTree.class, tree);
}
}

protected static class CollectingStateTree extends StateTree {
JsArray<StateNode> collectedNodes = JsCollections.array();
JsArray<JsonObject> collectedEventData = JsCollections.array();
Expand All @@ -53,17 +87,8 @@ protected static class CollectingStateTree extends StateTree {

public CollectingStateTree(ConstantPool constantPool,
ExistingElementMap existingElementMap) {
super(new Registry() {
@Override
public ConstantPool getConstantPool() {
return constantPool;
}

@Override
public ExistingElementMap getExistingElementMap() {
return existingElementMap;
}
});
super(new TestRegistry(constantPool, existingElementMap));
((TestRegistry) getRegistry()).setTree(this);
}

@Override
Expand Down
46 changes: 0 additions & 46 deletions ...tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/KeyPressListenerView.java
View file

This file was deleted.

0 comments on commit b7cae0e

Please sign in to comment.