It might be good for the javadoc on the setter methods to mention that they
have no immediate effect if called while the dialog is showing. In fact,
should we encourage people to hide the dialog before using the new setters?
It looks like we have no test coverage for the panel's behavior in its four
states? (modal x autohide.) You could factor out the guts of testPopup and
run it four times, once in each combination.
For that matter, we don't test auto-hide or modality at all, beyond your new
check that the bits get set. Is that possible?
rjrjr
On Tue, Oct 7, 2008 at 9:52 AM, John LaBanca <[EMAIL PROTECTED]> wrote:
> Ray -
>
> Please do a code review on this patch that adds getters and setters for
> modal and autoHide properties to PopupPanel.
>
> Description:
> =========
> It would be nice to have getters and setters for moal and autoHide
> properties in PopupPanel.
>
> Fix:
> ===
> Added them.
>
> Testing:
> ======
> Manually verified that changing the properties while the popup is visible
> does not cause any state problems. The booleans are checked on
> onEventPreview() at the time of an event, and no special setup/teardown is
> done as a result of them being set. I also added a unit test for the
> setters and getters.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
Index: user/test/com/google/gwt/user/client/ui/PopupTest.java
===================================================================
--- user/test/com/google/gwt/user/client/ui/PopupTest.java (revision 3723)
+++ user/test/com/google/gwt/user/client/ui/PopupTest.java (working copy)
@@ -50,6 +50,18 @@
assertFalse(popup.isAnimationEnabled());
popup.setAnimationEnabled(true);
assertTrue(popup.isAnimationEnabled());
+
+ // Modal
+ popup.setModal(true);
+ assertTrue(popup.isModal());
+ popup.setModal(false);
+ assertFalse(popup.isModal());
+
+ // AutoHide enabled
+ popup.setAutoHideEnabled(true);
+ assertTrue(popup.isAutoHideEnabled());
+ popup.setAutoHideEnabled(false);
+ assertFalse(popup.isAutoHideEnabled());
}
/**
Index: user/src/com/google/gwt/user/client/ui/PopupPanel.java
===================================================================
--- user/src/com/google/gwt/user/client/ui/PopupPanel.java (revision 3723)
+++ user/src/com/google/gwt/user/client/ui/PopupPanel.java (working copy)
@@ -435,6 +435,26 @@
return isAnimationEnabled;
}
+ /**
+ * Returns <code>true</code> if the popup should be automatically hidden when
+ * the user clicks outside of it.
+ *
+ * @return true if autoHide is enabled, false if disabled
+ */
+ public boolean isAutoHideEnabled() {
+ return autoHide;
+ }
+
+ /**
+ * Returns <code>true</code> if keyboard or mouse events that do not target
+ * the PopupPanel or its children should be ignored.
+ *
+ * @return true if popup is modal, false if not
+ */
+ public boolean isModal() {
+ return modal;
+ }
+
public boolean onEventPreview(Event event) {
Element target = DOM.eventGetTarget(event);
@@ -542,6 +562,16 @@
}
/**
+ * Enable or disable the autoHide feature. When enabled, the popup will be
+ * automatically hidden when the user clicks outside of it.
+ *
+ * @param autoHide true to enable autoHide, false to disable
+ */
+ public void setAutoHideEnabled(boolean autoHide) {
+ this.autoHide = autoHide;
+ }
+
+ /**
* Sets the height of the panel's child widget. If the panel's child widget
* has not been set, the height passed in will be cached and used to set the
* height immediately after the child widget is set.
@@ -566,6 +596,16 @@
}
/**
+ * When the popup is modal, keyboard or mouse events that do not target the
+ * PopupPanel or its children will be ignored.
+ *
+ * @param modal true to make the popup modal
+ */
+ public void setModal(boolean modal) {
+ this.modal = modal;
+ }
+
+ /**
* Sets the popup's position relative to the browser's client area. The
* popup's position may be set before calling [EMAIL PROTECTED] #show()}.
*
