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&lt;/my:ActionButton></pre></code>
Per below, I find eventSourceName a little confusing, and think a
subclass that had a specific "actionName" would be more straightforward,
e.g.:

<my:ActionButton actionName="delete"/>

You could also get fancy and make a class AppActions, with a method for
each action, e.g. delete() return ActionNames.DELETE; and then do:

<ui:with field=a type=...AppActions/>
<my:ActionButton actionName="{a.delete}"/>

(This is likely obvious, compile time checks for your action names, but
I point it out because I used a similar technique in our app and so am a
bit proud of myself for it. :-)

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> {
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".

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.

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 don't fully understand the sample app yet, so perhaps I'm missing
things.

(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.)

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);
So, just as a sanity check, this results in all widget events being
broadcast on the app-wide event bus, right?

I ran the sample and was seeing CellPreviewEvents,
LoadingStateChangedEvents, etc., from DataGrid/CellList/etc. all going
onto the event bus.

This seems somewhat at odds to my conceptual notion of the event
bus--that it should be for application-/business-specific events and not
for general widget chatter.

Perhaps I'm missing things, but I think this would get quite chatty.
Perhaps that isn't a perf concern, but it seems, conceptually, a little
odd.

Again, disclaimer, I need to play with the sample app some more, but
mixing the world of widgets/widget events/"browser stuff"
(HandlerManager) and presenter/presenter events/"business stuff"
(EventBus) doesn't seem ideal.

http://gwt-code-reviews.appspot.com/1449817/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to