diff options
3 files changed, 152 insertions, 58 deletions
diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index 30ecf64ff4..0df078d26e 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -820,7 +820,8 @@ public class Escalator extends Widget implements RequiresResize, * that the sizes of the scroll handles appear correct in the browser */ public void recalculateScrollbarsForVirtualViewport() { - double scrollContentHeight = body.calculateTotalRowHeight(); + double scrollContentHeight = body.calculateTotalRowHeight() + + body.spacerContainer.getSpacerHeightsSum(); double scrollContentWidth = columnConfiguration.calculateRowWidth(); double tableWrapperHeight = heightOfEscalator; @@ -2420,6 +2421,13 @@ public class Escalator extends Widget implements RequiresResize, return; } + /* + * TODO This will break the logical index calculation, as it will + * try to search for non- + */ + getLogger().warning( + "[[spacers]] scrolling and spacers near the bottom"); + boolean rowsWereMoved = false; final double topElementPosition; @@ -2587,6 +2595,13 @@ public class Escalator extends Widget implements RequiresResize, return; } + final double pxDiff = numberOfRows * getDefaultRowHeight(); + for (SpacerContainer.SpacerImpl spacer : spacerContainer + .getSpacersForRowAndAfter(index)) { + spacer.setPositionDiff(0, pxDiff); + spacer.setRowIndex(spacer.getRow() + numberOfRows); + } + /* * TODO: this method should probably only add physical rows, and not * populate them - let everything be populated as appropriate by the @@ -2632,13 +2647,6 @@ public class Escalator extends Widget implements RequiresResize, final int visualOffset = getLogicalRowIndex(visualRowOrder .getFirst()); - final double pxDiff = numberOfRows * getDefaultRowHeight(); - for (SpacerContainer.SpacerImpl spacer : spacerContainer - .getSpacersForRowAndAfter(index)) { - spacer.setPositionDiff(0, pxDiff); - spacer.setRowIndex(spacer.getRow() + numberOfRows); - } - /* * At this point, we have added new escalator rows, if so * needed. @@ -2648,33 +2656,33 @@ public class Escalator extends Widget implements RequiresResize, * the remaining rows aswell. */ final int rowsStillNeeded = numberOfRows - addedRows.size(); - final Range unupdatedVisual = convertToVisual(Range.withLength( - unupdatedLogicalStart, rowsStillNeeded)); - final int end = getEscalatorRowCount(); - final int start = end - unupdatedVisual.length(); - final int visualTargetIndex = unupdatedLogicalStart - - visualOffset; - moveAndUpdateEscalatorRows(Range.between(start, end), - visualTargetIndex, unupdatedLogicalStart); - - // move the surrounding rows to their correct places. - double rowTop = (unupdatedLogicalStart + (end - start)) - * getDefaultRowHeight(); - final ListIterator<TableRowElement> i = visualRowOrder - .listIterator(visualTargetIndex + (end - start)); - - int logicalRowIndexCursor = unupdatedLogicalStart; - while (i.hasNext()) { - SpacerContainer.SpacerImpl spacer = spacerContainer - .getSpacer(logicalRowIndexCursor); - if (spacer != null) { - rowTop += spacer.getHeight(); - } - logicalRowIndexCursor++; - final TableRowElement tr = i.next(); - setRowPosition(tr, 0, rowTop); - rowTop += getDefaultRowHeight(); + if (rowsStillNeeded > 0) { + final Range unupdatedVisual = convertToVisual(Range + .withLength(unupdatedLogicalStart, rowsStillNeeded)); + final int end = getEscalatorRowCount(); + final int start = end - unupdatedVisual.length(); + final int visualTargetIndex = unupdatedLogicalStart + - visualOffset; + moveAndUpdateEscalatorRows(Range.between(start, end), + visualTargetIndex, unupdatedLogicalStart); + + // move the surrounding rows to their correct places. + double rowTop = (unupdatedLogicalStart + (end - start)) + * getDefaultRowHeight(); + final ListIterator<TableRowElement> i = visualRowOrder + .listIterator(visualTargetIndex + (end - start)); + + int logicalRowIndexCursor = unupdatedLogicalStart; + while (i.hasNext()) { + rowTop += spacerContainer + .getSpacerHeight(logicalRowIndexCursor++); + + final TableRowElement tr = i.next(); + getLogger().warning("iterate"); + setRowPosition(tr, 0, rowTop); + rowTop += getDefaultRowHeight(); + } } fireRowVisibilityChangeEvent(); @@ -2786,11 +2794,8 @@ public class Escalator extends Widget implements RequiresResize, setRowPosition(tr, 0, newRowTop); newRowTop += getDefaultRowHeight(); - SpacerContainer.SpacerImpl spacer = spacerContainer - .getSpacer(logicalTargetIndex + i); - if (spacer != null) { - newRowTop += spacer.getHeight(); - } + newRowTop += spacerContainer + .getSpacerHeight(logicalTargetIndex + i); } } } @@ -2879,25 +2884,29 @@ public class Escalator extends Widget implements RequiresResize, index, escalatorRowsNeeded); visualRowOrder.addAll(index, addedRows); - /* - * We need to figure out the top positions for the rows we just - * added. - */ - for (int i = 0; i < addedRows.size(); i++) { - setRowPosition(addedRows.get(i), 0, (index + i) - * getDefaultRowHeight()); - } + double y = index * getDefaultRowHeight() + + spacerContainer.getSpacerHeightsSumUntilIndex(index); + for (int i = index; i < visualRowOrder.size(); i++) { + + final TableRowElement tr; + if (i - index < addedRows.size()) { + tr = addedRows.get(i - index); + } else { + tr = visualRowOrder.get(i); + } - /* Move the other rows away from above the added escalator rows */ - for (int i = index + addedRows.size(); i < visualRowOrder - .size(); i++) { - final TableRowElement tr = visualRowOrder.get(i); - setRowPosition(tr, 0, i * getDefaultRowHeight()); + getLogger().warning("y: " + y + ", index: " + i); + setRowPosition(tr, 0, y); + y += getDefaultRowHeight(); + double spacerHeight = spacerContainer.getSpacerHeight(i); + getLogger().warning( + "height: " + spacerHeight + ", index: " + i); + y += spacerHeight; } return addedRows; } else { - return new ArrayList<TableRowElement>(); + return Collections.emptyList(); } } @@ -4519,6 +4528,23 @@ public class Escalator extends Widget implements RequiresResize, } } + /** + * Calculates the sum of all spacers. + * + * @return sum of all spacers, or 0 if no spacers present + */ + public double getSpacerHeightsSum() { + return getHeights(rowIndexToSpacer.values()); + } + + /** + * Calculates the sum of all spacers from one row index onwards. + * + * @param logicalRowIndex + * the spacer to include as the first calculated spacer + * @return the sum of all spacers from {@code logicalRowIndex} and + * onwards, or 0 if no suitable spacers were found + */ @SuppressWarnings("boxing") public Collection<SpacerImpl> getSpacersForRowAndAfter( int logicalRowIndex) { @@ -4738,14 +4764,35 @@ public class Escalator extends Widget implements RequiresResize, */ @SuppressWarnings("boxing") public double getSpacerHeightsSumUntilIndex(int logicalIndex) { + return getHeights(rowIndexToSpacer.headMap(logicalIndex, false) + .values()); + } + + private double getHeights(Collection<SpacerImpl> spacers) { double heights = 0; - for (SpacerImpl spacer : rowIndexToSpacer.headMap(logicalIndex, - false).values()) { + for (SpacerImpl spacer : spacers) { heights += spacer.getHeight(); } return heights; } + /** + * Gets the height of the spacer for a row index. + * + * @param rowIndex + * the index of the row where the spacer should be + * @return the height of the spacer at index {@code rowIndex}, or 0 if + * there is no spacer there + */ + public double getSpacerHeight(int rowIndex) { + SpacerImpl spacer = getSpacer(rowIndex); + if (spacer != null) { + return spacer.getHeight(); + } else { + return 0; + } + } + private boolean spacerExists(int rowIndex) { return rowIndexToSpacer.containsKey(Integer.valueOf(rowIndex)); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java index ef2d605928..c22da47d62 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java @@ -38,8 +38,10 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest protected static final String COLUMNS = "Columns"; protected static final String ADD_ONE_COLUMN_TO_BEGINNING = "Add one column to beginning"; protected static final String ADD_ONE_ROW_TO_BEGINNING = "Add one row to beginning"; + protected static final String ADD_ONE_ROW_TO_END = "Add one row to end"; protected static final String REMOVE_ONE_COLUMN_FROM_BEGINNING = "Remove one column from beginning"; protected static final String REMOVE_ONE_ROW_FROM_BEGINNING = "Remove one row from beginning"; + protected static final String REMOVE_ALL_ROWS = "Remove all rows"; protected static final String REMOVE_50_ROWS_FROM_BOTTOM = "Remove 50 rows from bottom"; protected static final String REMOVE_50_ROWS_FROM_ALMOST_BOTTOM = "Remove 50 rows from almost bottom"; protected static final String ADD_ONE_OF_EACH_ROW = "Add one of each row"; diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java index 43a18b507d..1985de1c82 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java @@ -25,10 +25,12 @@ import java.util.regex.Pattern; import org.junit.Before; import org.junit.Test; +import org.openqa.selenium.WebElement; -import com.vaadin.testbench.TestBenchElement; +import com.vaadin.client.WidgetUtil; import com.vaadin.tests.components.grid.basicfeatures.EscalatorBasicClientFeaturesTest; +@SuppressWarnings("boxing") public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { //@formatter:off @@ -98,7 +100,6 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { } @Test - @SuppressWarnings("boxing") public void spacerPushesVisibleRowsDown() { double oldTop = getElementTop(getBodyRow(2)); selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); @@ -107,7 +108,51 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { assertGreater("Row below a spacer was not pushed down", newTop, oldTop); } - private static double getElementTop(TestBenchElement element) { + @Test + public void addingRowAboveSpacerPushesItDown() { + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, REMOVE_ALL_ROWS); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + + selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); + double oldTop = getElementTop(getSpacer(1)); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + double newTop = getElementTop(getSpacer(1)); + + assertGreater("Spacer should've been pushed down", newTop, oldTop); + } + + @Test + public void addingRowBelowSpacerDoesNotPushItDown() { + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, REMOVE_ALL_ROWS); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + + selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); + double oldTop = getElementTop(getSpacer(1)); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_END); + double newTop = getElementTop(getSpacer(1)); + + assertEquals("Spacer should've not been pushed down", newTop, oldTop, + WidgetUtil.PIXEL_EPSILON); + } + + @Test + public void addingRowBelowSpacerIsActuallyRenderedBelowWhenEscalatorIsEmpty() { + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, REMOVE_ALL_ROWS); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_BEGINNING); + + selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); + double spacerTop = getElementTop(getSpacer(1)); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, ADD_ONE_ROW_TO_END); + double rowTop = getElementTop(getBodyRow(2)); + + assertEquals("Next row should've been rendered below the spacer", + spacerTop + 100, rowTop, WidgetUtil.PIXEL_EPSILON); + } + + private static double getElementTop(WebElement element) { /* * we need to parse the style attribute, since using getCssValue gets a * normalized value that is harder to parse. |