diff options
6 files changed, 337 insertions, 60 deletions
diff --git a/src/com/vaadin/terminal/gwt/client/ui/treetable/VTreeTable.java b/src/com/vaadin/terminal/gwt/client/ui/treetable/VTreeTable.java index f7cd9d133e..9a8e0e9ce1 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/treetable/VTreeTable.java +++ b/src/com/vaadin/terminal/gwt/client/ui/treetable/VTreeTable.java @@ -379,26 +379,34 @@ public class VTreeTable extends VScrollTable { rowsToDelete.add(row); } } - RowCollapseAnimation anim = new RowCollapseAnimation(rowsToDelete) { - @Override - protected void onComplete() { - super.onComplete(); - // Actually unlink the rows and update the cache after the - // animation is done. - unlinkAndReindexRows(firstIndex, rows); - discardRowsOutsideCacheWindow(); - ensureCacheFilled(); - } - }; - anim.run(150); + if (!rowsToDelete.isEmpty()) { + // #8810 Only animate if there's something to animate + RowCollapseAnimation anim = new RowCollapseAnimation( + rowsToDelete) { + @Override + protected void onComplete() { + super.onComplete(); + // Actually unlink the rows and update the cache after + // the + // animation is done. + unlinkAndReindexRows(firstIndex, rows); + discardRowsOutsideCacheWindow(); + ensureCacheFilled(); + } + }; + anim.run(150); + } } protected List<VScrollTableRow> insertRowsAnimated(UIDL rowData, int firstIndex, int rows) { List<VScrollTableRow> insertedRows = insertAndReindexRows(rowData, firstIndex, rows); - RowExpandAnimation anim = new RowExpandAnimation(insertedRows); - anim.run(150); + if (!insertedRows.isEmpty()) { + // Only animate if there's something to animate (#8810) + RowExpandAnimation anim = new RowExpandAnimation(insertedRows); + anim.run(150); + } return insertedRows; } @@ -521,6 +529,10 @@ public class VTreeTable extends VScrollTable { private Element cloneTable; private AnimationPreparator preparator; + /** + * @param rows + * List of rows to animate. Must not be empty. + */ public RowExpandAnimation(List<VScrollTableRow> rows) { this.rows = rows; buildAndInsertAnimatingDiv(); @@ -641,6 +653,10 @@ public class VTreeTable extends VScrollTable { private final List<VScrollTableRow> rows; + /** + * @param rows + * List of rows to animate. Must not be empty. + */ public RowCollapseAnimation(List<VScrollTableRow> rows) { super(rows); this.rows = rows; diff --git a/src/com/vaadin/ui/TabSheet.java b/src/com/vaadin/ui/TabSheet.java index 23dee15359..7aef4a2b2a 100644 --- a/src/com/vaadin/ui/TabSheet.java +++ b/src/com/vaadin/ui/TabSheet.java @@ -108,6 +108,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, setWidth(100, UNITS_PERCENTAGE); setImmediate(true); setCloseHandler(new CloseHandler() { + @Override public void onTabClose(TabSheet tabsheet, Component c) { tabsheet.removeComponent(c); } @@ -120,6 +121,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, * * @return the unmodifiable Iterator of the tab content components */ + @Override public Iterator<Component> getComponentIterator() { return Collections.unmodifiableList(components).iterator(); } @@ -130,6 +132,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, * * @return the number of contained components */ + @Override public int getComponentCount() { return components.size(); } @@ -359,6 +362,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, * @throws PaintException * if the paint operation failed. */ + @Override public void paintContent(PaintTarget target) throws PaintException { if (areTabsHidden()) { @@ -683,6 +687,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, } // inherits javadoc + @Override public void changeVariables(Object source, Map<String, Object> variables) { if (variables.containsKey("selected")) { setSelectedTab(keyMapper.get((String) variables.get("selected"))); @@ -719,6 +724,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, * * {@inheritDoc} */ + @Override public void replaceComponent(Component oldComponent, Component newComponent) { if (selected == oldComponent) { @@ -729,25 +735,6 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, Tab newTab = tabs.get(newComponent); Tab oldTab = tabs.get(oldComponent); - // Gets the captions - String oldCaption = null; - Resource oldIcon = null; - String newCaption = null; - Resource newIcon = null; - - if (oldTab != null) { - oldCaption = oldTab.getCaption(); - oldIcon = oldTab.getIcon(); - } - - if (newTab != null) { - newCaption = newTab.getCaption(); - newIcon = newTab.getIcon(); - } else { - newCaption = newComponent.getCaption(); - newIcon = newComponent.getIcon(); - } - // Gets the locations int oldLocation = -1; int newLocation = -1; @@ -769,35 +756,21 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, addComponent(newComponent); } else if (newLocation == -1) { removeComponent(oldComponent); - keyMapper.remove(oldComponent); - newTab = addTab(newComponent); - components.remove(newComponent); - components.add(oldLocation, newComponent); - newTab.setCaption(oldCaption); - newTab.setIcon(oldIcon); + newTab = addTab(newComponent, oldLocation); + // Copy all relevant metadata to the new tab (#8793) + // TODO Should reuse the old tab instance instead? + copyTabMetadata(oldTab, newTab); } else { - if (oldLocation > newLocation) { - components.remove(oldComponent); - components.add(newLocation, oldComponent); - components.remove(newComponent); - components.add(oldLocation, newComponent); - } else { - components.remove(newComponent); - components.add(oldLocation, newComponent); - components.remove(oldComponent); - components.add(newLocation, oldComponent); - } + components.set(oldLocation, newComponent); + components.set(newLocation, oldComponent); - if (newTab != null) { - // This should always be true - newTab.setCaption(oldCaption); - newTab.setIcon(oldIcon); - } - if (oldTab != null) { - // This should always be true - oldTab.setCaption(newCaption); - oldTab.setIcon(newIcon); - } + // Tab associations are not changed, but metadata is swapped between + // the instances + // TODO Should reassociate the instances instead? + Tab tmp = new TabSheetTabImpl(null, null); + copyTabMetadata(newTab, tmp); + copyTabMetadata(oldTab, newTab); + copyTabMetadata(tmp, oldTab); requestRepaint(); } @@ -1106,28 +1079,34 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, /** * Returns the tab caption. Can never be null. */ + @Override public String getCaption() { return caption; } + @Override public void setCaption(String caption) { this.caption = caption; requestRepaint(); } + @Override public Resource getIcon() { return icon; } + @Override public void setIcon(Resource icon) { this.icon = icon; requestRepaint(); } + @Override public boolean isEnabled() { return enabled; } + @Override public void setEnabled(boolean enabled) { this.enabled = enabled; if (updateSelection()) { @@ -1136,10 +1115,12 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, requestRepaint(); } + @Override public boolean isVisible() { return visible; } + @Override public void setVisible(boolean visible) { this.visible = visible; if (updateSelection()) { @@ -1148,10 +1129,12 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, requestRepaint(); } + @Override public boolean isClosable() { return closable; } + @Override public void setClosable(boolean closable) { this.closable = closable; requestRepaint(); @@ -1161,24 +1144,29 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, } + @Override public String getDescription() { return description; } + @Override public void setDescription(String description) { this.description = description; requestRepaint(); } + @Override public ErrorMessage getComponentError() { return componentError; } + @Override public void setComponentError(ErrorMessage componentError) { this.componentError = componentError; requestRepaint(); } + @Override public Component getComponent() { for (Map.Entry<Component, Tab> entry : tabs.entrySet()) { if (entry.getValue() == this) { @@ -1188,11 +1176,13 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, return null; } + @Override public void setStyleName(String styleName) { this.styleName = styleName; requestRepaint(); } + @Override public String getStyleName() { return styleName; } @@ -1268,29 +1258,35 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, super.focus(); } + @Override public int getTabIndex() { return tabIndex; } + @Override public void setTabIndex(int tabIndex) { this.tabIndex = tabIndex; requestRepaint(); } + @Override public void addListener(BlurListener listener) { addListener(BlurEvent.EVENT_ID, BlurEvent.class, listener, BlurListener.blurMethod); } + @Override public void removeListener(BlurListener listener) { removeListener(BlurEvent.EVENT_ID, BlurEvent.class, listener); } + @Override public void addListener(FocusListener listener) { addListener(FocusEvent.EVENT_ID, FocusEvent.class, listener, FocusListener.focusMethod); } + @Override public void removeListener(FocusListener listener) { removeListener(FocusEvent.EVENT_ID, FocusEvent.class, listener); @@ -1300,4 +1296,23 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, public boolean isComponentVisible(Component childComponent) { return childComponent == getSelectedTab(); } + + /** + * Copies properties from one Tab to another. + * + * @param from + * The tab whose data to copy. + * @param to + * The tab to which copy the data. + */ + private static void copyTabMetadata(Tab from, Tab to) { + to.setCaption(from.getCaption()); + to.setIcon(from.getIcon()); + to.setDescription(from.getDescription()); + to.setVisible(from.isVisible()); + to.setEnabled(from.isEnabled()); + to.setClosable(from.isClosable()); + to.setStyleName(from.getStyleName()); + to.setComponentError(from.getComponentError()); + } } diff --git a/src/com/vaadin/ui/Table.java b/src/com/vaadin/ui/Table.java index e41c3d2a42..01b4e742e4 100644 --- a/src/com/vaadin/ui/Table.java +++ b/src/com/vaadin/ui/Table.java @@ -1569,6 +1569,13 @@ public class Table extends AbstractSelect implements Action.Container, } } else { // initial load + + // #8805 send one extra row in the beginning in case a partial + // row is shown on the UI + if (firstIndex > 0) { + firstIndex = firstIndex - 1; + rows = rows + 1; + } firstToBeRenderedInClient = firstIndex; } if (totalRows > 0) { diff --git a/tests/testbench/com/vaadin/tests/components/table/TableFirstRowFlicker.java b/tests/testbench/com/vaadin/tests/components/table/TableFirstRowFlicker.java new file mode 100644 index 0000000000..776e7956bf --- /dev/null +++ b/tests/testbench/com/vaadin/tests/components/table/TableFirstRowFlicker.java @@ -0,0 +1,85 @@ +package com.vaadin.tests.components.table; + +import com.vaadin.Application; +import com.vaadin.data.Container; +import com.vaadin.data.util.IndexedContainer; +import com.vaadin.ui.Label; +import com.vaadin.ui.ProgressIndicator; +import com.vaadin.ui.Table; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.Window; + +public class TableFirstRowFlicker extends Application { + + Table t; + + @Override + public void init() { + Window mainWindow = new Window("Table Row Flicker"); + mainWindow.getContent().setSizeFull(); + setMainWindow(mainWindow); + + t = new Table(); + t.setSizeFull(); + t.setSelectable(true); + t.setContainerDataSource(buildContainer()); + mainWindow.addComponent(t); + ((VerticalLayout) mainWindow.getContent()).setExpandRatio(t, 1); + + // Button button = new Button("Refresh"); + // button.addListener(new Button.ClickListener() { + // public void buttonClick(ClickEvent event) { + // t.refreshRowCache(); + // } + // }); + // mainWindow.addComponent(button); + + ProgressIndicator pi = new ProgressIndicator(); + pi.setPollingInterval(1000); + pi.setIndeterminate(true); + mainWindow.addComponent(pi); + + Thread r = new Thread() { + @Override + public void run() { + while (t != null) { + synchronized (t.getApplication()) { + int firstId = t.getCurrentPageFirstItemIndex(); + Object selected = t.getValue(); + t.setContainerDataSource(buildContainer()); + t.setValue(selected); + t.setCurrentPageFirstItemIndex(firstId); + // lighter alternative for all of above + // t.refreshRowCache(); + } + try { + Thread.sleep(500); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + System.out.println("Table update thread stopped"); + } + }; + r.start(); + } + + @Override + public void close() { + t = null; + super.close(); + } + + private Container buildContainer() { + IndexedContainer cont = new IndexedContainer(); + cont.addContainerProperty("name", Label.class, null); + for (int i = 0; i < 10000; i++) { + cont.addItem(i); + Label l = new Label("Item " + i); + l.setHeight("50px"); + cont.getContainerProperty(i, "name").setValue(l); + } + return cont; + } + +}
\ No newline at end of file diff --git a/tests/testbench/com/vaadin/tests/components/treetable/RowAnimation.html b/tests/testbench/com/vaadin/tests/components/treetable/RowAnimation.html new file mode 100644 index 0000000000..64ff061c13 --- /dev/null +++ b/tests/testbench/com/vaadin/tests/components/treetable/RowAnimation.html @@ -0,0 +1,77 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"> +<head profile="http://selenium-ide.openqa.org/profiles/test-case"> +<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> +<link rel="selenium.base" href="" /> +<title>TestAnimatedExpandCollapse</title> +</head> +<body> +<table cellpadding="1" cellspacing="1" border="1"> +<thead> +<tr><td rowspan="1" colspan="3">TestAnimatedExpandCollapse</td></tr> +</thead><tbody> +<tr> + <td>open</td> + <td>/run/com.vaadin.tests.components.treetable.TreeTableTest?restartApplication</td> + <td></td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_Smenu#item0</td> + <td>38,10</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::Root/VOverlay[0]/VMenuBar[0]#item0</td> + <td>35,5</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::Root/VOverlay[1]/VMenuBar[0]#item7</td> + <td>30,12</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]</td> + <td>12,7</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[0]/domChild[0]</td> + <td>31,7</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[2]/domChild[0]/domChild[0]/domChild[0]</td> + <td>29,8</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[3]/domChild[0]/domChild[0]/domChild[0]</td> + <td>30,6</td> +</tr> +<tr> + <td>assertText</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[0]</td> + <td>Item 4,1</td> +</tr> +<tr> + <td>assertText</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[2]/domChild[0]/domChild[0]</td> + <td>Item 5,1</td> +</tr> +<tr> + <td>assertText</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[3]/domChild[0]/domChild[0]</td> + <td>Item 6,1</td> +</tr> +<tr> + <td>assertText</td> + <td>vaadin=runcomvaadintestscomponentstreetableTreeTableTest::PID_StestComponent/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[4]/domChild[0]/domChild[0]</td> + <td>Item 7,1</td> +</tr> + +</tbody></table> +</body> +</html> diff --git a/tests/testbench/com/vaadin/tests/dd/NotPaintedAcceptSourceInTabSheet.java b/tests/testbench/com/vaadin/tests/dd/NotPaintedAcceptSourceInTabSheet.java new file mode 100644 index 0000000000..ac92193764 --- /dev/null +++ b/tests/testbench/com/vaadin/tests/dd/NotPaintedAcceptSourceInTabSheet.java @@ -0,0 +1,77 @@ +package com.vaadin.tests.dd; + +import com.vaadin.data.Item; +import com.vaadin.event.dd.DragAndDropEvent; +import com.vaadin.event.dd.DropHandler; +import com.vaadin.event.dd.acceptcriteria.AcceptCriterion; +import com.vaadin.event.dd.acceptcriteria.SourceIs; +import com.vaadin.tests.components.TestBase; +import com.vaadin.ui.AbstractSelect.AbstractSelectTargetDetails; +import com.vaadin.ui.TabSheet; +import com.vaadin.ui.Table; +import com.vaadin.ui.Table.TableDragMode; +import com.vaadin.ui.Table.TableTransferable; + +public class NotPaintedAcceptSourceInTabSheet extends TestBase { + + @Override + protected void setup() { + final Table source1 = createTable("Source 1"); + final Table source2 = createTable("Source 2"); + final Table target = createTable("Target"); + + source1.setDragMode(TableDragMode.ROW); + source2.setDragMode(TableDragMode.ROW); + + target.setDropHandler(new DropHandler() { + public AcceptCriterion getAcceptCriterion() { + return new SourceIs(source1, source2); + } + + public void drop(DragAndDropEvent event) { + TableTransferable transferable = (TableTransferable) event + .getTransferable(); + Item item = transferable.getSourceComponent().getItem( + transferable.getItemId()); + Object value = item.getItemProperty("value").getValue(); + AbstractSelectTargetDetails targetDetails = (AbstractSelectTargetDetails) event + .getTargetDetails(); + Object targetItemId = targetDetails.getItemIdOver(); + Object addItemAfter = target.addItemAfter(targetItemId); + target.getItem(addItemAfter).getItemProperty("value") + .setValue(value); + transferable.getSourceComponent().removeItem( + transferable.getItemId()); + } + }); + + TabSheet tabSheet = new TabSheet(); + tabSheet.addComponent(source1); + tabSheet.addComponent(source2); + + addComponent(tabSheet); + addComponent(target); + } + + private Table createTable(String caption) { + Table table = new Table(caption); + table.addContainerProperty("value", String.class, ""); + for (int i = 0; i < 10; i++) { + table.addItem(new Object[] { caption + " value " + i }, + Integer.valueOf(i)); + } + table.setWidth("300px"); + return table; + } + + @Override + protected String getDescription() { + return "Including a component in an accept criterion when the actual component is in a TabSheet and has not yet been painted should still allow painting the component properly when the tab is opened."; + } + + @Override + protected Integer getTicketNumber() { + return Integer.valueOf(8730); + } + +} |