More feedback on Activity/Place and how it would/could/should work with History
Thomas Broyer
I've started prototyping with Activities and can now give some more
feedback on how it compares to my previous own implementation of a
PlaceManager and how I used it with MVP.
First, my PlaceManager was nothing more than the PlaceController found
in GWT 2.1. This meant that when handling PlaceChangeEvents, I
instantiated a new presenter and showed its view right away. The view
generally displayed a "loading..." label while the presenter sent a
request to the server to retrieve the data and only then ask the view
to hide the "loading..." and display the loaded data.
It always floated around in my head that it'd be better to first load
the data and only then switch the views; which happens to be what
GMail does (seems to do), for instance. It makes error handling easier
because when there's an error loading the data (such as when you
temporarily lose your network (wifi?) connection) you're not stuck
with a "loading..." label but still are in front of the same "screen",
where you can click again to retry, or even click elsewhere to finally
do something else. Cancelling XHRs (hitting the Esc key in Firefox
aborts XHRs too, which is highly more probable than a loss of
connectivity) can also be the cause of such behaviors.
When I saw how Activity offers to handle this (async start), I was
really pleased.
Thinking a bit more about it, there's still some to be done, IMO.
1. in ActivityManager, the current activity is first stopped, then the
next activity replaces it and is started asynchronously. This means
that if the next activity (new current one) fails to load, the
activity corresponding to the displayed view is no longer known,
already stopped, and could even have been garbage collected! IMO, the
current activity should only be stopped and "forgotten" when the new
one is fully started (i.e. calls the Display back with a view to
display), no sooner.
It should be as easy as replacing the startingNext boolean with the
reference to the next activity in the ActivityManager, and switching
the current activity for the next one from within the
ProtectedDisplay.
2. The <P extends Place> type parameter should be better documented.
As I understand it from reading the code, this is intended so you can
use an application-specific Place subclass as the base class for all
your Places and it then saves you the cast from Place to
BaseMyAppPlace (or ScaffoldPlace in the scaffold app). Specifically,
it is not intended, as you could think at first sight, to "only see" a
subset of places you're interested in (say you have an activity that
only maps places extending BaseRecordPlace, you shouldn't use an
ActvityManager<BaseRecordPlace> as there's no guarantee you'll only be
passed BaseRecordPlace instances (this is partly because of how Java
generics are implemented, so any experienced Java developper should
get it right). Removing the generics wouldn't hurt that much and would
prevent confusions in what the type parameter means.
3. As implemented now, an Activity can call the Display back several
times. I've found a use case for that but I'd like to know if it's
intended or not (and whichever the outcome, it should probably be
documented). My use case: we have a "dashboard like" interface, with
regions similar to "gadgets". For better UX (performance experience),
we'd like to first display the view with "loading..." placeholders and
asynchronously load the data to show in each region. I thought about
implementing each "gadget" as an Activity, managed by the parent
Activity directly rather than an ActivityManager; i.e. the
DashboardActivity, in its start(), starts each sub-activity, each with
a Display corresponding to a region on the screen, and displays itself
right away. To handle loading errors, the idea is that each sub-
activity displays an "error view" with a "retry" link; when the user
clicks "retry", the request is retried and if successful the Display's
showActivityWidget is called again, this time with the "real" view.
Would this be a "supported use case"? If not, given that it's not
using an ActivityManager, I could always make it work, but then it
would mean the subactivities know that they are subactivities with a
slightly different Display contract than other activities (which means
we'd probably use other interfaces than Activity and Display to avoid
confusions).
4. plumbing with History could only be a matter of calling
PlaceController.goTo on History ValueChange, and calling
History.newItem(token, false) on each PlaceChangeEvent; with some
PlaceMapper or HistoryMapper to map history tokens to places (in my
implementation, I called it HistoryMapper, I also made it a dependency
of the PlaceManager but now think it should just be another component
listener to those events on the event bus). But I'd like things to
happen a bit differently for an even better user experience: similarly
to how I think the current activity should only be stopped when the
new one is fully started, I think the history token should only change
at the time too. This is tricky however because there can be more than
one ActivityManager in an app (or should there be a single one?).
Maybe there should be a "main activity manager" that'd set the history
when its activity is fully loaded? or maybe there should be a third
event to wait for all activities to be fully started (this was my
initial idea in my own implementation, but I don't have a concept of
activity with async start), and event on which ActivityManager (and
anyone interested) would "register", incrementing a counter, and call
back when data is ready, decrementing the counter, the
PlaceChangeEvent only being fired when the counter reaches 0, with the
effect of calling History.newItem and actually showing the views (just
in case there's another PlaceChange requested while data was being
loaded, which currently calls onCancel on the "next" activity).
This is a just an idea though, it wouldn't work when navigating using
the browser's history, so maybe (probably?) it's not worth the
complexity.
5. Finally, and this is more a "support question" than feedback, we
have "contextual" widgets, depending on the current activity. Think of
it as the contextual Ribbon tabs in MS Office 2007: when you're within
a table, table-specific tabs appear in the "global" Ribbon. We also
display a breadcrumb (several levels deep) and some kind of
"breadcrumb overview" on the right, a bit similar to how GMail
displays contextual widgets on the right of conversations, such as
when the displayed conversation contains an appointment, to easily add
it to your calendar. The idea was to create interfaces inheriting
IsWidget, such as HasRibbonTabs and HasBreadcrumbOverview; and on the
Display implementation, display the IsWidget#asWidget in the main
screen "region", then if the IsWidget is an instance of HasRibbonTabs
show other Widgets retrieved from the HasRibbonTabs interface API in
the Ribbon, same thing if the IsWidget is an instance of
HasBreadcrumbOverview. That way, the "activity's main view" can
directly listen on events of the Ribbon tabs' buttons to callback the
activity's Delegate, just as if they were directly on the view. The
alternative would be that each region has an ActivityManager, with
ActivityMappers doing the exact same tests ("are we in this context?")
just to call different factories ("return the appropriate Ribbon
activity" vs. "return the appropriate main activity" vs. "return the
appropriate breadcrumb overview activity"), each activity making
similar requests to get the data (with RequestFactory, they should be
batched and cached so it wouldn't be a bug deal, except in the number
of times we'd have to code the calls to RequestFactory). What do you
think?
Ray Ryan
2. The <P extends Place> type parameter should be better documented.
As I understand it from reading the code, this is intended so you can
use an application-specific Place subclass as the base class for all
your Places and it then saves you the cast from Place to
BaseMyAppPlace (or ScaffoldPlace in the scaffold app). Specifically,
it is not intended, as you could think at first sight, to "only see" a
subset of places you're interested in (say you have an activity that
only maps places extending BaseRecordPlace, you shouldn't use an
ActvityManager<BaseRecordPlace> as there's no guarantee you'll only be
passed BaseRecordPlace instances (this is partly because of how Java
generics are implemented, so any experienced Java developper should
get it right). Removing the generics wouldn't hurt that much and would
prevent confusions in what the type parameter means.
3. As implemented now, an Activity can call the Display back several
times. I've found a use case for that but I'd like to know if it's
intended or not (and whichever the outcome, it should probably be
documented). My use case: we have a "dashboard like" interface, with
regions similar to "gadgets". For better UX (performance experience),
we'd like to first display the view with "loading..." placeholders and
asynchronously load the data to show in each region. I thought about
implementing each "gadget" as an Activity, managed by the parent
Activity directly rather than an ActivityManager; i.e. the
DashboardActivity, in its start(), starts each sub-activity, each with
a Display corresponding to a region on the screen, and displays itself
right away. To handle loading errors, the idea is that each sub-
activity displays an "error view" with a "retry" link; when the user
clicks "retry", the request is retried and if successful the Display's
showActivityWidget is called again, this time with the "real" view.
Would this be a "supported use case"? If not, given that it's not
using an ActivityManager, I could always make it work, but then it
would mean the subactivities know that they are subactivities with a
slightly different Display contract than other activities (which means
we'd probably use other interfaces than Activity and Display to avoid
confusions).
4. plumbing with History could only be a matter of calling
PlaceController.goTo on History ValueChange, and calling
History.newItem(token, false) on each PlaceChangeEvent; with some
PlaceMapper or HistoryMapper to map history tokens to places (in my
implementation, I called it HistoryMapper, I also made it a dependency
of the PlaceManager but now think it should just be another component
listener to those events on the event bus). But I'd like things to
happen a bit differently for an even better user experience: similarly
to how I think the current activity should only be stopped when the
new one is fully started, I think the history token should only change
at the time too. This is tricky however because there can be more than
one ActivityManager in an app (or should there be a single one?).
Thomas Broyer
> Thanks for taking the time to right this up. It's really appreciated.
>
> On Wed, Jul 21, 2010 at 6:13 PM, Thomas Broyer <t.br...@gmail.com> wrote:
>>
>> 1. in ActivityManager, the current activity is first stopped, then the
>> next activity replaces it and is started asynchronously. This means
>> that if the next activity (new current one) fails to load, the
>> activity corresponding to the displayed view is no longer known,
>> already stopped, and could even have been garbage collected! IMO, the
>> current activity should only be stopped and "forgotten" when the new
>> one is fully started (i.e. calls the Display back with a view to
>> display), no sooner.
>> It should be as easy as replacing the startingNext boolean with the
>> reference to the next activity in the ActivityManager, and switching
>> the current activity for the next one from within the
>> ProtectedDisplay.
>
> I don't think I agree with this. I think the odds are that you're leaving an
> activity that thinks it's done now, and may be hard pressed to respond
> reasonably to events.
Right now, the workflow is:
1. currentActivity.mayStop()
2. currentActivity.onStop(), nextActivity.start(),
currentActivity=nextActivity; the currentActivity's view is still
displayed at that point, but it probably no longer has a Delegate
because onStop() is likely to have called setDelegate(null)
3. ... (time passes, waiting for the server to answer the new
activity's request; the previous activity's view is still displayed,
so the user still thinks it can interact with it, cancelling the
previous request)
4. response comes back from server, the new activity's view is finally displayed
What I was describing is:
1. currentActivity.mayStop()
2. nextActvity.start(); nextActivity knows it can be cancelled until
it calls showActivityWidget signaling it's "fully loaded"
3. ... (time passes, waiting for the server to answer the
nextActivity's request; the currentActivity's view is still displayed,
*and* responsive, as the activity hasn't yet been stopped at that
point)
4. response comes back from server, nextActivity calls
showActivityWidget => currentActivity.onStop(),
currentActivity=nextActivity
Now that I've written it down, I think there should be an onStopping()
call on the currentActivity at the time nextActivity is start()ed, on
step 2, so that on step 3 the currentActivity can cancel() the
starting nextActivity without necessarily going to another place.
This is needed because returning null from mayStop() doesn't
necessarily mean onStop will be called (another Activity, in another
ActivityManager, might have returned a non-null value), and returning
non-null doesn't necessarily mean onStop won't be called, because it's
just a mean to ask confirmation to the user. Note that because mayStop
can be called from window.onclosing, onStop might not be called at
all; even if we automatically call onStop in window.onclose, onStop
would have to be very "lightweight": no XHR/RPC/RequestFactory.
In the end, the workfow I'm asking for is:
1. currentActivity.mayStop()
2. currentActivity.onStopping(), nextActvity.start(); nextActivity
knows it can be cancelled until it calls showActivityWidget signaling
it's "fully loaded", and currentActivity knows it will be stopped, and
is passed a "handle" so it can cancel it.
3. ... (time passes, waiting for the server to answer the
nextActivity's request; the currentActivity's view is still displayed,
*and* responsive, as the activity hasn't yet been stopped at that
point; currentActivity can cancel nextActivity's loading, and the
whole navigation altogether)
4. response comes back from server, nextActivity calls
showActivityWidget => currentActivity.onStop(),
currentActivity=nextActivity
I understand it's a major change, which can be hard to coordinate
between multiple ActivityManager. Maybe it should map to following
events in the eventbus:
- PlaceChangeRequest => mayStop
- PlaceChangeAknowledge => onStopping/start
- PlaceChangeCancel => onCancel (should it also tell the "onStopping
activities" that one of them cancelled the move?)
- PlaceChange => only dispatched from showActivityWidget.
This seems closely related to the plumbing with History.
I would understand if you find this too complex and don't implement it
(or even try to); and I wouldn't mind, as you probably have more
experience (or can talk to others at Google with such experience) than
me and know if it's worth it or not.
If it's not though, I'd love to know how you handle errors and network
latency (i.e. latency between start() and showActivityWidget(), where
the view of the previous activity is still displayed –unless you think
about replacing it with a "loading screen", I noticed the TODO in the
code– but the activity already stopped), or if you code as if there
would be no error and network (and server) would be fast (or at least
"fast enough"), as was suggested in the "Architecting for performance
with GWT" talk at I/O.
No. I just thought about using the AbstractRecordListActivity for one
of these "gadgets", so started to think about using Activities; but I
would have done something else eventually if
AbstractRecordListActivity hadn't existed.
Note that I'm nesting activities but not using an ActivityManager, so
it's just a matter of using an existing API and trying not to overload
its intended behavior.
OK, we'll think about it (our approach looked a bit complex when we
started implementing it yesterday), thanks for the feedback.
--
Thomas Broyer
/tɔ.ma.bʁwa.je/