Skip to content

Commit

Permalink
Revert #4397
Browse files Browse the repository at this point in the history
  • Loading branch information
@gilberto-torrezan
gilberto-torrezan committed Aug 1, 2018
1 parent d688bba commit f4df17f
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 48 deletions.
47 changes: 20 additions & 27 deletions flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,25 @@ public void collectChanges(Consumer<NodeChange> collector) {
return;
}
if (isInactive()) {
if (!isInactiveSelf) {
/*
* We are here if: the node itself is not inactive but it has
* some ascendant which is inactive.
*
* In this case we send only some subset of changes (not from
* all the features). But we should send changes for all
* remaining features. Normally it automatically happens if the
* node becomes "visible". But if it was visible with some
* invisible parent then only the parent becomes dirty (when
* it's set visible) and this child will never participate in
* collection of changes since it's not marked as dirty.
*
* So here such node (which is active itself but its ascendant
* is inactive) we mark as dirty again to be able to collect its
* changes later on when its ascendant becomes active.
*/
getOwner().markAsDirty(this);
}
if (isInitialChanges) {
// send only required (reported) features updates
Stream<NodeFeature> initialFeatures = Stream
Expand Down Expand Up @@ -693,33 +712,7 @@ private Stream<NodeFeature> getDisalowFeatures() {
}

private void setInactive(boolean inactive) {
if (isInactiveSelf != inactive) {
isInactiveSelf = inactive;

visitNodeTree(child -> {
if (!this.equals(child) && !child.isInactiveSelf) {
/*
* We are here if: the child node itself is not inactive but
* it has some ascendant which is inactive.
*
* In this case we send only some subset of changes (not
* from all the features). But we should send changes for
* all remaining features. Normally it automatically happens
* if the node becomes "visible". But if it was visible with
* some invisible parent then only the parent becomes dirty
* (when it's set visible) and this child will never
* participate in collection of changes since it's not
* marked as dirty.
*
* So here such node (which is active itself but its
* ascendant is inactive) we mark as dirty again to be able
* to collect its changes later on when its ascendant
* becomes active.
*/
child.markAsDirty();
}
});
}
isInactiveSelf = inactive;
}

/**
Expand Down
39 changes: 18 additions & 21 deletions flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,32 +659,31 @@ public void collectChanges_initiallyInactiveViaParentElement_sendOnlyDisalowAndR

private void assertCollectChanges_initiallyInactive(StateNode stateNode,
ElementPropertyMap properties, Consumer<Boolean> activityUpdater) {

properties.setProperty("foo", "bar");

TestStateTree tree = (TestStateTree) stateNode.getOwner();
tree.dirtyNodes.clear();

ElementData visibility = stateNode.getFeature(ElementData.class);

activityUpdater.accept(false);

// activity updater may modify visibility of the node itself or its
// ancestor. The number of changes will depend on whether the subject
// node is visible or not
boolean visibilityChanged = !visibility.isVisible();

properties.setProperty("foo", "bar");

TestStateTree tree = (TestStateTree) stateNode.getOwner();

tree.dirtyNodes.clear();

List<NodeChange> changes = new ArrayList<>();
stateNode.collectChanges(changes::add);

if (visibilityChanged) {
Assert.assertEquals(1, tree.dirtyNodes.size());
Assert.assertThat(tree.dirtyNodes, CoreMatchers.hasItem(stateNode));
Assert.assertEquals(0, tree.dirtyNodes.size());
} else {
// the target node should be marked as dirty because it's visible
// but its parent is inactive
Assert.assertEquals(2, tree.dirtyNodes.size());
stateNode.visitNodeTree(node -> Assert.assertThat(tree.dirtyNodes,
CoreMatchers.hasItem(node)));
Assert.assertEquals(1, tree.dirtyNodes.size());
tree.dirtyNodes.contains(stateNode);
}

Assert.assertEquals(visibilityChanged ? 3 : 2, changes.size());
Expand All @@ -700,6 +699,7 @@ private void assertCollectChanges_initiallyInactive(StateNode stateNode,
Assert.assertTrue("No tag change found", tagFound.isPresent());
MapPutChange tagChange = tagFound.get();


MapPutChange change = (MapPutChange) changes.get(1);
if (visibilityChanged) {
Assert.assertThat(changes.get(2),
Expand Down Expand Up @@ -784,9 +784,6 @@ private void assertCollectChanges_initiallyVisible(StateNode stateNode,

changes.clear();

TestStateTree tree = (TestStateTree) stateNode.getOwner();
tree.dirtyNodes.clear();

// now make the node inactive via the VisibiltyData

activityUpdater.accept(false);
Expand All @@ -796,6 +793,9 @@ private void assertCollectChanges_initiallyVisible(StateNode stateNode,

properties.setProperty("foo", "baz");

TestStateTree tree = (TestStateTree) stateNode.getOwner();
tree.dirtyNodes.clear();

stateNode.collectChanges(changes::add);

// activity updater may modify visibility of the node itself or its
Expand All @@ -808,18 +808,16 @@ private void assertCollectChanges_initiallyVisible(StateNode stateNode,

MapPutChange change;
if (visibilityChanged) {
Assert.assertEquals(1, tree.dirtyNodes.size());
Assert.assertThat(tree.dirtyNodes, CoreMatchers.hasItem(stateNode));
Assert.assertEquals(0, tree.dirtyNodes.size());
Assert.assertThat(changes.get(0),
CoreMatchers.instanceOf(MapPutChange.class));
change = (MapPutChange) changes.get(0);
Assert.assertEquals(ElementData.class, change.getFeature());
} else {
// the target node should be marked as dirty because it's visible
// but its parent is inactive
Assert.assertEquals(2, tree.dirtyNodes.size());
stateNode.visitNodeTree(node -> Assert.assertThat(tree.dirtyNodes,
CoreMatchers.hasItem(node)));
Assert.assertEquals(1, tree.dirtyNodes.size());
tree.dirtyNodes.contains(stateNode);
}

changes.clear();
Expand All @@ -843,8 +841,7 @@ private void assertCollectChanges_initiallyVisible(StateNode stateNode,
.equals(change.getFeature()) ? change
: (MapPutChange) changes.get(1);
propertyChange = change.equals(visibilityChange)
? (MapPutChange) changes.get(1)
: change;
? (MapPutChange) changes.get(1) : change;
} else {
propertyChange = change;
}
Expand Down

0 comments on commit f4df17f

Please sign in to comment.