diff options
7 files changed, 523 insertions, 13 deletions
diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 51e986933c..d17f378611 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -477,7 +477,15 @@ public class GridConnector extends AbstractHasComponentsConnector implements } @SuppressWarnings("boxing") - private class DetailsConnectorFetcher implements DeferredWorker { + private static class DetailsConnectorFetcher implements DeferredWorker { + + private static final int FETCH_TIMEOUT_MS = 5000; + + public interface Listener { + void fetchHasBeenScheduled(int id); + + void fetchHasReturned(int id); + } /** A flag making sure that we don't call scheduleFinally many times. */ private boolean fetcherHasBeenCalled = false; @@ -493,14 +501,26 @@ public class GridConnector extends AbstractHasComponentsConnector implements public void execute() { int currentFetchId = detailsFetchCounter++; pendingFetches.add(currentFetchId); - getRpcProxy(GridServerRpc.class).sendDetailsComponents( - currentFetchId); + rpc.sendDetailsComponents(currentFetchId); fetcherHasBeenCalled = false; + if (listener != null) { + listener.fetchHasBeenScheduled(currentFetchId); + } + assert assertRequestDoesNotTimeout(currentFetchId); } }; + private DetailsConnectorFetcher.Listener listener = null; + + private final GridServerRpc rpc; + + public DetailsConnectorFetcher(GridServerRpc rpc) { + assert rpc != null : "RPC was null"; + this.rpc = rpc; + } + public void schedule() { if (!fetcherHasBeenCalled) { Scheduler.get().scheduleFinally(lazyDetailsFetcher); @@ -509,10 +529,17 @@ public class GridConnector extends AbstractHasComponentsConnector implements } public void responseReceived(int fetchId) { - /* Ignore negative fetchIds (they're pushed, not fetched) */ - if (fetchId >= 0) { - boolean success = pendingFetches.remove(fetchId); - assert success : "Received a response with an unidentified fetch id"; + + if (fetchId < 0) { + /* Ignore negative fetchIds (they're pushed, not fetched) */ + return; + } + + boolean success = pendingFetches.remove(fetchId); + assert success : "Received a response with an unidentified fetch id"; + + if (listener != null) { + listener.fetchHasReturned(fetchId); } } @@ -534,9 +561,113 @@ public class GridConnector extends AbstractHasComponentsConnector implements assert !pendingFetches.contains(fetchId) : "Fetch id " + fetchId + " timed out."; } - }.schedule(1000); + }.schedule(FETCH_TIMEOUT_MS); return true; } + + public void setListener(DetailsConnectorFetcher.Listener listener) { + // if more are needed, feel free to convert this into a collection. + this.listener = listener; + } + } + + /** + * The functionality that makes sure that the scroll position is still kept + * up-to-date even if more details are being fetched lazily. + */ + private class LazyDetailsScrollAdjuster implements DeferredWorker { + + private static final int SCROLL_TO_END_ID = -2; + private static final int NO_SCROLL_SCHEDULED = -1; + + private class ScrollStopChecker implements DeferredWorker { + private final ScheduledCommand checkCommand = new ScheduledCommand() { + @Override + public void execute() { + isScheduled = false; + if (queuedFetches.isEmpty()) { + currentRow = NO_SCROLL_SCHEDULED; + destination = null; + } + } + }; + + private boolean isScheduled = false; + + public void schedule() { + if (isScheduled) { + return; + } + Scheduler.get().scheduleDeferred(checkCommand); + isScheduled = true; + } + + @Override + public boolean isWorkPending() { + return isScheduled; + } + } + + private DetailsConnectorFetcher.Listener fetcherListener = new DetailsConnectorFetcher.Listener() { + @Override + @SuppressWarnings("boxing") + public void fetchHasBeenScheduled(int id) { + if (currentRow != NO_SCROLL_SCHEDULED) { + queuedFetches.add(id); + } + } + + @Override + @SuppressWarnings("boxing") + public void fetchHasReturned(int id) { + if (currentRow == NO_SCROLL_SCHEDULED + || queuedFetches.isEmpty()) { + return; + } + + queuedFetches.remove(id); + if (currentRow == SCROLL_TO_END_ID) { + getWidget().scrollToEnd(); + } else { + getWidget().scrollToRow(currentRow, destination); + } + + /* + * Schedule a deferred call whether we should stop adjusting for + * scrolling. + * + * This is done deferredly just because we can't be absolutely + * certain whether this most recent scrolling won't cascade into + * further lazy details loading (perhaps deferredly). + */ + scrollStopChecker.schedule(); + } + }; + + private int currentRow = NO_SCROLL_SCHEDULED; + private final Set<Integer> queuedFetches = new HashSet<Integer>(); + private final ScrollStopChecker scrollStopChecker = new ScrollStopChecker(); + private ScrollDestination destination; + + public LazyDetailsScrollAdjuster() { + detailsConnectorFetcher.setListener(fetcherListener); + } + + public void adjustForEnd() { + currentRow = SCROLL_TO_END_ID; + } + + public void adjustFor(int row, ScrollDestination destination) { + currentRow = row; + this.destination = destination; + } + + @Override + public boolean isWorkPending() { + return currentRow != NO_SCROLL_SCHEDULED + || !queuedFetches.isEmpty() + || scrollStopChecker.isWorkPending(); + } } /** @@ -597,7 +728,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements private final CustomDetailsGenerator customDetailsGenerator = new CustomDetailsGenerator(); - private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher(); + private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher( + getRpcProxy(GridServerRpc.class)); private final DetailsListener detailsListener = new DetailsListener() { @Override @@ -618,6 +750,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements } }; + private final LazyDetailsScrollAdjuster lazyDetailsScrollAdjuster = new LazyDetailsScrollAdjuster(); + @Override @SuppressWarnings("unchecked") public Grid<JsonObject> getWidget() { @@ -637,6 +771,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements registerRpc(GridClientRpc.class, new GridClientRpc() { @Override public void scrollToStart() { + /* + * no need for lazyDetailsScrollAdjuster, because the start is + * always 0, won't change a bit. + */ Scheduler.get().scheduleFinally(new ScheduledCommand() { @Override public void execute() { @@ -647,6 +785,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public void scrollToEnd() { + lazyDetailsScrollAdjuster.adjustForEnd(); Scheduler.get().scheduleFinally(new ScheduledCommand() { @Override public void execute() { @@ -658,6 +797,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public void scrollToRow(final int row, final ScrollDestination destination) { + lazyDetailsScrollAdjuster.adjustFor(row, destination); Scheduler.get().scheduleFinally(new ScheduledCommand() { @Override public void execute() { @@ -1266,7 +1406,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public boolean isWorkPending() { - return detailsConnectorFetcher.isWorkPending(); + return detailsConnectorFetcher.isWorkPending() + || lazyDetailsScrollAdjuster.isWorkPending(); } public DetailsListener getDetailsListener() { diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index e02b4e9f83..6d9b8ee70a 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -2042,7 +2042,7 @@ public class Escalator extends Widget implements RequiresResize, TableCellElement cellOriginal = rowElement.getCells().getItem( colIndex); - if (cellIsPartOfSpan(cellOriginal)) { + if (cellOriginal == null || cellIsPartOfSpan(cellOriginal)) { continue; } diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 69fbb41fc2..2cb7ab352a 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -250,6 +250,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * * @since 7.5.0 * @author Vaadin Ltd + * @see DetailsGenerator#NULL */ public interface DetailsGenerator extends Serializable { @@ -3968,6 +3969,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Scrolls to a certain item, using {@link ScrollDestination#ANY}. + * <p> + * If the item has visible details, its size will also be taken into + * account. * * @param itemId * id of item to scroll to. @@ -3980,6 +3984,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Scrolls to a certain item, using user-specified scroll destination. + * <p> + * If the item has visible details, its size will also be taken into + * account. * * @param itemId * id of item to scroll to. diff --git a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java index d4707f9ba5..3c6d993482 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java @@ -28,7 +28,8 @@ import com.vaadin.shared.communication.ClientRpc; public interface GridClientRpc extends ClientRpc { /** - * Command client Grid to scroll to a specific data row. + * Command client Grid to scroll to a specific data row and its (optional) + * details. * * @param row * zero-based row index. If the row index is below zero or above diff --git a/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java new file mode 100644 index 0000000000..5659f01bdd --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java @@ -0,0 +1,133 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid; + +import com.vaadin.annotations.Theme; +import com.vaadin.data.Property.ValueChangeEvent; +import com.vaadin.data.Property.ValueChangeListener; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.util.Person; +import com.vaadin.tests.util.PersonContainer; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.CheckBox; +import com.vaadin.ui.Component; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.DetailsGenerator; +import com.vaadin.ui.Grid.RowReference; +import com.vaadin.ui.Grid.SelectionMode; +import com.vaadin.ui.Label; +import com.vaadin.ui.Layout; +import com.vaadin.ui.TextField; +import com.vaadin.ui.UI; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.themes.ValoTheme; + +@Theme(ValoTheme.THEME_NAME) +public class GridScrollToRowWithDetails extends UI { + + private final DetailsGenerator detailsGenerator = new DetailsGenerator() { + @Override + public Component getDetails(RowReference rowReference) { + Person person = (Person) rowReference.getItemId(); + Label label = new Label(person.getFirstName() + " " + + person.getLastName()); + label.setHeight("30px"); + return label; + } + }; + + private TextField numberTextField; + private Grid grid; + + @Override + protected void init(VaadinRequest request) { + + Layout layout = new VerticalLayout(); + + grid = new Grid(PersonContainer.createWithTestData(1000)); + grid.setSelectionMode(SelectionMode.NONE); + layout.addComponent(grid); + + final CheckBox checkbox = new CheckBox("Details generator"); + checkbox.addValueChangeListener(new ValueChangeListener() { + @Override + @SuppressWarnings("boxing") + public void valueChange(ValueChangeEvent event) { + if (checkbox.getValue()) { + grid.setDetailsGenerator(detailsGenerator); + } else { + grid.setDetailsGenerator(DetailsGenerator.NULL); + } + } + }); + layout.addComponent(checkbox); + + numberTextField = new TextField("Row"); + numberTextField.setImmediate(false); + layout.addComponent(numberTextField); + + layout.addComponent(new Button("Toggle", new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + toggle(); + } + })); + + layout.addComponent(new Button("Scroll to", new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + scrollTo(); + } + })); + + layout.addComponent(new Button("Toggle and scroll", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + toggle(); + scrollTo(); + } + })); + layout.addComponent(new Button("Scroll and toggle", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + scrollTo(); + toggle(); + } + })); + + setContent(layout); + } + + private void toggle() { + Object itemId = getItemId(); + boolean isVisible = grid.isDetailsVisible(itemId); + grid.setDetailsVisible(itemId, !isVisible); + } + + private void scrollTo() { + grid.scrollTo(getItemId()); + } + + private Object getItemId() { + int row = Integer.parseInt(numberTextField.getValue()); + Object itemId = grid.getContainerDataSource().getIdByIndex(row); + return itemId; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java new file mode 100644 index 0000000000..b6ecd3f6e2 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java @@ -0,0 +1,224 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid; + +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.JavascriptExecutor; +import org.openqa.selenium.WebElement; + +import com.vaadin.shared.ui.grid.Range; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.CheckBoxElement; +import com.vaadin.testbench.elements.TextFieldElement; +import com.vaadin.tests.components.grid.basicfeatures.element.CustomGridElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridScrollToRowWithDetailsTest extends MultiBrowserTest { + + private static class Param { + private final int rowIndex; + private final boolean useGenerator; + private final boolean scrollFirstToBottom; + private final int scrollTarget; + + public Param(int rowIndex, boolean useGenerator, + boolean scrollFirstToBottom, int scrollTarget) { + this.rowIndex = rowIndex; + this.useGenerator = useGenerator; + this.scrollFirstToBottom = scrollFirstToBottom; + this.scrollTarget = Math.max(0, scrollTarget); + } + + public int getRowIndex() { + return rowIndex; + } + + public boolean useGenerator() { + return useGenerator; + } + + public boolean scrollFirstToBottom() { + return scrollFirstToBottom; + } + + public int getScrollTarget() { + return scrollTarget; + } + + @Override + public String toString() { + return "Param [rowIndex=" + getRowIndex() + ", useGenerator=" + + useGenerator() + ", scrollFirstToBottom=" + + scrollFirstToBottom() + ", scrollTarget=" + + getScrollTarget() + "]"; + } + } + + public static Collection<Param> parameters() { + List<Param> data = new ArrayList<Param>(); + + int[][] params = new int[][] {// @formatter:off + // row, top+noGen, top+gen, bot+noGen, bot+gen + { 0, 0, 0, 0, 0 }, + { 500, 18741, 18723, 19000, 19000 }, + { 999, 37703, 37685, 37703, 37685 }, + }; + // @formatter:on + + for (int i[] : params) { + int rowIndex = i[0]; + int targetTopScrollWithoutGenerator = i[1]; + int targetTopScrollWithGenerator = i[2]; + int targetBottomScrollWithoutGenerator = i[3]; + int targetBottomScrollWithGenerator = i[4]; + + data.add(new Param(rowIndex, false, false, + targetTopScrollWithoutGenerator)); + data.add(new Param(rowIndex, true, false, + targetTopScrollWithGenerator)); + data.add(new Param(rowIndex, false, true, + targetBottomScrollWithoutGenerator)); + data.add(new Param(rowIndex, true, true, + targetBottomScrollWithGenerator)); + } + + return data; + } + + @Before + public void setUp() { + setDebug(true); + } + + @Test + public void toggleAndScroll() throws Throwable { + for (Param param : parameters()) { + try { + openTestURL(); + useGenerator(param.useGenerator()); + scrollToBottom(param.scrollFirstToBottom()); + + // the tested method + toggleAndScroll(param.getRowIndex()); + + Range allowedRange = Range.withLength( + param.getScrollTarget() - 5, 10); + assertTrue( + allowedRange + " does not contain " + getScrollTop(), + allowedRange.contains(getScrollTop())); + } catch (Throwable t) { + throw new Throwable("" + param, t); + } + } + } + + @Test + public void scrollAndToggle() throws Throwable { + for (Param param : parameters()) { + try { + openTestURL(); + useGenerator(param.useGenerator()); + scrollToBottom(param.scrollFirstToBottom()); + + // the tested method + scrollAndToggle(param.getRowIndex()); + + Range allowedRange = Range.withLength( + param.getScrollTarget() - 5, 10); + assertTrue( + allowedRange + " does not contain " + getScrollTop(), + allowedRange.contains(getScrollTop())); + } catch (Throwable t) { + throw new Throwable("" + param, t); + } + } + } + + private void scrollToBottom(boolean scrollFirstToBottom) { + if (scrollFirstToBottom) { + executeScript("arguments[0].scrollTop = 9999999", + getVerticalScrollbar()); + } + } + + private void useGenerator(boolean use) { + CheckBoxElement checkBox = $(CheckBoxElement.class).first(); + boolean isChecked = isCheckedValo(checkBox); + if (use != isChecked) { + clickValo(checkBox); + } + } + + @SuppressWarnings("boxing") + private boolean isCheckedValo(CheckBoxElement checkBoxElement) { + WebElement checkbox = checkBoxElement.findElement(By.tagName("input")); + Object value = executeScript("return arguments[0].checked;", checkbox); + return (Boolean) value; + } + + private void clickValo(CheckBoxElement checkBoxElement) { + checkBoxElement.findElement(By.tagName("label")).click(); + } + + private Object executeScript(String string, Object... param) { + return ((JavascriptExecutor) getDriver()).executeScript(string, param); + } + + private void scrollAndToggle(int row) { + setRow(row); + getScrollAndToggle().click(); + } + + private void toggleAndScroll(int row) { + setRow(row); + getToggleAndScroll().click(); + } + + private ButtonElement getScrollAndToggle() { + return $(ButtonElement.class).caption("Scroll and toggle").first(); + } + + private ButtonElement getToggleAndScroll() { + return $(ButtonElement.class).caption("Toggle and scroll").first(); + } + + private void setRow(int row) { + $(TextFieldElement.class).first().setValue(String.valueOf(row)); + } + + private CustomGridElement getGrid() { + return $(CustomGridElement.class).first(); + } + + private int getScrollTop() { + return ((Long) executeScript("return arguments[0].scrollTop;", + getVerticalScrollbar())).intValue(); + } + + private WebElement getVerticalScrollbar() { + WebElement scrollBar = getGrid().findElement( + By.className("v-grid-scroller-vertical")); + return scrollBar; + } +} diff --git a/uitest/src/com/vaadin/tests/util/PersonContainer.java b/uitest/src/com/vaadin/tests/util/PersonContainer.java index 611e5d3adb..709086be29 100644 --- a/uitest/src/com/vaadin/tests/util/PersonContainer.java +++ b/uitest/src/com/vaadin/tests/util/PersonContainer.java @@ -32,10 +32,14 @@ public class PersonContainer extends BeanItemContainer<Person> implements } public static PersonContainer createWithTestData() { + return createWithTestData(100); + } + + public static PersonContainer createWithTestData(int size) { PersonContainer c = null; Random r = new Random(0); c = new PersonContainer(); - for (int i = 0; i < 100; i++) { + for (int i = 0; i < size; i++) { Person p = new Person(); p.setFirstName(TestDataGenerator.getFirstName(r)); p.setLastName(TestDataGenerator.getLastName(r)); |