Revision: 8386
Author: rj...@google.com
Date: Fri Jul 16 07:47:49 2010
Log: 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: robertvaw...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=8386
Added:
/trunk/user/test/com/google/gwt/app/AppJreSuite.java
/trunk/user/test/com/google/gwt/app/place/PlaceChangeRequestedEventTest.java
/trunk/user/test/com/google/gwt/app/place/PlaceControllerTest.java
Modified:
/trunk/user/src/com/google/gwt/app/place/AbstractActivity.java
/trunk/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
/trunk/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
/trunk/user/src/com/google/gwt/app/place/Activity.java
/trunk/user/src/com/google/gwt/app/place/ActivityManager.java
/trunk/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java
/trunk/user/src/com/google/gwt/app/place/PlaceController.java
/trunk/user/test/com/google/gwt/app/place/ActivityManagerTest.java
=======================================
--- /dev/null
+++ /trunk/user/test/com/google/gwt/app/AppJreSuite.java Fri Jul 16
07:47:49 2010
@@ -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;
+ }
+}
=======================================
--- /dev/null
+++
/trunk/user/test/com/google/gwt/app/place/PlaceChangeRequestedEventTest.java
Fri Jul 16 07:47:49 2010
@@ -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());
+ }
+}
=======================================
--- /dev/null
+++ /trunk/user/test/com/google/gwt/app/place/PlaceControllerTest.java Fri
Jul 16 07:47:49 2010
@@ -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);
+ }
+}
=======================================
--- /trunk/user/src/com/google/gwt/app/place/AbstractActivity.java Thu Jun
24 14:48:00 2010
+++ /trunk/user/src/com/google/gwt/app/place/AbstractActivity.java Fri Jul
16 07:47:49 2010
@@ -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;
- }
-
-}
+}
=======================================
---
/trunk/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
Thu Jun 24 14:48:00 2010
+++
/trunk/user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
Fri Jul 16 07:47:49 2010
@@ -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 {
@@ -66,6 +67,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();
@@ -145,11 +154,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.
=======================================
---
/trunk/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
Fri Jul 2 09:26:53 2010
+++
/trunk/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
Fri Jul 16 07:47:49 2010
@@ -79,6 +79,10 @@
public RecordListView<R> getView() {
return view;
}
+
+ public String mayStop() {
+ return null;
+ }
public void onCancel() {
onStop();
@@ -163,10 +167,6 @@
break;
}
}
-
- public boolean willStop() {
- return true;
- }
protected abstract RecordListRequest<R> createRangeRequest(Range range);
=======================================
--- /trunk/user/src/com/google/gwt/app/place/Activity.java Thu Jun 24
14:48:00 2010
+++ /trunk/user/src/com/google/gwt/app/place/Activity.java Fri Jul 16
07:47:49 2010
@@ -35,6 +35,14 @@
public interface Display {
void showActivityWidget(IsWidget widget);
}
+
+ /**
+ * 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
@@ -56,6 +64,4 @@
* @param panel the panel to display this activity's widget when it is
ready
*/
void start(Display panel);
-
- boolean willStop();
-}
+}
=======================================
--- /trunk/user/src/com/google/gwt/app/place/ActivityManager.java Thu Jun
24 14:48:00 2010
+++ /trunk/user/src/com/google/gwt/app/place/ActivityManager.java Fri Jul
16 07:47:49 2010
@@ -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());
}
}
=======================================
--- /trunk/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java
Thu Jun 24 14:48:00 2010
+++ /trunk/user/src/com/google/gwt/app/place/PlaceChangeRequestedEvent.java
Fri Jul 16 07:47:49 2010
@@ -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
=======================================
--- /trunk/user/src/com/google/gwt/app/place/PlaceController.java Thu Jun
24 14:48:00 2010
+++ /trunk/user/src/com/google/gwt/app/place/PlaceController.java Fri Jul
16 07:47:49 2010
@@ -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, (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;
+ }
+}
=======================================
--- /trunk/user/test/com/google/gwt/app/place/ActivityManagerTest.java Thu
Jun 24 14:48:00 2010
+++ /trunk/user/test/com/google/gwt/app/place/ActivityManagerTest.java Fri
Jul 16 07:47:49 2010
@@ -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);
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
