Thanks as always for the thoughtful feedback.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/client/event/ActionEvent.java File samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/client/event/ActionEvent.java (right): http://gwt-code-reviews.appspot.com/1449817/diff/4001/samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/client/event/ActionEvent.java#newcode36 samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/client/event/ActionEvent.java:36: * <my:ActionButton eventSourceName="delete">Delete</my:ActionButton></pre></code> Yeah, clearly it should be an object and not a string. http://gwt-code-reviews.appspot.com/1449817/diff/4001/samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/client/event/ActionEvent.java#newcode43 samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/client/event/ActionEvent.java:43: public class ActionEvent extends GwtEvent<ActionEvent.Handler> { Remember that this is just a quick sketch, there is a reason it's not in GWT yet, if ever. Re: the abuse of source, I think it's more that source has evolved into a misnomer in our event system, and we're stuck with it. Source is our second level key when registering handlers, so it's going to get twisted. e.g. RequestFactory uses this by setting proxy class objects as the source of its change events, to allow people to subscribe only to change notices for particularly types of objects. On 2011/06/16 16:48:48, stephenh wrote:
It seems odd to have an ActionEvent that doesn't have the current
action (e.g.
"addTask") as an "action" field. IMO, using the event bus source name
is tricky
and abuses the concept of "source".
My inclination would be to add a getAction() method to the event that is an alias for getSource().
Thinking of use cases, if I wanted to be logging all of the
ActionEvents that
happened in my app, it'd be nice to have an actionEvent.getAction()
method
instead of having to register multiple handlers with the source as
each
ActionName.
That would work here, just need to expose a register(EventBus, Handler) method. Done.
Also, if a button/widget wanted to change which action it was firing
based on
some conditional logic, it seems it'd be a lot easier to do ActionEvent.fire(..., thisAction || thatAction) in it's handler
instead of
changing the widget's event source name on the fly.
I would just add methods like setAddSourceName(), setBarSourceName() to such a widget. It's not like you can only use the built in one.
I don't fully understand the sample app yet, so perhaps I'm missing
things. Its in a bit of turmoil.
(I would go further and assert that, IMO, the notion of "event source"
shouldn't
be baked into the event bus anyway. Event buses are, again IMO, about broadcasting app-wide events such that you shouldn't care where they
came from
or who fired them. That's the point of the event bus--decoupling where
events
come from. If you need more semantics than "some event happened", it's
probably
clearer to encode it in the Event itself, e.g. with actionName. Then
you can
have a strong type, enum even, instead of the Object source.)
Again, we're back to source-is-misnomer. http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/Widget.java File user/src/com/google/gwt/user/client/ui/Widget.java (right): http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/Widget.java#newcode152 user/src/com/google/gwt/user/client/ui/Widget.java:152: getEventBus().fireEventFromSource(event, eventSourceName); Don't worry that I'm going to submit this patch half baked. It's sparked a lot of good discussion which will take time to resolve. (I'm about to take a long vacation, though, so the conversation will drag a bit.) I'm trying to create a gradual adoption slope for this style of coding, to make the eventbus accessible without forcing people to come up with an entire app architecture, so this retrofit seemed slick. I do worry about the chatter. But on the other hand, any event that a widget is posting is already one that has declared those events as part of its public API. And by providing the setEventBus method we leave the door open for more sophisticated apps to take control over who gets to say what where. E.g., a presenter could tell its view to post only to a narrow event bus the presenter provides: view.setEventBus(ourPrivateBus); view.setDriver(this); Hmm. For that matter you could imagine code generating such a bus. Suddenly every object can have @UiField without needing a ui.xml for it. Holy cow, I like that a lot. Okay, in the course of typing this I've gone from wavering on this approach to being prepared to dig in my heels very hard. http://gwt-code-reviews.appspot.com/1449817/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
