Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Commit

Permalink
Don't apply new filter to DP if filter not changed (#231)
Browse files Browse the repository at this point in the history
Otherwise, DataProvider will get a new instance of the filter object
-> DataCommunicator thinks filter changed
-> destroys all data
-> rendered components are destroyed
-> client still thinks that everything is fine
-> flow-component-renderer doesn't find destroyed component instances
-> components not rendered

Fix #227
  • Loading branch information
pekam committed Mar 21, 2019
1 parent e9b6c4d commit 82deeae
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ protected void assertNotRendered(String innerHTML) {
matchingItem.isPresent());
}

protected void assertComponentRendered(String componentHtml) {
assertRendered("<flow-component-renderer appid=\"ROOT\">"
+ componentHtml + "</flow-component-renderer>");
}

// Gets the innerHTML of all the actually rendered item elements.
// There's more items loaded though.
protected List<String> getOverlayContents() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ public void loadItems_changeRenderer() {
beanBox.openPopup();
clickButton("component-renderer");
beanBox.openPopup();
assertRendered("<flow-component-renderer appid=\"ROOT\">"
+ "<h4>Person 4</h4></flow-component-renderer>");
assertComponentRendered("<h4>Person 4</h4>");
assertLoadedItemsCount("Only the first page should be loaded.", 50,
beanBox);
}
Expand All @@ -256,8 +255,7 @@ public void setItemLabelGenerator_setComponentRenderer_labelGeneratorUsedForText
clickButton("item-label-generator");
clickButton("component-renderer");
beanBox.openPopup();
assertRendered("<flow-component-renderer appid=\"ROOT\">"
+ "<h4>Person 4</h4></flow-component-renderer>");
assertComponentRendered("<h4>Person 4</h4>");
getItemElements().get(7).click();
Assert.assertEquals("Born 7", getTextFieldValue(beanBox));

Expand Down Expand Up @@ -409,6 +407,18 @@ public void filterAndSelectItemNotOnFirstPage_setCurrentValue_valueCorrect() {
Assert.assertEquals(item, getSelectedItemLabel(stringBox));
}

@Test // https://github.com/vaadin/vaadin-combo-box-flow/issues/227
public void setComponentRenderer_scrollDown_scrollUp_itemsRendered() {
clickButton("component-renderer");
beanBox.openPopup();
scrollToItem(beanBox, 300);
scrollToItem(beanBox, 0);

assertComponentRendered("<h4>Person 0</h4>");
assertComponentRendered("<h4>Person 4</h4>");
assertComponentRendered("<h4>Person 9</h4>");
}

private void assertMessage(String expectedMessage) {
Assert.assertEquals(expectedMessage, $("div").id("message").getText());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,6 @@ public void commit(int updateId) {
enqueue("$connector.confirm", updateId, ComboBox.this.lastFilter);
queue.forEach(Runnable::run);
queue.clear();

ComboBox.this.lastFilter = null;
}

private void enqueue(String name, Serializable... arguments) {
Expand Down Expand Up @@ -502,8 +500,12 @@ public <C> void setDataProvider(DataProvider<T, C> dataProvider,
.setDataProvider(dataProvider,
convertOrNull.apply(getFilterString()));

filterSlot = filter -> providerFilterSlot
.accept(convertOrNull.apply(filter));
filterSlot = filter -> {
if (!Objects.equals(filter, lastFilter)) {
providerFilterSlot.accept(convertOrNull.apply(filter));
lastFilter = filter;
}
};

boolean shouldForceServerSideFiltering = userProvidedFilter == UserProvidedFilter.YES;

Expand Down Expand Up @@ -938,8 +940,6 @@ private void confirmUpdate(int id) {

@ClientCallable
private void setRequestedRange(int start, int length, String filter) {
this.lastFilter = filter;

dataCommunicator.setRequestedRange(start, length);
filterSlot.accept(filter);
}
Expand Down

0 comments on commit 82deeae

Please sign in to comment.