shale-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Schofield" <sean.schofi...@gmail.com>
Subject Re: [dialog] Progress on weekend experiments
Date Thu, 31 Aug 2006 23:45:30 GMT
@Craig,

Two questions regarding your initial check in ...

1.) Why doesn't the navigation handler call render response after
setting the view?  Is that something that is frowned upon?  I noticed
the myfaces implementation does this (perhaps incorrectly.)

2.) Why does LegacyContext need a reference to Contexts?  Its
currently not being used.   No big deal - I know its the sandbox - but
did you have something in mind for this reference?

Off to create a few test dialogs.  I will report back shortly.

Sean

On 8/31/06, Sean Schofield <sean.schofield@gmail.com> wrote:
> On 8/28/06, Craig McClanahan <craigmcc@apache.org> wrote:
> > I've checked in some progress on my experiments with how we can improve the
> > Dialog functionality.  Even though they're not done yet, they are far enough
> > along to want some feedback and to make them available for experimentation
> > and further development.  This mail will serve as a record of my thinking so
> > far, so you can get a feel for why I took the approach I've taken so far.
>
> Great.  I think we should do more of this sandbox in progress type
> stuff.  Its easier to contribute your own ideas if the other person
> hasn't nailed everything down yet.
>
> > Initially, I wanted to address two shortcomings and one expanded feature in
> > our current approach:
> >
> > * Make it possible for a single user to have multiple active dialog
> > instances
> >   (although limited to one per JSF view, per our discussion).
>
> Great.  I see this also handles the "aborted" dialog case (user closes
> popup window.)  Nicely done.
>
> > * Make it possible to programmatically start up a dialog instance, as well
> > as
> >   start one with a prefix on a logical outcome.
>
> This will be real nice.  I see how the create method establishes a
> context but will this work in a popup scenario?  User clicks on a
> "Start dialog" link and the dialog starts in a new popup window.  I'm
> thinking one could use a view controller for your pop up window and
> call the necessary create method.  Does the programmer need to find
> the first page of the dialog on their own in this instance?
>
> > * Make it possible to plug in different state machine implementations
> >   (i.e. Commons SCXML), perhaps with different detailed feature sets.
>
> I nice to have bonus.
>
> > I have *not* tried to address synchronizing state when the user presses the
> > browser navigation buttons yet.  One can of worms at a time :-).  Rahul had
> > some comments earlier today on how SWF deals with this.  Seam does something
> > similar, and we'll undoubtedly need to add functionality here later.
>
> Don't worry.  We're not going to solve this anytime soon.
>
> > To address these initial concerns, I focused first on abstracting out the
> > APIs that an application *using* the dialog feature should have to interact
> > with, and then providing generic integration for the navigation handler that
> > can start a new dialog, or operate an existing one.  The guts of actually
> > processing the transitions, then, is delegated to a state-machine-specific
> > implementation.  The key APIs are (so far) incredibly simple -- they are in
> > the "shale-dialog2" module in the sandbox.  There are two fundamental
> > interfaces:
> >
> > org.apache.shale.dialog2.Context:
> >
> > * Represents the current state of an active dialog instance (similar to
> > Status in the current implementation)
> >
> > * One instance per active dialog, rather than one per user
> >
> > * Cached in a o.a.s.dialog2.Contexts object stored in session scope (see
> > below)
> >   in between requests
> >
> > * Generic phase listener (o.a.s.dialog2.faces.Dialog2PhaseListener) is
> > responsible
> >   for saving and restoring access to the right instance:
> >
> >   - Before render response, caches the identifier of the currently ative
> > instance
> >     (if any) in the JSF component tree, as a generic attribute of the view
> > root.
> >
> >   - After restore view, pulls the dialog identifier out, grabs the cached
> > Context
> >     instance from the session scope Contexts object, and exposes it as a
> > request
> >     scope attribute under a well known key ("dialog2" for the moment).
> >
> > * As we did in the original impl, Context exposes a "data" property for the
> > app to
> >   store whatever it wants.  So, binding expressions work very similarly to
> > what they
> >   did before ("#{dialog2.data.name}"), but with the bugfix that it is one
> > instance per view
> >   instead of one instance per user.
> >
> > * Contains an advance() method that can be called to have the state machine
> > work its
> >   way through the transitions until it runs into a state that needs input
> > from the user (in
> >   the terminology of the existing impl, that means when you hit a view
> > state).  The advance
> >   method returns the view id of the page to display, and a generic
> > navigation handler
> >   implementation (o.a.s.dialog2.faces.Dialog2NavigationHandler) then does
> > the actual
> >   JSF navigation.  NOTE - this navigation handler is generally the only code
> > that should
> >   call this method.
>
> Context stuff looks good.  I'm going to try creating a few dialogs
> later tonight to actually test it out.  Maybe even add some test
> cases.
>
> > org.apache.shale.dialog2.Contexts:
>
> The name confuses me.  I would prefer to call it ContextManager or
> something like that.  IMO its too confusing (visually) to have Context
> and Contexts.  They are too similar looking and I find myself stopping
> and saying "Now which one is it again?"
>
> > * Defined by a particular implementation as a session scoped managed bean
> > that is
> >   stored under a well known key.
> >
> > * Maintains a cache of Context instances for the current user, keyed by
> > dialog identifiers,
> >   so we get the one-per-view capability.
> >
> > * Includes a create() method so that you can programmatically start a dialog
> > instance.
>
> The functionality of this class looks good.  Just don't like the name.
>  Is it just me or does the code look a lot cleaner with this
> refactoring?  It seems easier to follow this way.
>
> > The remaining classes in shale-dialog2 implement the JSF integration
> > (navigation handler and phase listener), and should not be utilized directly
> > by applications.
>
> Makes sense.
>
> > To validate this design, I've refactored the current support into a module
> > (shale-dialog2-legacy) that does pretty much the same thing as the existing
> > implementation, but organzied with the new APIs.  (Side note ... Sean is
> > definitely not going to like what I did to the config classes, making
> > everything I could package private ... that's my new habit when designing
> > APIs ... we can make things more public later as needed, but cannot really
> > go the other way.)  As I mentioned in the commit comments, this stuff has
> > only been desk checked and not actually used yet, so it undoubtedly has
> > bugs, but it should be easy to make functional -- and it benefits from the
> > "dialog instance per view" fix in the underlying architecture to fix that
> > problem.
>
> I fixed a DTD issue in your faces-config.xml that my IDE was
> complaining about.  I will put it to an actual test later today.
> Don't worry about the private vs. public business.  If we take care of
> the major problems there will be no need to subclass anymore.
>
> > A corresponding module for Commons SCXML has been started, but I got
> > sidetracked a bit on the legacy one.  It'll be pretty easy to finish this
> > off in a similar fashion, though ... that'll be my next step unless someone
> > wants to volunteer to beat me to it.
>
> I see Rahul has volunteered for this.  Better him then me ;-)  I think
> we should ship 1.0.4 with the "legacy" implementation and ship SCXML
> when its ready.  Also, I think we should change everything from dialog
> to dialog2, etc. once we move out of the sandbox.  I'm assuming this
> "2" and "legacy" stuff is just to keep us from getting confused until
> then?
>
> Nice work.
>
> > Craig
>
> Sean
> >
>

Mime
View raw message