Revision: 7746
Author: [email protected]
Date: Thu Mar 18 11:37:10 2010
Log: Remove the glass panel from DOM when Popup#removeFromParent is called.
http://gwt-code-reviews.appspot.com/220801
Review by: [email protected]
http://code.google.com/p/google-web-toolkit/source/detail?r=7746
Modified:
/trunk/user/src/com/google/gwt/user/client/ui/PopupPanel.java
/trunk/user/test/com/google/gwt/user/client/ui/PopupTest.java
=======================================
--- /trunk/user/src/com/google/gwt/user/client/ui/PopupPanel.java Tue Feb
16 07:47:04 2010
+++ /trunk/user/src/com/google/gwt/user/client/ui/PopupPanel.java Thu Mar
18 11:37:10 2010
@@ -34,12 +34,11 @@
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.i18n.client.LocaleInfo;
-import com.google.gwt.user.client.Command;
import com.google.gwt.user.client.DOM;
-import com.google.gwt.user.client.DeferredCommand;
import com.google.gwt.user.client.Event;
import com.google.gwt.user.client.EventPreview;
import com.google.gwt.user.client.History;
+import com.google.gwt.user.client.Timer;
import com.google.gwt.user.client.Window;
import com.google.gwt.user.client.Event.NativePreviewEvent;
import com.google.gwt.user.client.Event.NativePreviewHandler;
@@ -133,6 +132,13 @@
*/
private PopupPanel curPanel = null;
+ /**
+ * Indicates whether or not the {...@link PopupPanel} is in the process of
+ * unloading. If the popup is unloading, then the animation just does
+ * cleanup.
+ */
+ private boolean isUnloading;
+
/**
* The offset height and width of the current {...@link PopupPanel}.
*/
@@ -143,6 +149,11 @@
*/
private boolean showing;
+ /**
+ * The timer used to delay the show animation.
+ */
+ private Timer showTimer;
+
/**
* A boolean indicating whether the glass element is currently
attached.
*/
@@ -166,12 +177,25 @@
*
* @param showing true if the popup is showing, false if not
*/
- public void setState(boolean showing) {
+ public void setState(boolean showing, boolean isUnloading) {
// Immediately complete previous open/close animation
+ this.isUnloading = isUnloading;
cancel();
+ // If there is a pending timer to start a show animation, then just
cancel
+ // the timer and complete the show operation.
+ if (showTimer != null) {
+ showTimer.cancel();
+ showTimer = null;
+ onComplete();
+ }
+
+ // Update the logical state.
+ curPanel.showing = showing;
+ curPanel.updateHandlers();
+
// Determine if we need to animate
- boolean animate = curPanel.isAnimationEnabled;
+ boolean animate = !isUnloading && curPanel.isAnimationEnabled;
if (curPanel.animType != AnimationType.CENTER && !showing) {
animate = false;
}
@@ -196,14 +220,21 @@
impl.setClip(curPanel.getElement(), getRectString(0, 0, 0, 0));
RootPanel.get().add(curPanel);
impl.onShow(curPanel.getElement());
- }
-
- // Wait for the popup panel to be attached before running the
animation
- DeferredCommand.addCommand(new Command() {
- public void execute() {
- run(ANIMATION_DURATION);
- }
- });
+
+ // Wait for the popup panel and iframe to be attached before
running
+ // the animation. We use a Timer instead of a DeferredCommand so
we
+ // can cancel it if the popup is hidden synchronously.
+ showTimer = new Timer() {
+ @Override
+ public void run() {
+ showTimer = null;
+ ResizeAnimation.this.run(ANIMATION_DURATION);
+ }
+ };
+ showTimer.schedule(1);
+ } else {
+ run(ANIMATION_DURATION);
+ }
} else {
onInstantaneousRun();
}
@@ -213,7 +244,9 @@
protected void onComplete() {
if (!showing) {
maybeShowGlass();
- RootPanel.get().remove(curPanel);
+ if (!isUnloading) {
+ RootPanel.get().remove(curPanel);
+ }
impl.onHide(curPanel.getElement());
}
impl.setClip(curPanel.getElement(), "rect(auto, auto, auto, auto)");
@@ -311,7 +344,9 @@
RootPanel.get().add(curPanel);
impl.onShow(curPanel.getElement());
} else {
- RootPanel.get().remove(curPanel);
+ if (!isUnloading) {
+ RootPanel.get().remove(curPanel);
+ }
impl.onHide(curPanel.getElement());
}
DOM.setStyleAttribute(curPanel.getElement(), "overflow", "visible");
@@ -582,7 +617,7 @@
if (!isShowing()) {
return;
}
- setState(false, true);
+ resizeAnimation.setState(false, false);
CloseEvent.fire(this, this, autoClosed);
}
@@ -967,7 +1002,7 @@
if (showing) {
return;
}
- setState(true, true);
+ resizeAnimation.setState(true, false);
}
/**
@@ -1019,11 +1054,13 @@
@Override
protected void onUnload() {
+ super.onUnload();
+
// Just to be sure, we perform cleanup when the popup is unloaded (i.e.
// removed from the DOM). This is normally taken care of in hide(),
but it
// can be missed if someone removes the popup directly from the
RootPanel.
if (isShowing()) {
- setState(false, false);
+ resizeAnimation.setState(false, true);
}
}
@@ -1363,22 +1400,20 @@
}
/**
- * Set the showing state of the popup. If maybeAnimate is true, the
animation
- * will be used to set the state. If it is false, the animation will be
- * cancelled.
- *
- * @param showing the new state
- * @param maybeAnimate true to possibly run the animation
+ * Register or unregister the handlers used by {...@link PopupPanel}.
*/
- private void setState(boolean showing, boolean maybeAnimate) {
- if (maybeAnimate) {
- resizeAnimation.setState(showing);
- } else {
- resizeAnimation.cancel();
- }
- this.showing = showing;
-
- // Create or remove the native preview handler
+ private void updateHandlers() {
+ // Remove any existing handlers.
+ if (nativePreviewHandlerRegistration != null) {
+ nativePreviewHandlerRegistration.removeHandler();
+ nativePreviewHandlerRegistration = null;
+ }
+ if (historyHandlerRegistration != null) {
+ historyHandlerRegistration.removeHandler();
+ historyHandlerRegistration = null;
+ }
+
+ // Create handlers if showing.
if (showing) {
nativePreviewHandlerRegistration = Event.addNativePreviewHandler(new
NativePreviewHandler() {
public void onPreviewNativeEvent(NativePreviewEvent event) {
@@ -1392,15 +1427,6 @@
}
}
});
- } else {
- if (nativePreviewHandlerRegistration != null) {
- nativePreviewHandlerRegistration.removeHandler();
- nativePreviewHandlerRegistration = null;
- }
- if (historyHandlerRegistration != null) {
- historyHandlerRegistration.removeHandler();
- historyHandlerRegistration = null;
- }
}
}
}
=======================================
--- /trunk/user/test/com/google/gwt/user/client/ui/PopupTest.java Thu Feb
25 06:57:14 2010
+++ /trunk/user/test/com/google/gwt/user/client/ui/PopupTest.java Thu Mar
18 11:37:10 2010
@@ -269,6 +269,23 @@
assertTrue(isAttached(glass));
popup.hide();
}
+
+ /**
+ * Test the hiding a popup while it is showing will not result in an
illegal
+ * state.
+ */
+ public void testHideWhileShowing() {
+ PopupPanel popup = createPopupPanel();
+
+ // Start showing the popup.
+ popup.setAnimationEnabled(true);
+ popup.show();
+ assertTrue(popup.isShowing());
+
+ // Hide the popup while its showing.
+ popup.hide();
+ assertFalse(popup.isShowing());
+ }
/**
* Test that the onLoad method is only called once when showing the
popup.
@@ -365,6 +382,21 @@
});
popup.hide();
}
+
+ /**
+ * Issue 4720: Glass is not removed when removeFromParent is called.
+ */
+ public void testRemoveFromParent() {
+ PopupPanel popup = createPopupPanel();
+ popup.setGlassEnabled(true);
+ assertNull(popup.getGlassElement().getParentElement());
+
+ popup.show();
+ assertNotNull(popup.getGlassElement().getParentElement());
+
+ popup.removeFromParent();
+ assertNull(popup.getGlassElement().getParentElement());
+ }
public void testSeparateContainers() {
TestablePopupPanel p1 = new TestablePopupPanel();
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
To unsubscribe from this group, send email to
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with
the words "REMOVE ME" as the subject.
