diff options
4 files changed, 113 insertions, 3 deletions
diff --git a/client/src/com/vaadin/client/widget/escalator/RowContainer.java b/client/src/com/vaadin/client/widget/escalator/RowContainer.java index ea56c25062..e2baf9a03b 100644 --- a/client/src/com/vaadin/client/widget/escalator/RowContainer.java +++ b/client/src/com/vaadin/client/widget/escalator/RowContainer.java @@ -50,6 +50,10 @@ public interface RowContainer { * <p> * If a spacer is already registered with the given row index, that * spacer will be updated with the given height. + * <p> + * <em>Note:</em> The row index for a spacer will change if rows are + * inserted or removed above the current position. Spacers will also be + * removed alongside their associated rows * * @param rowIndex * the row index for the spacer to modify. The affected @@ -59,6 +63,8 @@ public interface RowContainer { * negative, the affected spacer (if exists) will be removed * @throws IllegalArgumentException * if {@code rowIndex} is not a valid row index + * @see #insertRows(int, int) + * @see #removeRows(int, int) */ void setSpacer(int rowIndex, double height) throws IllegalArgumentException; @@ -88,6 +94,26 @@ public interface RowContainer { */ SpacerUpdater getSpacerUpdater(); + /** + * {@inheritDoc} + * <p> + * Any spacers underneath {@code index} will be offset and "pushed" + * down. This also modifies the row index they are associated with. + */ + @Override + public void insertRows(int index, int numberOfRows) + throws IndexOutOfBoundsException, IllegalArgumentException; + + /** + * {@inheritDoc} + * <p> + * Any spacers underneath {@code index} will be offset and "pulled" up. + * This also modifies the row index they are associated with. Any + * spacers in the removed range will also be closed and removed. + */ + @Override + public void removeRows(int index, int numberOfRows) + throws IndexOutOfBoundsException, IllegalArgumentException; } /** diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index b36b94c8f3..9d5526b14a 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -2925,6 +2925,12 @@ public class Escalator extends Widget implements RequiresResize, /* * Removing spacers as the very first step will correct the * scrollbars and row offsets right away. + * + * TODO: actually, it kinda sounds like a Grid feature that a spacer + * would be associated with a particular row. Maybe it would be + * better to have a spacer separate from rows, and simply collapse + * them if they happen to end up on top of each other. This would + * probably make supporting the -1 row pretty easy, too. */ spacerContainer.paintRemoveSpacers(removedRowsRange); @@ -3576,7 +3582,7 @@ public class Escalator extends Widget implements RequiresResize, // TODO getLogger().warning( - "[[spacers]] removing rows while shrinking the body " + "[[spacers]] removing spacers while shrinking the body " + "section"); final ListIterator<TableRowElement> iter = visualRowOrder @@ -4511,6 +4517,7 @@ public class Escalator extends Widget implements RequiresResize, SpacerImpl spacer = rowIndexToSpacer.remove(this.rowIndex); assert this == spacer : "trying to move an unexpected spacer."; this.rowIndex = rowIndex; + root.setPropertyInt(SPACER_LOGICAL_ROW_PROPERTY, rowIndex); rowIndexToSpacer.put(this.rowIndex, this); } } 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 c22da47d62..25c4c1ff67 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java @@ -70,6 +70,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest protected static final String COLSPAN_NONE = "Apply no colspan"; protected static final String SPACERS = "Spacers"; protected static final String ROW_1 = "Row 1"; + protected static final String ROW_99 = "Row 99"; protected static final String SET_100PX = "Set 100px"; protected static final String REMOVE = "Remove"; @@ -245,6 +246,11 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest getVerticalScrollbar().scroll(px); } + protected long getScrollTop() { + return ((Long) executeScript("return arguments[0].scrollTop;", + getVerticalScrollbar())).longValue(); + } + private TestBenchElement getVerticalScrollbar() { return (TestBenchElement) getEscalator().findElement( By.className("v-escalator-scroller-vertical")); @@ -254,6 +260,11 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest getHorizontalScrollbar().scrollLeft(px); } + private long getScrollLeft() { + return ((Long) executeScript("return arguments[0].scrollLeft;", + getHorizontalScrollbar())).longValue(); + } + private TestBenchElement getHorizontalScrollbar() { return (TestBenchElement) getEscalator().findElement( By.className("v-escalator-scroller-horizontal")); @@ -271,6 +282,11 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest return getEscalator().findElements(By.className("v-escalator-spacer")); } + protected boolean spacersAreFoundInDom() { + List<WebElement> spacers = getSpacers(); + return spacers != null && !spacers.isEmpty(); + } + @SuppressWarnings("boxing") protected WebElement getSpacer(int logicalRowIndex) { List<WebElement> spacers = getSpacers(); 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 1985de1c82..85acdc29cc 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 @@ -16,6 +16,7 @@ package com.vaadin.tests.components.grid.basicfeatures.escalator; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -28,6 +29,7 @@ import org.junit.Test; import org.openqa.selenium.WebElement; import com.vaadin.client.WidgetUtil; +import com.vaadin.testbench.elements.NotificationElement; import com.vaadin.tests.components.grid.basicfeatures.EscalatorBasicClientFeaturesTest; @SuppressWarnings("boxing") @@ -87,7 +89,8 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { @Test public void openVisibleSpacer() { - assertNull("No spacers should be shown at the start", getSpacer(1)); + assertFalse("No spacers should be shown at the start", + spacersAreFoundInDom()); selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); assertNotNull("Spacer should be shown after setting it", getSpacer(1)); } @@ -117,7 +120,7 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { 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)); + double newTop = getElementTop(getSpacer(2)); assertGreater("Spacer should've been pushed down", newTop, oldTop); } @@ -152,6 +155,64 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { spacerTop + 100, rowTop, WidgetUtil.PIXEL_EPSILON); } + @Test + public void addSpacerAtBottomThenScrollThere() { + selectMenuPath(FEATURES, SPACERS, ROW_99, SET_100PX); + scrollVerticallyTo(999999); + + assertFalse("Did not expect a notification", + $(NotificationElement.class).exists()); + } + + @Test + public void scrollToBottomThenAddSpacerThere() { + scrollVerticallyTo(999999); + long oldBottomScrollTop = getScrollTop(); + selectMenuPath(FEATURES, SPACERS, ROW_99, SET_100PX); + + assertEquals("Adding a spacer underneath the current viewport should " + + "not scroll anywhere", oldBottomScrollTop, getScrollTop()); + assertFalse("Got an unexpected notification", + $(NotificationElement.class).exists()); + + scrollVerticallyTo(999999); + + assertFalse("Got an unexpected notification", + $(NotificationElement.class).exists()); + assertGreater("Adding a spacer should've made the scrollbar scroll " + + "further", getScrollTop(), oldBottomScrollTop); + } + + @Test + public void removingRowAboveSpacerMovesSpacerUp() { + selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); + WebElement spacer = getSpacer(1); + double originalElementTop = getElementTop(spacer); + + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, + REMOVE_ONE_ROW_FROM_BEGINNING); + assertLessThan("spacer should've moved up", getElementTop(spacer), + originalElementTop); + assertNull("No spacer for row 1 should be found after removing the " + + "top row", getSpacer(1)); + } + + @Test + public void removingSpacedRowRemovesSpacer() { + selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); + assertTrue("Spacer should've been found in the DOM", + spacersAreFoundInDom()); + + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, + REMOVE_ONE_ROW_FROM_BEGINNING); + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, + REMOVE_ONE_ROW_FROM_BEGINNING); + + assertFalse("No spacers should be in the DOM after removing " + + "associated spacer", spacersAreFoundInDom()); + + } + private static double getElementTop(WebElement element) { /* * we need to parse the style attribute, since using getCssValue gets a |