diff options
5 files changed, 91 insertions, 19 deletions
diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java index 6799c0bd23..c6aa7c6291 100644 --- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java +++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java @@ -467,19 +467,25 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { size -= count; // shift indices to fill the cache correctly - for (int i = firstRowIndex + count; i < cached.getEnd(); i++) { + int firstMoved = Math.max(firstRowIndex + count, cached.getStart()); + for (int i = firstMoved; i < cached.getEnd(); i++) { moveRowFromIndexToIndex(i, i - count); } Range removedRange = Range.withLength(firstRowIndex, count); if (cached.isSubsetOf(removedRange)) { + // Whole cache is part of the removal. Empty cache cached = Range.withLength(0, 0); } else if (removedRange.intersects(cached)) { + // Removal and cache share some indices. fix accordingly. Range[] partitions = cached.partitionWith(removedRange); Range remainsBefore = partitions[0]; Range transposedRemainsAfter = partitions[2].offsetBy(-removedRange .length()); cached = remainsBefore.combineWith(transposedRemainsAfter); + } else if (removedRange.getEnd() <= cached.getStart()) { + // Removal was before the cache. offset the cache. + cached = cached.offsetBy(-removedRange.length()); } assertDataChangeHandlerIsInjected(); @@ -502,7 +508,7 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { size += count; - if (firstRowIndex < cached.getStart()) { + if (firstRowIndex <= cached.getStart()) { Range oldCached = cached; cached = cached.offsetBy(count); @@ -539,7 +545,10 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { T row = indexToRowMap.remove(oldIndex); if (indexToRowMap.containsKey(newIndex)) { // Old row is about to be overwritten. Remove it from keyCache. - keyToIndexMap.remove(getRowKey(indexToRowMap.get(newIndex))); + T row2 = indexToRowMap.remove(newIndex); + if (row2 != null) { + keyToIndexMap.remove(getRowKey(row2)); + } } indexToRowMap.put(newIndex, row); if (row != null) { @@ -580,6 +589,7 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { private Range getMinCacheRange() { Range availableDataRange = getAvailableRangeForCache(); + Range minCacheRange = cacheStrategy.getMinCacheRange( requestedAvailability, cached, availableDataRange); diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 50a41cd324..63070dcd92 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -1780,6 +1780,7 @@ public class Grid<T> extends ResizeComposite implements boolean insertionIsAboveFocusedCell = (added.getStart() <= rowWithFocus); if (bodyHasFocus && insertionIsAboveFocusedCell) { rowWithFocus += added.length(); + refreshRow(rowWithFocus); } } @@ -1792,32 +1793,29 @@ public class Grid<T> extends ResizeComposite implements * a range of removed rows */ public void rowsRemovedFromBody(Range removed) { - int focusedColumn = cellFocusRange.getStart(); if (containerWithFocus != escalator.getBody()) { return; } else if (!removed.contains(rowWithFocus)) { if (removed.getStart() > rowWithFocus) { return; } - setCellFocus(rowWithFocus - removed.length(), focusedColumn, - containerWithFocus); + rowWithFocus = rowWithFocus - removed.length(); } else { if (containerWithFocus.getRowCount() > removed.getEnd()) { - setCellFocus(removed.getStart(), focusedColumn, - containerWithFocus); + rowWithFocus = removed.getStart(); } else if (removed.getStart() > 0) { - setCellFocus(removed.getStart() - 1, focusedColumn, - containerWithFocus); + rowWithFocus = removed.getStart() - 1; } else { if (escalator.getHeader().getRowCount() > 0) { - setCellFocus(lastFocusedHeaderRow, focusedColumn, - escalator.getHeader()); + rowWithFocus = lastFocusedHeaderRow; + containerWithFocus = escalator.getHeader(); } else if (escalator.getFooter().getRowCount() > 0) { - setCellFocus(lastFocusedFooterRow, focusedColumn, - escalator.getFooter()); + rowWithFocus = lastFocusedFooterRow; + containerWithFocus = escalator.getFooter(); } } } + refreshRow(rowWithFocus); } } diff --git a/shared/src/com/vaadin/shared/ui/grid/Range.java b/shared/src/com/vaadin/shared/ui/grid/Range.java index 2054845320..6be9e04cbc 100644 --- a/shared/src/com/vaadin/shared/ui/grid/Range.java +++ b/shared/src/com/vaadin/shared/ui/grid/Range.java @@ -177,6 +177,10 @@ public final class Range implements Serializable { * range */ public boolean isSubsetOf(final Range other) { + if (isEmpty() && other.isEmpty()) { + return true; + } + return other.getStart() <= getStart() && getEnd() <= other.getEnd(); } @@ -411,8 +415,10 @@ public final class Range implements Serializable { * @return a bounded range */ public Range restrictTo(Range bounds) { - boolean startWithin = getStart() >= bounds.getStart(); - boolean endWithin = getEnd() <= bounds.getEnd(); + boolean startWithin = bounds.contains(getStart()); + boolean endWithin = bounds.contains(getEnd()); + boolean boundsWithin = getStart() < bounds.getStart() + && getEnd() >= bounds.getEnd(); if (startWithin) { if (endWithin) { @@ -423,8 +429,10 @@ public final class Range implements Serializable { } else { if (endWithin) { return Range.between(bounds.getStart(), getEnd()); - } else { + } else if (boundsWithin) { return bounds; + } else { + return Range.withLength(getStart(), 0); } } } diff --git a/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java b/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java index ab67b22d0b..e3cae858ee 100644 --- a/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java +++ b/shared/tests/src/com/vaadin/shared/ui/grid/RangeTest.java @@ -372,6 +372,7 @@ public class RangeTest { assertTrue(r2 == r3); } + @Test public void restrictTo_notInterstecting() { Range r1 = Range.between(5, 10); Range r2 = Range.between(15, 20); @@ -385,6 +386,7 @@ public class RangeTest { r4.isEmpty()); } + @Test public void restrictTo_startOutside() { Range r1 = Range.between(5, 10); Range r2 = Range.between(7, 15); @@ -392,8 +394,11 @@ public class RangeTest { Range r3 = r1.restrictTo(r2); assertEquals(Range.between(7, 10), r3); + + assertEquals(r2.restrictTo(r1), r3); } + @Test public void restrictTo_endOutside() { Range r1 = Range.between(5, 10); Range r2 = Range.between(4, 7); @@ -401,6 +406,20 @@ public class RangeTest { Range r3 = r1.restrictTo(r2); assertEquals(Range.between(5, 7), r3); + + assertEquals(r2.restrictTo(r1), r3); + } + + @Test + public void restrictTo_empty() { + Range r1 = Range.between(5, 10); + Range r2 = Range.between(0, 0); + + Range r3 = r1.restrictTo(r2); + assertTrue(r3.isEmpty()); + + Range r4 = r2.restrictTo(r1); + assertTrue(r4.isEmpty()); } } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java index 337293d687..9e1a9d5e91 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java @@ -26,10 +26,10 @@ import static org.junit.Assert.fail; import java.util.List; import org.junit.Test; -import org.openqa.selenium.By; import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; +import com.vaadin.testbench.By; import com.vaadin.testbench.TestBenchElement; import com.vaadin.testbench.elements.GridElement; import com.vaadin.testbench.elements.GridElement.GridCellElement; @@ -188,7 +188,8 @@ public class GridStructureTest extends GridBasicFeaturesTest { public void testItemSetChangeEvent() throws Exception { openTestURL(); - final By newRow = By.xpath("//td[text()='newcell: 0']"); + final org.openqa.selenium.By newRow = By + .xpath("//td[text()='newcell: 0']"); assertTrue("Unexpected initial state", !isElementPresent(newRow)); @@ -382,6 +383,42 @@ public class GridStructureTest extends GridBasicFeaturesTest { assertEquals("Grid scrolled unexpectedly", cellContent, cell.getText()); } + @Test + public void testRemoveAndAddRowAboveViewport() { + setDebug(true); + openTestURL(); + + GridCellElement cell = getGridElement().getCell(500, 1); + String cellContent = cell.getText(); + selectMenuPath("Component", "Body rows", "Remove first row"); + + assertFalse("Error notification was present after removing row", + isElementPresent(NotificationElement.class)); + + assertEquals("Grid scrolled unexpectedly", cellContent, cell.getText()); + + selectMenuPath("Component", "Body rows", "Add first row"); + + assertFalse("Error notification was present after adding row", + isElementPresent(NotificationElement.class)); + + assertEquals("Grid scrolled unexpectedly", cellContent, cell.getText()); + } + + @Test + public void testScrollAndRemoveAll() { + setDebug(true); + openTestURL(); + + getGridElement().scrollToRow(500); + selectMenuPath("Component", "Body rows", "Remove all rows"); + + assertFalse("Error notification was present after removing all rows", + isElementPresent(NotificationElement.class)); + + assertFalse(getGridElement().isElementPresent(By.vaadin("#cell[0][0]"))); + } + private void assertPrimaryStylename(String stylename) { assertTrue(getGridElement().getAttribute("class").contains(stylename)); |