diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java b/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java index 9ab9ac6f3d5..d94f23ed6d7 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java @@ -378,6 +378,25 @@ public void collectChanges(Consumer 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 initialFeatures = Stream @@ -693,33 +712,7 @@ private Stream 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; } /** diff --git a/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java b/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java index c2137e866a6..8700e18d661 100644 --- a/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java @@ -659,13 +659,8 @@ public void collectChanges_initiallyInactiveViaParentElement_sendOnlyDisalowAndR private void assertCollectChanges_initiallyInactive(StateNode stateNode, ElementPropertyMap properties, Consumer 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 @@ -673,18 +668,22 @@ private void assertCollectChanges_initiallyInactive(StateNode stateNode, // node is visible or not boolean visibilityChanged = !visibility.isVisible(); + properties.setProperty("foo", "bar"); + + TestStateTree tree = (TestStateTree) stateNode.getOwner(); + + tree.dirtyNodes.clear(); + List 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()); @@ -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), @@ -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); @@ -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 @@ -808,8 +808,7 @@ 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); @@ -817,9 +816,8 @@ private void assertCollectChanges_initiallyVisible(StateNode stateNode, } 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(); @@ -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; }