Thomas Broyer has posted comments on this change.
Change subject: Introduces generic Composite widgets.
......................................................................
Patch Set 3:
(1 comment)
Note that SuggestBox's event forwarding was deprecated in favor of exposing
the internal TextBox, so you should do
suggestBox.getTextBox().addFocusHandler(handler) instead of
suggestBox.addFocusHandler(handler). It breaks encapsulation but I believe
there are cases where it's impractical and causes more harm than good.
PanelComposite is a good example too: panels shouldn't be composites as it
breaks the add(child) vs. getParent() symmetry, and fixing it would require
too much work.
In the end, I tend to think that composites shouldn't expose low-level
events from their wrapped widgets, and shouldn't be containers.
For events, either you only use high-level events such as SelectionEvent
for SuggestBox, ValueChangeEvent for ValueListBox or ValuePicker, or
OpenEvent and CloseEvent for DisclosurePanel.
For panels, you'd rather extend Panel, or not make a Composite and instead
simply implement IsWidget and HasWidgets.ForIsWidget.
Note that we have precedents of composites that are panel and violate the
principle of least surprise for getParent(): DisclosurePanel, TabBar and
TabPanel are such widgets. Our existing widgets that delegate events
however are either broken or have deprecated/removed the delegation.
Conclusion: it's a real mess, do whatever you think is best, but I'd rather
go with only genericized Composite and ResizeComposite, possibly
introducing the 'delegate' concept, and without FocusComposite and
PanelComposite.
....................................................
File user/src/com/google/gwt/user/client/ui/FocusComposite.java
Line 127: return getDelegate().addDoubleClickHandler(handler);
That'd mean making overrideSource public, which I don't think is something
we want to do.
An alternative would be to add some generic helper in
com.google.gwt.event.shared that would internally do the overrideSource and
GwtEvent#dispatch, and then use:
private GwtEventForwardingHelper helper = new
GwtEventForwardingHelper(this);
public HandlerRegistration addDoubleClickHandler(final DoubleClickHandler
handler) {
return getDelegate().addDoubleClickHandler(new DoubleClickHandler() {
@Override
public void onDoubleClick(DoubleClickEvent event) {
helper.dispatch(event, handler);
}
});
}
Actually, it wouldn't even need to be in com.google.gwt.event and reusable:
we could make a local/inner class extending EventBus so it could call
setSourceOfEvent and dispatchEvent:
// extends SimpleEventBus, all we want is accessing 'protected static'
methods from EventBus
private static class GwtEventForwardingHelper extends SimpleEventBus {
public static <H> void dispatch(Composite<?> source, GwtEvent<H> event,
H handler) {
Object oldSource = event.getSource();
setSourceOfEvent(event, source);
try {
dispatchEvent(event, handler);
} finally {
setSourceOfEvent(event, oldSource);
}
}
}
// helper method for Composite subclasses
protected <H> void dispatchEvent(GwtEvent<H> event, H handler) {
GwtEventForwardingHelper.dispatch(this, event, handler);
}
public HandlerRegistration addDoubleClickHandler(final DoubleClickHandler
handler) {
return getDelegate().addDoubleClickHandler(new DoubleClickHandler() {
@Override
public void onDoubleClick(DoubleClickEvent event) {
dispatchEvent(event, handler);
}
});
}
--
To view, visit https://gwt-review.googlesource.com/2582
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Brian Slesinsky <skybr...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Jens Nehlmeier <jens.nehlme...@gmail.com>
Gerrit-Reviewer: Leeroy Jenkins <jenk...@gwtproject.org>
Gerrit-Reviewer: Matthew Dempsky <mdemp...@gwtproject.org>
Gerrit-Reviewer: Thomas Broyer <t.bro...@gmail.com>
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
