Puts PlaceController in the business of showing the user
"Are you sure" prompts on place change cancellation, per
Thomas Broyer's suggestion
http://groups.google.com/group/google-web-toolkit-contributors/browse_thread/thread/7d96ce234abe67cd
This is the only generalized way I can see to keep the user from
losing changes on window close, etc.
Review at http://gwt-code-reviews.appspot.com/698801
Review by: robertvawter@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8386 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/app/place/AbstractActivity.java b/user/src/com/google/gwt/app/place/AbstractActivity.java
index e474377..90c8777 100644
--- a/user/src/com/google/gwt/app/place/AbstractActivity.java
+++ b/user/src/com/google/gwt/app/place/AbstractActivity.java
@@ -26,14 +26,13 @@
*/
public abstract class AbstractActivity implements Activity {
+ public String mayStop() {
+ return null;
+ }
+
public void onCancel() {
}
public void onStop() {
}
-
- public boolean willStop() {
- return true;
- }
-
}
diff --git a/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java b/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
index 627b1b4..743c47b 100644
--- a/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
+++ b/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
@@ -57,8 +57,9 @@
}
public void cancelClicked() {
- if (willStop()) {
- deltas = null; // silence the next willStop() call when place changes
+ String unsavedChangesWarning = mayStop();
+ if ((unsavedChangesWarning == null) || Window.confirm(unsavedChangesWarning)) {
+ deltas = null; // silence the next mayStop() call when place changes
if (creating) {
display.showActivityWidget(null);
} else {
@@ -67,6 +68,14 @@
}
}
+ public String mayStop() {
+ if (deltas != null && deltas.isChanged()) {
+ return "Are you sure you want to abandon your changes?";
+ }
+
+ return null;
+ }
+
public void onCancel() {
onStop();
}
@@ -146,11 +155,6 @@
}
}
- public boolean willStop() {
- return deltas == null || !deltas.isChanged()
- || Window.confirm("Are you sure you want to abandon your changes?");
- }
-
/**
* Called when the user has clicked Cancel or has successfully saved.
*/
diff --git a/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java b/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
index c1c613d..d063f3f 100644
--- a/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
+++ b/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
@@ -80,6 +80,10 @@
return view;
}
+ public String mayStop() {
+ return null;
+ }
+
public void onCancel() {
onStop();
}
@@ -164,10 +168,6 @@
}
}
- public boolean willStop() {
- return true;
- }
-
protected abstract RecordListRequest<R> createRangeRequest(Range range);
protected abstract void fireCountRequest(Receiver<Long> callback);
diff --git a/user/src/com/google/gwt/app/place/Activity.java b/user/src/com/google/gwt/app/place/Activity.java
index c72abd0..8fdbc9e 100644
--- a/user/src/com/google/gwt/app/place/Activity.java
+++ b/user/src/com/google/gwt/app/place/Activity.java
@@ -37,6 +37,14 @@
}
/**
+ * Called when the user is trying to navigate away from this activity.
+ *
+ * @return A message to display to the user, e.g. to warn of unsaved work,
+ * or null to say nothing
+ */
+ String mayStop();
+
+ /**
* Called when {@link #start} has not yet replied to its callback, but the
* user has lost interest.
*/
@@ -56,6 +64,4 @@
* @param panel the panel to display this activity's widget when it is ready
*/
void start(Display panel);
-
- boolean willStop();
}
diff --git a/user/src/com/google/gwt/app/place/ActivityManager.java b/user/src/com/google/gwt/app/place/ActivityManager.java
index e595583..d1164ae 100644
--- a/user/src/com/google/gwt/app/place/ActivityManager.java
+++ b/user/src/com/google/gwt/app/place/ActivityManager.java
@@ -123,22 +123,8 @@
* @see PlaceChangeRequestedEvent.Handler#onPlaceChangeRequested(PlaceChangeRequestedEvent)
*/
public void onPlaceChangeRequested(PlaceChangeRequestedEvent<P> event) {
- if (!event.isRejected()) {
-
- /*
- * TODO Allow asynchronous willClose check? Could have the event object
- * vend callbacks. Place change doesn't happen until all vended callbacks,
- * if any, reply with yes. Would likely need to add onPlaceChangeCanceled?
- *
- * Complicated, but I really want to keep AM and PC isolated. Alternative
- * is to mash them together and take place conversation off the event bus.
- * And it's still complicated, just very slightly less so.
- *
- * Let's see if a real use case pops up.
- */
- if (currentActivity != null && !currentActivity.willStop()) {
- event.reject();
- }
+ if (currentActivity != null) {
+ event.setWarning(currentActivity.mayStop());
}
}
diff --git a/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java b/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java
index 2f67328..9c5c7ff 100644
--- a/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java
+++ b/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java
@@ -24,7 +24,9 @@
* development, and is very likely to be deleted. Use it at your own risk.
* </span>
* </p>
- * Event thrown when the user may go to a new place in the app. May be rejected.
+ * Event thrown when the user may go to a new place in the app, or tries to
+ * leave it. Receivers can call {@link #setWarning(String)} request that the
+ * user be prompted to confirm the change.
*
* @param <P> the type of the requested place
*/
@@ -33,6 +35,7 @@
/**
* Implemented by handlers of PlaceChangeRequestedEvent.
+ *
* @param <P> the type of the requested Place
*/
public interface Handler<P extends Place> extends EventHandler {
@@ -41,7 +44,7 @@
public static final Type<Handler<?>> TYPE = new Type<Handler<?>>();
- private boolean rejected = false;
+ private String warning;
private final P newPlace;
@@ -56,16 +59,36 @@
return (Type) TYPE;
}
+ /**
+ * @return the place we may navigate to, or null on window close
+ */
public P getNewPlace() {
return newPlace;
}
- public boolean isRejected() {
- return rejected;
+ /**
+ * @return the warning message to show the user before allowing the place
+ * change, or null if none has been set
+ */
+ public String getWarning() {
+ return warning;
}
- public void reject() {
- this.rejected = true;
+ /**
+ * Set a message to warn the user that it might be unwise to navigate away
+ * from the current place, e.g. due to unsaved changes. If the user clicks
+ * okay to that message, navigation will be canceled.
+ * <p>
+ * Calling with a null warning is the same as not calling the method at all --
+ * the user will not be prompted.
+ * <p>
+ * Only the first non-null call to setWarning has any effect. That is, once
+ * the warning message has been set it cannot be cleared.
+ */
+ public void setWarning(String warning) {
+ if (this.warning == null) {
+ this.warning = warning;
+ }
}
@Override
diff --git a/user/src/com/google/gwt/app/place/PlaceController.java b/user/src/com/google/gwt/app/place/PlaceController.java
index f1c29cb..5fd829a 100644
--- a/user/src/com/google/gwt/app/place/PlaceController.java
+++ b/user/src/com/google/gwt/app/place/PlaceController.java
@@ -1,12 +1,12 @@
/*
* Copyright 2010 Google Inc.
- *
+ *
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -15,7 +15,12 @@
*/
package com.google.gwt.app.place;
+import com.google.gwt.core.client.GWT;
import com.google.gwt.event.shared.HandlerManager;
+import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.user.client.Window;
+import com.google.gwt.user.client.Window.ClosingEvent;
+import com.google.gwt.user.client.Window.ClosingHandler;
/**
* <p>
@@ -24,28 +29,95 @@
* </span>
* </p>
* In charge of the user's location in the app.
- *
+ *
* @param <P> the type of places managed
*/
public class PlaceController<P extends Place> {
+
+ /**
+ * Default implementation of {@link Delegate}, based on {@link Window}.
+ */
+ public static class DefaultDelegate implements Delegate {
+ public HandlerRegistration addWindowClosingHandler(ClosingHandler handler) {
+ return Window.addWindowClosingHandler(handler);
+ }
+
+ public boolean confirm(String message) {
+ return Window.confirm(message);
+ }
+ }
+
+ /**
+ * Optional delegate in charge of Window related events. Provides nice
+ * isolation for unit testing, and allows customization of confirmation
+ * handling.
+ */
+ public interface Delegate {
+ HandlerRegistration addWindowClosingHandler(ClosingHandler handler);
+
+ boolean confirm(String message);
+ }
+
private final HandlerManager eventBus;
+ private final Delegate delegate;
private P where;
+ /**
+ * Create a new PlaceController with a {@link DefaultDelegate}.
+ * The DefaultDelegate is created via a call to GWT.create(), so
+ * an alternative default implementation can be provided through
+ * <replace-with> rules in a gwt.xml file.
+ */
public PlaceController(HandlerManager eventBus) {
- this.eventBus = eventBus;
+ this(eventBus, (Delegate) GWT.create(DefaultDelegate.class));
}
+ /**
+ * Create a new PlaceController.
+ */
+ public PlaceController(HandlerManager eventBus, Delegate delegate) {
+ this.eventBus = eventBus;
+ this.delegate = delegate;
+ delegate.addWindowClosingHandler(new ClosingHandler() {
+ public void onWindowClosing(ClosingEvent event) {
+ String warning = maybeGoTo(null);
+ if (warning != null) {
+ event.setMessage(warning);
+ }
+ }
+ });
+ }
+
+ /**
+ * @return the current place
+ */
public P getWhere() {
return where;
}
+ /**
+ * Request a change to a new place. It is not a given that we'll actually get
+ * there. First a {@link PlaceChangeRequestedEvent} will be posted to the
+ * event bus. If any receivers post a warning message to that event, it will
+ * be presented to the user via {@link Delegate#confirm(String)} (which is
+ * typically a call to {@link Window#confirm(String)}). If she cancels, the
+ * current location will not change. Otherwise, the location changes and a
+ * {@link PlaceChangeEvent} is posted announcing the new place.
+ */
public void goTo(P newPlace) {
- PlaceChangeRequestedEvent<P> willChange = new PlaceChangeRequestedEvent<P>(newPlace);
- eventBus.fireEvent(willChange);
- if (!willChange.isRejected()) {
+ String warning = maybeGoTo(newPlace);
+ if (warning == null || delegate.confirm(warning)) {
where = newPlace;
eventBus.fireEvent(new PlaceChangeEvent<P>(newPlace));
}
}
+
+ private String maybeGoTo(P newPlace) {
+ PlaceChangeRequestedEvent<P> willChange = new PlaceChangeRequestedEvent<P>(
+ newPlace);
+ eventBus.fireEvent(willChange);
+ String warning = willChange.getWarning();
+ return warning;
+ }
}
diff --git a/user/test/com/google/gwt/app/AppJreSuite.java b/user/test/com/google/gwt/app/AppJreSuite.java
new file mode 100644
index 0000000..da566a4
--- /dev/null
+++ b/user/test/com/google/gwt/app/AppJreSuite.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.app;
+
+import com.google.gwt.app.place.PlaceChangeRequestedEventTest;
+import com.google.gwt.app.place.PlaceControllerTest;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+/**
+ * Suite of UiBinder tests that require the JRE.
+ */
+public class AppJreSuite {
+ public static Test suite() {
+ TestSuite suite = new TestSuite("app package tests that require the JRE");
+ suite.addTestSuite(PlaceControllerTest.class);
+ suite.addTestSuite(PlaceChangeRequestedEventTest.class);
+ return suite;
+ }
+}
diff --git a/user/test/com/google/gwt/app/place/ActivityManagerTest.java b/user/test/com/google/gwt/app/place/ActivityManagerTest.java
index 1faf89f..73e1ef3 100644
--- a/user/test/com/google/gwt/app/place/ActivityManagerTest.java
+++ b/user/test/com/google/gwt/app/place/ActivityManagerTest.java
@@ -60,7 +60,7 @@
boolean canceled = false;
boolean stopped = false;
Display display = null;
- boolean willStop = true;
+ String stopWarning = null;
MyView view;
SyncActivity(MyView view) {
@@ -80,8 +80,8 @@
display.showActivityWidget(view);
}
- public boolean willStop() {
- return willStop;
+ public String mayStop() {
+ return stopWarning;
}
}
@@ -133,7 +133,7 @@
PlaceChangeRequestedEvent<MyPlace> event = new PlaceChangeRequestedEvent<MyPlace>(
place1);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertNull(realDisplay.widget);
assertFalse(asyncActivity1.stopped);
assertFalse(asyncActivity1.canceled);
@@ -152,7 +152,7 @@
event = new PlaceChangeRequestedEvent<MyPlace>(place2);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertEquals(asyncActivity1.view, realDisplay.widget);
assertFalse(asyncActivity1.stopped);
assertFalse(asyncActivity1.canceled);
@@ -199,7 +199,7 @@
PlaceChangeRequestedEvent<MyPlace> event = new PlaceChangeRequestedEvent<MyPlace>(
place1);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertNull(realDisplay.widget);
assertFalse(asyncActivity1.stopped);
assertFalse(asyncActivity1.canceled);
@@ -213,7 +213,7 @@
event = new PlaceChangeRequestedEvent<MyPlace>(place2);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertNull(realDisplay.widget);
assertFalse(asyncActivity1.stopped);
assertFalse(asyncActivity1.canceled);
@@ -251,12 +251,12 @@
public void testRejected() {
manager.setDisplay(realDisplay);
- activity1.willStop = false;
+ activity1.stopWarning = "Stop fool!";
PlaceChangeRequestedEvent<MyPlace> event = new PlaceChangeRequestedEvent<MyPlace>(
place1);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertNull(realDisplay.widget);
eventBus.fireEvent(new PlaceChangeEvent<Place>(place1));
@@ -264,7 +264,7 @@
event = new PlaceChangeRequestedEvent<MyPlace>(place2);
eventBus.fireEvent(event);
- assertTrue(event.isRejected());
+ assertEquals(activity1.stopWarning, event.getWarning());
assertEquals(activity1.view, realDisplay.widget);
assertFalse(activity1.stopped);
assertFalse(activity1.canceled);
@@ -276,7 +276,7 @@
PlaceChangeRequestedEvent<MyPlace> event = new PlaceChangeRequestedEvent<MyPlace>(
place1);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertNull(realDisplay.widget);
assertFalse(activity1.stopped);
assertFalse(activity1.canceled);
@@ -288,7 +288,7 @@
event = new PlaceChangeRequestedEvent<MyPlace>(place2);
eventBus.fireEvent(event);
- assertFalse(event.isRejected());
+ assertNull(event.getWarning());
assertEquals(activity1.view, realDisplay.widget);
assertFalse(activity1.stopped);
assertFalse(activity1.canceled);
diff --git a/user/test/com/google/gwt/app/place/PlaceChangeRequestedEventTest.java b/user/test/com/google/gwt/app/place/PlaceChangeRequestedEventTest.java
new file mode 100644
index 0000000..7163e22
--- /dev/null
+++ b/user/test/com/google/gwt/app/place/PlaceChangeRequestedEventTest.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.app.place;
+
+import junit.framework.TestCase;
+
+/**
+ * Eponymous test class.
+ */
+public class PlaceChangeRequestedEventTest extends TestCase {
+ private static final String W1 = "foo";
+
+ public void testNoClobberWarning() {
+ PlaceChangeRequestedEvent<Place> e = new PlaceChangeRequestedEvent<Place>(
+ new Place() {
+ });
+
+ assertNull(e.getWarning());
+ e.setWarning(W1);
+ assertEquals(W1, e.getWarning());
+ e.setWarning("bar");
+ assertEquals(W1, e.getWarning());
+ e.setWarning(null);
+ assertEquals(W1, e.getWarning());
+ }
+}
diff --git a/user/test/com/google/gwt/app/place/PlaceControllerTest.java b/user/test/com/google/gwt/app/place/PlaceControllerTest.java
new file mode 100644
index 0000000..5ee19fa
--- /dev/null
+++ b/user/test/com/google/gwt/app/place/PlaceControllerTest.java
@@ -0,0 +1,127 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.app.place;
+
+import com.google.gwt.event.shared.HandlerManager;
+import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.user.client.Window.ClosingEvent;
+import com.google.gwt.user.client.Window.ClosingHandler;
+
+import junit.framework.TestCase;
+
+/**
+ * Eponymous test class.
+ */
+public class PlaceControllerTest extends TestCase {
+
+ private final class Canceler implements
+ PlaceChangeRequestedEvent.Handler<MyPlace> {
+ MyPlace calledWith = null;
+ String warning = "Stop fool!";
+
+ public void onPlaceChangeRequested(PlaceChangeRequestedEvent<MyPlace> event) {
+ calledWith = event.getNewPlace();
+ event.setWarning(warning);
+ }
+ }
+
+ private static class Delegate implements PlaceController.Delegate {
+ String message = null;
+ boolean confirm = false;
+ ClosingHandler handler = null;
+
+ public HandlerRegistration addWindowClosingHandler(ClosingHandler handler) {
+ this.handler = handler;
+ return new HandlerRegistration() {
+ public void removeHandler() {
+ throw new UnsupportedOperationException("Auto-generated method stub");
+ }
+ };
+ }
+
+ public void close() {
+ ClosingEvent event = new ClosingEvent();
+ handler.onWindowClosing(event);
+ message = event.getMessage();
+ }
+
+ public boolean confirm(String message) {
+ this.message = message;
+ return confirm;
+ }
+ }
+
+ private static class MyPlace extends Place {
+ }
+
+ private class SimpleHandler implements PlaceChangeEvent.Handler<MyPlace> {
+ MyPlace calledWith = null;
+
+ public void onPlaceChange(PlaceChangeEvent<MyPlace> event) {
+ calledWith = event.getNewPlace();
+ }
+ }
+
+ private HandlerManager eventBus = new HandlerManager(null);
+ private Delegate delegate = new Delegate();
+ private PlaceController<MyPlace> placeController = new PlaceController<MyPlace>(
+ eventBus, delegate);
+
+ public void testConfirmCancelOnUserNav() {
+ SimpleHandler handler = new SimpleHandler();
+ eventBus.addHandler(PlaceChangeEvent.TYPE, handler);
+
+ Canceler canceler = new Canceler();
+ eventBus.addHandler(PlaceChangeRequestedEvent.TYPE, canceler);
+
+ MyPlace place = new MyPlace();
+
+ placeController.goTo(place);
+ assertNull(handler.calledWith);
+ assertEquals(place, canceler.calledWith);
+ assertEquals(canceler.warning, delegate.message);
+
+ delegate.confirm = true;
+
+ placeController.goTo(place);
+ assertEquals(place, canceler.calledWith);
+ }
+
+ public void testConfirmCancelOnWindowClose() {
+ SimpleHandler handler = new SimpleHandler();
+ eventBus.addHandler(PlaceChangeEvent.TYPE, handler);
+
+ Canceler canceler = new Canceler();
+ eventBus.addHandler(PlaceChangeRequestedEvent.TYPE, canceler);
+
+ assertNull(handler.calledWith);
+ assertNull(delegate.message);
+ delegate.close();
+ assertEquals(canceler.warning, delegate.message);
+ assertNull(handler.calledWith);
+ }
+
+ public void testSimple() {
+ SimpleHandler handler = new SimpleHandler();
+ eventBus.addHandler(PlaceChangeEvent.TYPE, handler);
+ MyPlace place1 = new MyPlace();
+ MyPlace place2 = new MyPlace();
+ placeController.goTo(place1);
+ assertEquals(place1, handler.calledWith);
+ placeController.goTo(place2);
+ assertEquals(place2, handler.calledWith);
+ }
+}
