From 58a7fd830a71987c9dfe4e3ceb37ae73ab31ceb1 Mon Sep 17 00:00:00 2001 From: Matti Tahvonen Date: Thu, 25 Sep 2008 06:18:25 +0000 Subject: [PATCH] fixes #1973, also makes uidl with table more efficient. Table should now comfort with subtree caching and no hacky requestRepaints are needed for child components. May cause regressions. svn changeset:5510/svn branch:trunk --- .../terminal/gwt/client/ui/IScrollTable.java | 36 +++++++++-- src/com/itmill/toolkit/ui/Table.java | 60 +++++++++++-------- 2 files changed, 68 insertions(+), 28 deletions(-) diff --git a/src/com/itmill/toolkit/terminal/gwt/client/ui/IScrollTable.java b/src/com/itmill/toolkit/terminal/gwt/client/ui/IScrollTable.java index ec804d9c2b..d104ec030c 100644 --- a/src/com/itmill/toolkit/terminal/gwt/client/ui/IScrollTable.java +++ b/src/com/itmill/toolkit/terminal/gwt/client/ui/IScrollTable.java @@ -7,6 +7,7 @@ package com.itmill.toolkit.terminal.gwt.client.ui; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Set; import java.util.Vector; @@ -20,6 +21,7 @@ import com.google.gwt.user.client.Timer; import com.google.gwt.user.client.Window; import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.FlowPanel; +import com.google.gwt.user.client.ui.HasWidgets; import com.google.gwt.user.client.ui.Panel; import com.google.gwt.user.client.ui.RootPanel; import com.google.gwt.user.client.ui.ScrollListener; @@ -138,6 +140,8 @@ public class IScrollTable extends Composite implements Table, ScrollListener, */ boolean recalcWidths = false; + private LinkedList lazyUnregistryBag = new LinkedList(); + public IScrollTable() { bodyContainer.addScrollListener(this); @@ -264,7 +268,7 @@ public class IScrollTable extends Composite implements Table, ScrollListener, } else { if (tBody != null) { tBody.removeFromParent(); - client.unregisterChildPaintables(tBody); + lazyUnregistryBag.add(tBody); } tBody = new IScrollTableBody(); @@ -277,6 +281,20 @@ public class IScrollTable extends Composite implements Table, ScrollListener, } } hideScrollPositionAnnotation(); + purgeUnregistryBag(); + } + + /** + * Unregisters Paintables in "trashed" HasWidgets (IScrollTableBodys or + * IScrollTableRows). This is done lazily as Table must survive from + * "subtreecaching" logic. + */ + private void purgeUnregistryBag() { + for (Iterator iterator = lazyUnregistryBag.iterator(); iterator + .hasNext();) { + client.unregisterChildPaintables((HasWidgets) iterator.next()); + } + lazyUnregistryBag.clear(); } private void updateActionMap(UIDL c) { @@ -1857,7 +1875,7 @@ public class IScrollTable extends Composite implements Table, ScrollListener, } final IScrollTableRow toBeRemoved = (IScrollTableRow) renderedRows .get(index); - client.unregisterChildPaintables(toBeRemoved); + lazyUnregistryBag.add(toBeRemoved); DOM.removeChild(tBody, toBeRemoved.getElement()); orphan(toBeRemoved); renderedRows.remove(index); @@ -2096,6 +2114,9 @@ public class IScrollTable extends Composite implements Table, ScrollListener, } DOM.appendChild(td, container); DOM.appendChild(getElement(), td); + // ensure widget not attached to another element (possible tBody + // change) + w.removeFromParent(); DOM.appendChild(container, w.getElement()); adopt(w); childWidgets.add(w); @@ -2106,8 +2127,15 @@ public class IScrollTable extends Composite implements Table, ScrollListener, } public boolean remove(Widget w) { - // TODO Auto-generated method stub - return false; + if (childWidgets.contains(w)) { + orphan(w); + DOM.removeChild(DOM.getParent(w.getElement()), w + .getElement()); + childWidgets.remove(w); + return true; + } else { + return false; + } } private void handleClickEvent(Event event) { diff --git a/src/com/itmill/toolkit/ui/Table.java b/src/com/itmill/toolkit/ui/Table.java index b600aa31e1..adb78be2d2 100644 --- a/src/com/itmill/toolkit/ui/Table.java +++ b/src/com/itmill/toolkit/ui/Table.java @@ -1233,6 +1233,8 @@ public class Table extends AbstractSelect implements Action.Container, Object[][] cells = new Object[cols + CELL_FIRSTCOL][rows]; if (rows == 0) { pageBuffer = cells; + unregisterPropertiesAndComponents(oldListenedProperties, + oldVisibleComponents); return; } @@ -1357,30 +1359,45 @@ public class Table extends AbstractSelect implements Action.Container, // Saves the results to internal buffer pageBuffer = cells; - if (oldVisibleComponents != null) { - for (final Iterator i = oldVisibleComponents.iterator(); i - .hasNext();) { - Component c = (Component) i.next(); - if (!visibleComponents.contains(c)) { - c.setParent(null); - } + unregisterPropertiesAndComponents(oldListenedProperties, + oldVisibleComponents); + + requestRepaint(); + } + + } + + /** + * Helper method to remove listeners and maintain correct component + * hierarchy. Detaches properties and components if those are no more + * rendered in client. + * + * @param oldListenedProperties + * set of properties that where listened in last render + * @param oldVisibleComponents + * set of components that where attached in last render + */ + private void unregisterPropertiesAndComponents( + HashSet oldListenedProperties, HashSet oldVisibleComponents) { + if (oldVisibleComponents != null) { + for (final Iterator i = oldVisibleComponents.iterator(); i + .hasNext();) { + Component c = (Component) i.next(); + if (!visibleComponents.contains(c)) { + c.setParent(null); } } + } - if (oldListenedProperties != null) { - for (final Iterator i = oldListenedProperties.iterator(); i - .hasNext();) { - Property.ValueChangeNotifier o = (ValueChangeNotifier) i - .next(); - if (!listenedProperties.contains(o)) { - o.removeListener(this); - } + if (oldListenedProperties != null) { + for (final Iterator i = oldListenedProperties.iterator(); i + .hasNext();) { + Property.ValueChangeNotifier o = (ValueChangeNotifier) i.next(); + if (!listenedProperties.contains(o)) { + o.removeListener(this); } } - - requestRepaint(); } - } /** @@ -1565,6 +1582,7 @@ public class Table extends AbstractSelect implements Action.Container, // Assure visual refresh resetPageBuffer(); + enableContentRefreshing(true); } @@ -2001,12 +2019,6 @@ public class Table extends AbstractSelect implements Action.Container, if (c == null) { target.addText(""); } else { - /* - * FIXME ensuring that table never gets "cached" child - * paints by calling requestRepaint for child component - * IScrollTable currently can's survive those. - */ - c.requestRepaint(); c.paint(target); } } else { -- 2.39.5