Skip to content

Commit

Permalink
refactor: patch data generator instead of scheduling renderer
Browse files Browse the repository at this point in the history
  • Loading branch information
@ugur-vaadin
ugur-vaadin committed Nov 12, 2024
1 parent 4128359 commit 9959a86
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 70 deletions.
125 changes: 63 additions & 62 deletions ...-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,6 @@ public static class Column<T> extends AbstractColumn<Column<T>> {
private String columnKey; // defined and used by the user

private boolean sortingEnabled;
private boolean rendererSetupScheduled;

private Component editorComponent;
private EditorRenderer<T> editorRenderer;
Expand Down Expand Up @@ -485,8 +484,38 @@ public Column(Grid<T> grid, String columnId, Renderer<T> renderer) {
super(grid);
Objects.requireNonNull(renderer);
this.columnInternalId = columnId;
this.renderer = renderer;

comparator = (a, b) -> 0;
setRenderer(renderer);

rendering = renderer.render(getElement(), (KeyMapper<T>) getGrid()
.getDataCommunicator().getKeyMapper());

Optional<DataGenerator<T>> dataGenerator = rendering
.getDataGenerator();

if (dataGenerator.isPresent()) {
var generator = dataGenerator.get();
DataGenerator<T> 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() {
Expand Down Expand Up @@ -530,10 +559,33 @@ public Renderer<T> getRenderer() {
public Column<T> setRenderer(Renderer<T> 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<T>) 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<T> editorRendering = editorRenderer
.render(getElement(), null);
editorDataGeneratorRegistration = editorRendering
.getDataGenerator()
.map(dataGenerator -> grid.addDataGenerator(
(DataGenerator) dataGenerator))
.orElse(null);
}

getGrid().getDataCommunicator().reset();
return this;
}

Expand Down Expand Up @@ -1133,18 +1185,6 @@ public Column<T> 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;
Expand All @@ -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<T>) 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<T> 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<T>> dataGenerator = editorRendering
.getDataGenerator();
if (dataGenerator.isPresent()) {
editorDataGeneratorRegistration = grid
.addDataGenerator((DataGenerator) dataGenerator.get());
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions ...grid-flow/src/test/java/com/vaadin/flow/component/grid/GridHiddenColumnRenderingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ public void initiallyHiddenColumnWithValueProvider_setVisible_rendererCalledOnce
}

@Test
public void columnWithValueProvider_toggleHiddenTwiceInRoundTrip_rendererCalledOncePerItem() {
public void columnWithValueProvider_toggleHiddenTwiceInRoundTrip_rendererCalledAtMostOncePerItem() {
Grid.Column<String> column = addColumnWithValueProvider();
initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem(
initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem(
column);
}

Expand Down Expand Up @@ -128,9 +128,9 @@ public void initiallyHiddenComponentColumn_setVisible_rendererCalledOncePerItem(
}

@Test
public void componentColumn_toggleHiddenTwiceInRoundTrip_rendererCalledOncePerItem() {
public void componentColumn_toggleHiddenTwiceInRoundTrip_rendererCalledAtMostOncePerItem() {
Grid.Column<String> column = addComponentColumn();
initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem(
initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem(
column);
}

Expand Down Expand Up @@ -167,9 +167,9 @@ public void initiallyHiddenColumnWithCustomRenderer_setVisible_rendererCalledOnc
}

@Test
public void columnWithCustomRenderer_toggleHiddenTwiceInRoundTrip_rendererCalledOncePerItem() {
public void columnWithCustomRenderer_toggleHiddenTwiceInRoundTrip_rendererCalledAtMostOncePerItem() {
Grid.Column<String> column = addColumnWithCustomRenderer();
initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledOncePerItem(
initiallyVisibleColumn_toggleHiddenTwiceInRoundTrip_assertRendererCalledAtMostOncePerItem(
column);
}

Expand Down Expand Up @@ -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<String> 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(
Expand Down

0 comments on commit 9959a86

Please sign in to comment.