From 9959a86b677a329954cd0fb963eedabde3d22c2b Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 11 Nov 2024 23:02:48 +0200 Subject: [PATCH] refactor: patch data generator instead of scheduling renderer --- .../com/vaadin/flow/component/grid/Grid.java | 125 +++++++++--------- .../grid/GridHiddenColumnRenderingTest.java | 16 +-- 2 files changed, 71 insertions(+), 70 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java index a030159d68b..75fa35c2ba3 100755 --- a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java @@ -444,7 +444,6 @@ public static class Column extends AbstractColumn> { private String columnKey; // defined and used by the user private boolean sortingEnabled; - private boolean rendererSetupScheduled; private Component editorComponent; private EditorRenderer editorRenderer; @@ -485,8 +484,38 @@ public Column(Grid grid, String columnId, Renderer renderer) { super(grid); Objects.requireNonNull(renderer); this.columnInternalId = columnId; + this.renderer = renderer; + comparator = (a, b) -> 0; - setRenderer(renderer); + + rendering = renderer.render(getElement(), (KeyMapper) getGrid() + .getDataCommunicator().getKeyMapper()); + + Optional> dataGenerator = rendering + .getDataGenerator(); + + if (dataGenerator.isPresent()) { + var generator = dataGenerator.get(); + DataGenerator conditionalDataGenerator = (item, + jsonObject) -> { + if (Column.this.isVisible()) { + generator.generateData(item, jsonObject); + } + }; + + columnDataGeneratorRegistration = grid + .addDataGenerator(conditionalDataGenerator); + } + } + + @Override + public void setVisible(boolean visible) { + boolean resetDataCommunicator = visible && !isVisible(); + super.setVisible(visible); + if (resetDataCommunicator) { + getGrid().getDataCommunicator().reset(); + + } } protected void destroyDataGenerators() { @@ -530,10 +559,33 @@ public Renderer getRenderer() { public Column setRenderer(Renderer renderer) { this.renderer = Objects.requireNonNull(renderer, "Renderer must not be null."); - clearRendering(); - getElement().getNode() - .runWhenAttached(ui -> scheduleRendererSetup()); - addAttachListener(e -> scheduleRendererSetup()); + + destroyDataGenerators(); + if (rendering != null) { + rendering.getRegistration().remove(); + } + + rendering = renderer.render(getElement(), (KeyMapper) getGrid() + .getDataCommunicator().getKeyMapper()); + + columnDataGeneratorRegistration = rendering.getDataGenerator() + .map(dataGenerator -> grid + .addDataGenerator((DataGenerator) dataGenerator)) + .orElse(null); + + // The editor renderer is a wrapper around the regular renderer, so + // we need to apply it again afterwards + if (editorRenderer != null) { + Rendering editorRendering = editorRenderer + .render(getElement(), null); + editorDataGeneratorRegistration = editorRendering + .getDataGenerator() + .map(dataGenerator -> grid.addDataGenerator( + (DataGenerator) dataGenerator)) + .orElse(null); + } + + getGrid().getDataCommunicator().reset(); return this; } @@ -1133,18 +1185,6 @@ public Column setRowHeader(boolean rowHeader) { return this; } - @Override - public void setVisible(boolean visible) { - boolean isInitiallyVisible = isVisible(); - super.setVisible(visible); - if (isInitiallyVisible && !visible) { - clearRendering(); - } - if (!isInitiallyVisible && visible) { - scheduleRendererSetup(); - } - } - @Override protected Column getBottomLevelColumn() { return this; @@ -1154,54 +1194,15 @@ protected Column getBottomLevelColumn() { private void setupColumnEditor() { editorRenderer = new EditorRenderer<>((Editor) grid.getEditor(), columnInternalId); - setupEditorRenderer(); - } - private void setupRenderer() { - if (renderer == null) { - return; - } - rendering = renderer.render(getElement(), (KeyMapper) getGrid() - .getDataCommunicator().getKeyMapper()); - columnDataGeneratorRegistration = rendering.getDataGenerator() - .map(dataGenerator -> grid - .addDataGenerator((DataGenerator) dataGenerator)) - .orElse(null); - grid.getDataProvider().refreshAll(); - } - - private void setupEditorRenderer() { - if (editorRenderer == null) { - return; - } Rendering editorRendering = editorRenderer.render(getElement(), null); - editorDataGeneratorRegistration = editorRendering.getDataGenerator() - .map(dataGenerator -> grid - .addDataGenerator((DataGenerator) dataGenerator)) - .orElse(null); - } - private void scheduleRendererSetup() { - if (rendererSetupScheduled) { - return; - } - rendererSetupScheduled = true; - getUI().ifPresent(ui -> ui.beforeClientResponse(this, ctx -> { - if (rendererSetupScheduled && isVisible()) { - setupRenderer(); - // The editor renderer is a wrapper around the regular - // renderer, so we need to apply it again afterward. - setupEditorRenderer(); - } - rendererSetupScheduled = false; - })); - } - - private void clearRendering() { - destroyDataGenerators(); - if (rendering != null) { - rendering.getRegistration().remove(); + Optional> dataGenerator = editorRendering + .getDataGenerator(); + if (dataGenerator.isPresent()) { + editorDataGeneratorRegistration = grid + .addDataGenerator((DataGenerator) dataGenerator.get()); } } } diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridHiddenColumnRenderingTest.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridHiddenColumnRenderingTest.java index c13b7e865aa..144eeaf160a 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridHiddenColumnRenderingTest.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/GridHiddenColumnRenderingTest.java @@ -89,9 +89,9 @@ public void initiallyHiddenColumnWithValueProvider_setVisible_rendererCalledOnce } @Test - public void columnWithValueProvider_toggleHiddenTwiceInRoundTrip_rendererCalledOncePerItem() { + public void columnWithValueProvider_toggleHiddenTwiceInRoundTrip_rendererCalledAtMostOncePerItem() { Grid.Column column = addColumnWithValueProvider(); - initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem( + initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem( column); } @@ -128,9 +128,9 @@ public void initiallyHiddenComponentColumn_setVisible_rendererCalledOncePerItem( } @Test - public void componentColumn_toggleHiddenTwiceInRoundTrip_rendererCalledOncePerItem() { + public void componentColumn_toggleHiddenTwiceInRoundTrip_rendererCalledAtMostOncePerItem() { Grid.Column column = addComponentColumn(); - initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem( + initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem( column); } @@ -167,9 +167,9 @@ public void initiallyHiddenColumnWithCustomRenderer_setVisible_rendererCalledOnc } @Test - public void columnWithCustomRenderer_toggleHiddenTwiceInRoundTrip_rendererCalledOncePerItem() { + public void columnWithCustomRenderer_toggleHiddenTwiceInRoundTrip_rendererCalledAtMostOncePerItem() { Grid.Column column = addColumnWithCustomRenderer(); - initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem( + initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem( column); } @@ -316,14 +316,14 @@ private void initiallyHiddenColumn_setVisible_assertRendererCalledOncePerItem( Assert.assertEquals(ITEM_COUNT, callCount.get()); } - private void initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem( + private void initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem( Grid.Column column) { fakeClientCommunication(); column.setVisible(false); column.setVisible(true); callCount.set(0); fakeClientCommunication(); - Assert.assertEquals(ITEM_COUNT, callCount.get()); + Assert.assertTrue(callCount.get() <= ITEM_COUNT); } private void initiallyHiddenColumn_toggleHiddenTwiceInRoundTrip_assertRendererNotCalled(