From: Artur Signell Date: Sun, 11 Jan 2009 19:52:56 +0000 (+0000) Subject: Test cases and fix for #2425/#2289 - Removing a Tab from TabSheet or Accordion should... X-Git-Tag: 6.7.0.beta1~3362 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b6b4318b59bba65af0f1cb38942babf664fd6ed9;p=vaadin-framework.git Test cases and fix for #2425/#2289 - Removing a Tab from TabSheet or Accordion should now work properly and onAttach is not called before child is attached. svn changeset:6496/svn branch:trunk --- diff --git a/src/com/itmill/toolkit/terminal/gwt/client/ui/IAccordion.java b/src/com/itmill/toolkit/terminal/gwt/client/ui/IAccordion.java index a97787218f..aeec3bf1b0 100644 --- a/src/com/itmill/toolkit/terminal/gwt/client/ui/IAccordion.java +++ b/src/com/itmill/toolkit/terminal/gwt/client/ui/IAccordion.java @@ -1,6 +1,5 @@ package com.itmill.toolkit.terminal.gwt.client.ui; -import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -27,8 +26,6 @@ public class IAccordion extends ITabsheetBase implements public static final String CLASSNAME = "i-accordion"; - private ArrayList stack = new ArrayList(); - private Set paintables = new HashSet(); private String height; @@ -65,7 +62,7 @@ public class IAccordion extends ITabsheetBase implements * the content area is */ if (selectedUIDLItemIndex >= 0) { - StackItem selectedItem = stack.get(selectedUIDLItemIndex); + StackItem selectedItem = getStackItem(selectedUIDLItemIndex); UIDL selectedTabUIDL = lazyUpdateMap.remove(selectedItem); open(selectedUIDLItemIndex); @@ -95,17 +92,17 @@ public class IAccordion extends ITabsheetBase implements boolean hidden) { StackItem item; int itemIndex; - if (stack.size() <= index) { + if (getWidgetCount() <= index) { // Create stackItem and render caption item = new StackItem(tabUidl); - if (stack.size() == 0) { + if (getWidgetCount() == 0) { item.addStyleDependentName("first"); } - stack.add(item); - itemIndex = stack.size() - 1; + itemIndex = getWidgetCount(); add(item, getElement()); } else { - item = stack.get(index); + item = getStackItem(index); + item = moveStackItemIfNeeded(item, index, tabUidl); itemIndex = index; item.updateCaption(tabUidl); } @@ -121,8 +118,72 @@ public class IAccordion extends ITabsheetBase implements } } + /** + * This method tries to find out if a tab has been rendered with a different + * index previously. If this is the case it re-orders the children so the + * same StackItem is used for rendering this time. E.g. if the first tab has + * been removed all tabs which contain cached content must be moved 1 step + * up to preserve the cached content. + * + * @param item + * @param newIndex + * @param tabUidl + * @return + */ + private StackItem moveStackItemIfNeeded(StackItem item, int newIndex, + UIDL tabUidl) { + UIDL tabContentUIDL = null; + Paintable tabContent = null; + if (tabUidl.getChildCount() > 0) { + tabContentUIDL = tabUidl.getChildUIDL(0); + tabContent = client.getPaintable(tabContentUIDL); + } + + Widget itemWidget = item.getComponent(); + if (tabContent != null) { + if (tabContent != itemWidget) { + /* + * This is not the same widget as before, find out if it has + * been moved + */ + int oldIndex = -1; + StackItem oldItem = null; + for (int i = 0; i < getWidgetCount(); i++) { + Widget w = getWidget(i); + oldItem = (StackItem) w; + if (tabContent == oldItem.getComponent()) { + oldIndex = i; + break; + } + } + + if (oldIndex != -1 && oldIndex > newIndex) { + /* + * The tab has previously been rendered in another position + * so we must move the cached content to correct position. + * We move only items with oldIndex > newIndex to prevent + * moving items already rendered in this update. If for + * instance tabs 1,2,3 are removed and added as 3,2,1 we + * cannot re-use "1" when we get to the third tab. + */ + insert(oldItem, getElement(), newIndex, true); + return oldItem; + } + } + } else { + // Tab which has never been loaded. Must assure we use an empty + // StackItem + Widget oldWidget = item.getComponent(); + if (oldWidget != null) { + item = new StackItem(tabUidl); + insert(item, getElement(), newIndex, true); + } + } + return item; + } + private void open(int itemIndex) { - StackItem item = stack.get(itemIndex); + StackItem item = (StackItem) getWidget(itemIndex); boolean alreadyOpen = false; if (openTab != null) { if (openTab.isOpen()) { @@ -157,7 +218,7 @@ public class IAccordion extends ITabsheetBase implements @Override protected void selectTab(final int index, final UIDL contentUidl) { - StackItem item = stack.get(index); + StackItem item = getStackItem(index); if (index != activeTabIndex) { open(index); iLayout(); @@ -169,7 +230,7 @@ public class IAccordion extends ITabsheetBase implements } public void onSelectTab(StackItem item) { - final int index = stack.indexOf(item); + final int index = getWidgetIndex(item); if (index != activeTabIndex && !disabled && !readonly && !disabledTabKeys.contains(tabKeys.get(index))) { addStyleDependentName("loading"); @@ -236,8 +297,8 @@ public class IAccordion extends ITabsheetBase implements // HEIGHT if (!isDynamicHeight()) { int usedPixels = 0; - for (Iterator iterator = stack.iterator(); iterator.hasNext();) { - StackItem item = (StackItem) iterator.next(); + for (Widget w : getChildren()) { + StackItem item = (StackItem) w; if (item == openTab) { usedPixels += item.getCaptionHeight(); } else { @@ -271,7 +332,8 @@ public class IAccordion extends ITabsheetBase implements if (isDynamicWidth()) { int maxWidth = 40; - for (StackItem si : stack) { + for (Widget w : getChildren()) { + StackItem si = (StackItem) w; int captionWidth = si.getCaptionWidth(); if (captionWidth > maxWidth) { maxWidth = captionWidth; @@ -308,6 +370,13 @@ public class IAccordion extends ITabsheetBase implements } } + public Widget getComponent() { + if (getWidgetCount() < 2) { + return null; + } + return getWidget(1); + } + @Override public void setVisible(boolean visible) { super.setVisible(visible); @@ -480,7 +549,6 @@ public class IAccordion extends ITabsheetBase implements @Override protected void clearPaintables() { - stack.clear(); clear(); } @@ -506,7 +574,8 @@ public class IAccordion extends ITabsheetBase implements } public void replaceChildComponent(Widget oldComponent, Widget newComponent) { - for (StackItem item : stack) { + for (Widget w : getChildren()) { + StackItem item = (StackItem) w; if (item.getPaintable() == oldComponent) { item.replacePaintable((Paintable) newComponent); return; @@ -515,8 +584,8 @@ public class IAccordion extends ITabsheetBase implements } public void updateCaption(Paintable component, UIDL uidl) { - for (Iterator iterator = stack.iterator(); iterator.hasNext();) { - StackItem si = (StackItem) iterator.next(); + for (Widget w : getChildren()) { + StackItem si = (StackItem) w; if (si.getPaintable() == component) { boolean visible = si.isVisible(); si.updateCaption(uidl); @@ -567,13 +636,25 @@ public class IAccordion extends ITabsheetBase implements @Override protected int getTabCount() { - return stack.size(); + return getWidgetCount(); } @Override protected void removeTab(int index) { - StackItem item = stack.get(index); + StackItem item = getStackItem(index); remove(item); } + @Override + protected Paintable getTab(int index) { + if (index < getWidgetCount()) { + return (Paintable) (getStackItem(index)).getPaintable(); + } + + return null; + } + + private StackItem getStackItem(int index) { + return (StackItem) getWidget(index); + } } diff --git a/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheet.java b/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheet.java index 3c963feed3..590d4d7259 100644 --- a/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheet.java +++ b/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheet.java @@ -16,7 +16,6 @@ import com.google.gwt.user.client.Element; import com.google.gwt.user.client.Event; import com.google.gwt.user.client.ui.ClickListener; import com.google.gwt.user.client.ui.ComplexPanel; -import com.google.gwt.user.client.ui.Label; import com.google.gwt.user.client.ui.Widget; import com.itmill.toolkit.terminal.gwt.client.ApplicationConnection; import com.itmill.toolkit.terminal.gwt.client.ICaption; @@ -417,25 +416,63 @@ public class ITabsheet extends ITabsheetBase { */ c.setWidth(c.getRequiredWidth() + "px"); captions.put("" + index, c); + + UIDL tabContentUIDL = null; + Paintable tabContent = null; + if (tabUidl.getChildCount() > 0) { + tabContentUIDL = tabUidl.getChildUIDL(0); + tabContent = client.getPaintable(tabContentUIDL); + } + + if (tabContent != null) { + /* This is a tab with content information */ + + int oldIndex = tp.getWidgetIndex((Widget) tabContent); + if (oldIndex != -1 && oldIndex != index) { + /* + * The tab has previously been rendered in another position so + * we must move the cached content to correct position + */ + tp.insert((Widget) tabContent, index); + } + } else { + /* A tab whose content has not yet been loaded */ + + /* + * Make sure there is a corresponding empty tab in tp. The same + * operation as the moving above but for not-loaded tabs. + */ + if (index < tp.getWidgetCount()) { + Widget oldWidget = tp.getWidget(index); + if (!(oldWidget instanceof PlaceHolder)) { + tp.insert(new PlaceHolder(), index); + } + } + + } + if (selected) { - renderContent(tabUidl.getChildUIDL(0)); + renderContent(tabContentUIDL); tb.selectTab(index); } else { - if (tabUidl.getChildCount() > 0) { + if (tabContentUIDL != null) { // updating a drawn child on hidden tab - Paintable paintable = client.getPaintable(tabUidl - .getChildUIDL(0)); - - if (tp.getWidgetIndex((Widget) paintable) < 0) { - tp.insert((Widget) paintable, index); + if (tp.getWidgetIndex((Widget) tabContent) < 0) { + tp.insert((Widget) tabContent, index); } - paintable.updateFromUIDL(tabUidl.getChildUIDL(0), client); + tabContent.updateFromUIDL(tabContentUIDL, client); } else if (tp.getWidgetCount() <= index) { - tp.add(new Label("")); + tp.add(new PlaceHolder()); } } } + public class PlaceHolder extends ILabel { + public PlaceHolder() { + super(""); + } + } + @Override protected void selectTab(int index, final UIDL contentUidl) { if (index != activeTabIndex) { @@ -709,6 +746,14 @@ public class ITabsheet extends ITabsheetBase { return tb.getWidgetCount(); } + @Override + protected Paintable getTab(int index) { + if (tp.getWidgetCount() > index) { + return (Paintable) tp.getWidget(index); + } + return null; + } + @Override protected void removeTab(int index) { tb.removeTab(index); diff --git a/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetBase.java b/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetBase.java index d6cb534577..8117ef6151 100644 --- a/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetBase.java +++ b/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetBase.java @@ -45,6 +45,8 @@ abstract class ITabsheetBase extends ComplexPanel implements Container { // Render content final UIDL tabs = uidl.getChildUIDL(0); + + // Paintables in the TabSheet before update ArrayList oldPaintables = new ArrayList(); for (Iterator iterator = getPaintableIterator(); iterator.hasNext();) { oldPaintables.add(iterator.next()); @@ -70,10 +72,6 @@ abstract class ITabsheetBase extends ComplexPanel implements Container { if (selected) { activeTabIndex = index; } - if (tab.getChildCount() > 0) { - Paintable p = client.getPaintable(tab.getChildUIDL(0)); - oldPaintables.remove(p); - } renderTab(tab, index, selected, hidden); index++; } @@ -83,6 +81,12 @@ abstract class ITabsheetBase extends ComplexPanel implements Container { removeTab(index); } + for (int i = 0; i < getTabCount(); i++) { + Paintable p = getTab(i); + oldPaintables.remove(p); + } + + // Perform unregister for any paintables removed during update for (Iterator iterator = oldPaintables.iterator(); iterator.hasNext();) { Object oldPaintable = iterator.next(); if (oldPaintable instanceof Paintable) { @@ -126,6 +130,12 @@ abstract class ITabsheetBase extends ComplexPanel implements Container { */ protected abstract int getTabCount(); + /** + * Implement in extending classes. This method should return the Paintable + * corresponding to the given index. + */ + protected abstract Paintable getTab(int index); + /** * Implement in extending classes. This method should remove the rendered * tab with the specified index. diff --git a/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetPanel.java b/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetPanel.java index a21d1b7d98..e9dc44ee3c 100644 --- a/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetPanel.java +++ b/src/com/itmill/toolkit/terminal/gwt/client/ui/ITabsheetPanel.java @@ -72,20 +72,25 @@ public class ITabsheetPanel extends ComplexPanel { */ public void insert(Widget w, int beforeIndex) { Element el = createContainerElement(); - super.insert(w, el, beforeIndex, false); DOM.insertChild(getElement(), el, beforeIndex); + super.insert(w, el, beforeIndex, false); } @Override public boolean remove(Widget w) { - final int index = getWidgetIndex(w); + Element child = w.getElement(); + Element parent = null; + if (child != null) { + parent = DOM.getParent(child); + } final boolean removed = super.remove(w); if (removed) { if (visibleWidget == w) { visibleWidget = null; } - Element child = DOM.getChild(getElement(), index); - DOM.removeChild(getElement(), child); + if (parent != null) { + DOM.removeChild(getElement(), parent); + } } return removed; } diff --git a/src/com/itmill/toolkit/tests/components/TestBase.java b/src/com/itmill/toolkit/tests/components/TestBase.java new file mode 100644 index 0000000000..0cc1a0f08c --- /dev/null +++ b/src/com/itmill/toolkit/tests/components/TestBase.java @@ -0,0 +1,45 @@ +package com.itmill.toolkit.tests.components; + +import com.itmill.toolkit.Application; +import com.itmill.toolkit.ui.Label; +import com.itmill.toolkit.ui.Layout; +import com.itmill.toolkit.ui.SplitPanel; +import com.itmill.toolkit.ui.VerticalLayout; +import com.itmill.toolkit.ui.Window; + +public abstract class TestBase extends Application { + + @Override + public final void init() { + window = new Window(getClass().getName()); + setMainWindow(window); + window.getLayout().setSizeFull(); + + Label label = new Label(getDescription()); + label.setWidth("100%"); + window.getLayout().addComponent(label); + + layout = new VerticalLayout(); + window.getLayout().addComponent(layout); + ((VerticalLayout) window.getLayout()).setExpandRatio(layout, 1); + + setup(); + } + + private Window window; + private SplitPanel splitPanel; + private Layout layout; + + public TestBase() { + + } + + protected Layout getLayout() { + return layout; + } + + protected abstract String getDescription(); + + protected abstract void setup(); + +} diff --git a/src/com/itmill/toolkit/tests/components/accordion/RemoveTabs.java b/src/com/itmill/toolkit/tests/components/accordion/RemoveTabs.java new file mode 100644 index 0000000000..7ee5e071a0 --- /dev/null +++ b/src/com/itmill/toolkit/tests/components/accordion/RemoveTabs.java @@ -0,0 +1,125 @@ +package com.itmill.toolkit.tests.components.accordion; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import com.itmill.toolkit.tests.components.TestBase; +import com.itmill.toolkit.ui.AbstractComponentContainer; +import com.itmill.toolkit.ui.Accordion; +import com.itmill.toolkit.ui.Button; +import com.itmill.toolkit.ui.Component; +import com.itmill.toolkit.ui.Label; +import com.itmill.toolkit.ui.Button.ClickEvent; + +public class RemoveTabs extends TestBase { + + private Accordion accordion; + + protected Component[] tab = new Component[5]; + + private Button closeCurrent; + private Button closeFirst; + private Button closeLast; + private Button reorderTabs; + + @Override + protected String getDescription() { + return "Tests the removal of individual tabs from an Accordion. No matter what is done in this test the tab caption \"Tab X\" should always match the content \"Tab X\". Use \"remove first\" and \"remove active\" buttons to remove the first or the active tab. The \"reorder\" button reverses the order by adding and removing all components."; + } + + @Override + protected void setup() { + accordion = new Accordion(); + for (int i = 1; i <= tab.length; i++) { + tab[i - 1] = new Label("This is the contents of tab " + i); + tab[i - 1].setCaption("Tab " + i); + + accordion.addComponent(tab[i - 1]); + } + + getLayout().addComponent(accordion); + + closeCurrent = new Button("Close current tab"); + closeCurrent.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + closeCurrentTab(); + + } + }); + + closeFirst = new Button("close first tab"); + closeFirst.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + closeFirstTab(); + + } + }); + + closeLast = new Button("close last tab"); + closeLast.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + closeLastTab(); + + } + }); + + reorderTabs = new Button("reorder"); + reorderTabs.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + reorder(); + + } + }); + + getLayout().addComponent(closeFirst); + getLayout().addComponent(closeLast); + getLayout().addComponent(closeCurrent); + getLayout().addComponent(reorderTabs); + + } + + private void closeCurrentTab() { + Component c = accordion.getSelectedTab(); + if (c != null) { + accordion.removeComponent(c); + } + } + + private void closeFirstTab() { + accordion.removeComponent((Component) accordion.getComponentIterator() + .next()); + } + + @SuppressWarnings("unchecked") + private void closeLastTab() { + Iterator i = accordion.getComponentIterator(); + Component last = null; + while (i.hasNext()) { + last = (Component) i.next(); + + } + accordion.removeComponent(last); + } + + @SuppressWarnings("unchecked") + private void reorder() { + AbstractComponentContainer container = accordion; + + if (container != null) { + List c = new ArrayList(); + Iterator i = container.getComponentIterator(); + while (i.hasNext()) { + Component comp = i.next(); + c.add(comp); + } + container.removeAllComponents(); + + for (int j = c.size() - 1; j >= 0; j--) { + container.addComponent(c.get(j)); + } + + } + } + +} diff --git a/src/com/itmill/toolkit/tests/components/tabsheet/RemoveTabs.java b/src/com/itmill/toolkit/tests/components/tabsheet/RemoveTabs.java new file mode 100644 index 0000000000..bbae6caf24 --- /dev/null +++ b/src/com/itmill/toolkit/tests/components/tabsheet/RemoveTabs.java @@ -0,0 +1,124 @@ +package com.itmill.toolkit.tests.components.tabsheet; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import com.itmill.toolkit.tests.components.TestBase; +import com.itmill.toolkit.ui.AbstractComponentContainer; +import com.itmill.toolkit.ui.Button; +import com.itmill.toolkit.ui.Component; +import com.itmill.toolkit.ui.Label; +import com.itmill.toolkit.ui.TabSheet; +import com.itmill.toolkit.ui.Button.ClickEvent; + +public class RemoveTabs extends TestBase { + + protected TabSheet tabsheet; + + protected Component[] tab = new Component[5]; + + private Button closeCurrent; + private Button closeFirst; + private Button closeLast; + private Button reorderTabs; + + @Override + protected String getDescription() { + return "Tests the removal of individual tabs from a Tabsheet. No matter what is done in this test the tab caption \"Tab X\" should always match the content \"Tab X\". Use \"remove first\" and \"remove active\" buttons to remove the first or the active tab. The \"reorder\" button reverses the order by adding and removing all components."; + } + + @Override + protected void setup() { + tabsheet = new TabSheet(); + for (int i = 1; i <= tab.length; i++) { + tab[i - 1] = new Label("This is the contents of tab " + i); + tab[i - 1].setCaption("Tab " + i); + + tabsheet.addComponent(tab[i - 1]); + } + + getLayout().addComponent(tabsheet); + + closeCurrent = new Button("Close current tab"); + closeCurrent.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + closeCurrentTab(); + + } + }); + + closeFirst = new Button("close first tab"); + closeFirst.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + closeFirstTab(); + + } + }); + + closeLast = new Button("close last tab"); + closeLast.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + closeLastTab(); + + } + }); + + reorderTabs = new Button("reorder"); + reorderTabs.addListener(new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + reorder(); + + } + }); + + getLayout().addComponent(closeFirst); + getLayout().addComponent(closeLast); + getLayout().addComponent(closeCurrent); + getLayout().addComponent(reorderTabs); + + } + + private void closeCurrentTab() { + Component c = tabsheet.getSelectedTab(); + if (c != null) { + tabsheet.removeComponent(c); + } + } + + private void closeFirstTab() { + tabsheet.removeComponent((Component) tabsheet.getComponentIterator() + .next()); + } + + @SuppressWarnings("unchecked") + private void closeLastTab() { + Iterator i = tabsheet.getComponentIterator(); + Component last = null; + while (i.hasNext()) { + last = (Component) i.next(); + + } + tabsheet.removeComponent(last); + } + + @SuppressWarnings("unchecked") + private void reorder() { + AbstractComponentContainer container = tabsheet; + + if (container != null) { + List c = new ArrayList(); + Iterator i = container.getComponentIterator(); + while (i.hasNext()) { + Component comp = i.next(); + c.add(comp); + } + container.removeAllComponents(); + + for (int j = c.size() - 1; j >= 0; j--) { + container.addComponent(c.get(j)); + } + + } + } +}