[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: Yes, as Brian tried to explain, I ended up with this design after playing with plenty of alternatives and uses cases; at the end this is the one that I am most comfortable with. I'm open to other suggestions that would help us to write good reusable XYZComposite classes (inside or outside SDK) and still reasonable to use with UIBinder. Please play with them in your IDE and try to see how it will look like in practice for different use cases. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: Hi, Goktug and I talked about this. Goktug is pretty sure that delegation is necessary to make FocusComposite work properly, but it's rather difficult to explain why, so the conversation is getting frustrating. So let's put this patch aside for a while. It would be helpful for someone to download the FocusComposite patch and attempt to make it work without delegation in Composite. Then we can compare solutions and go from there. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: -Code-Review re: "In the GWT SDK, we don't have those reusable classes so we are also duplicating the code in a poor way. ValueListBox received the second patch since I joined the team." I found one patch: http://gwt-code-reviews.appspot.com/1884803/ It seems pretty straightforward. What's the other patch? -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: When you call initWidget(Widget,T), you already have a variable holding the delegate (because you have to put it as a child of the "root widget"), and changing that variable to a field to make it accessible elsewhere in the class is a no-brainer (and if you use UiBinder to construct your Composite's internals, you already have that field as a @UiField, so the concept of delegate at the Composite level only comes as a duplicate). For simple composites, they will use the single initWidget and they might use the getDelegate for access to base widget but this is not really the use case to go with getDelegate and this is NOT why I introduced it. The only use case for getDelegate is when you have someone extending your Composite, so they can gain direct access to that "central widget"; but a) extending a Composite is probably an anti-pattern and b) exposing your widget to subclasses is a no-brainer too. I think you totally missed the point. This is the reason why I'm saying we are going in circles. The conversation became too long and the original intention for the change started to get lost. I started working on this is, because it is not to possible safely extend Composite to introduce more reusable classes (inside or outside of SDK) that does more delegations (ResizeComposite, FocusComposite, PanelComposite, IndexPanelComposite etc). In our internal code there are already multiple of them, duplicated in different projects and written poorly. In the GWT SDK, we don't have those reusable classes so we are also duplicating the code in a poor way. ValueListBox received the second patch since I joined the team. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: Code-Review-1 (1 comment) I tend to agree with Jens here (now that we see that in isolation, without the FocusComposite and PanelComposite) When you call initWidget(Widget,T), you already have a variable holding the delegate (because you have to put it as a child of the "root widget"), and changing that variable to a field to make it accessible elsewhere in the class is a no-brainer (and if you use UiBinder to construct your Composite's internals, you already have that field as a @UiField, so the concept of delegate at the Composite level only comes as a duplicate). The only use case for getDelegate is when you have someone extending your Composite, so they can gain direct access to that "central widget"; but a) extending a Composite is probably an anti-pattern and b) exposing your widget to subclasses is a no-brainer too. I really think what we want is to have getWidget() return some 'T extends Widget'. File user/src/com/google/gwt/user/client/ui/ResizeComposite.java Line 25: public abstract class ResizeCompositeRequiresResize> The problem here is that actually you'll want getWidget to implement RequiresResize, not necessarily getDelegate (the goal of ResizeComposite is to be able to use a RequiresResize widget as the "root widget"). What we want here is more: public abstract class ResizeCompositeextends Widget> extends Composite implements RequiresResize and that actually doesn't simplify the implementation of ResizeComposite (it would if Composite had 2 type parameters, for the "root widget" and the delegate). -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: We are really going in circles. The delegate is part of Composite because of the same reason why somebody cannot safely introduce generics by extending this class. There is really nothing complicated about the basic use case and I don't think it will cause confusion. People don't need to know the delegate concept until they need to know about it. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Jens Nehlmeier has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: So if the delegate feature sounds nice but has the potential to create confusion, why should it be added to GWT? Making getWidget()/initWidget() typed would also solve the issues that this CL references. A developer that wants that delegation feature can still have it by extending an "ordinary" typed composite and introduce the code that Goktug has introduced to support a delegate. After this discussion I dont really see the point of adding a delegation feature to Composite. I am willing to give a +1 if anyone can convince me that its worth it to have the delegate concept in GWT proper, so it can be used by everyone, although we now know it can cause confusions. Otherwise I will probably vote -1. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: Code-Review+2 -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: Goktug and I talked about this. I'll try to summarize: - How did this get so complicated? UIBinder requires a zero-argument constructor and also requires you to pass "this" to its methods. So we cannot use constructors for initialization, and that's why we have initWidget() in the first place. - Why not have a separate class that supports delegates? Because that would force PanelComposite and FocusComposite to make a decision on which class to inherit from. We want to defer that decision to the subclass when it calls either the one or two-argument version of initWidget(). - Why not have Composite? Because in the simple case we only want one type variable for the wrapped widget. It looks a little weird and it's a bit hard to explain, but examples make sense. - Can we make delegation transparent for parent-child relationships, similar to web components? Probably not because lots of things would break. (Maybe we should update the documentation on HasWidgets to say that it adds its argument as a descendant.) - Can we make delegation of event handlers transparent? Probably not a good idea, because it adds complexity and relatively few event handlers check the source. (Maybe update the addFocusHandler/addBlurHandler javadoc to say that the source may be a descendant?) -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 3: (1 comment) File user/src/com/google/gwt/user/client/ui/FocusComposite.java Line 127: return getDelegate().addDoubleClickHandler(handler); Hmm, I thought they were in the same package. Your suggestion is not keeping track of previous addHandler calls. So it will be causing dublicate events for each addXXX calls. Also we don't need GwtEventForwardingHelper as calling fireEvent will do the trick. Am I missing anything? (Whatever we do, it would be great to encapsulate it as helper for composite to make it straightforward for other subclasses.) -- 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 Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: (1 comment) To be clear, nobody needs to do any migration, this change is backward compatible. File user/src/com/google/gwt/user/client/ui/ResizeComposite.java Line 25: public abstract class ResizeCompositeRequiresResize> I may have not understood your question correctly but a subclass looking for different semantics can extend from Composite directly and enforce its own semantics. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: (1 comment) File user/src/com/google/gwt/user/client/ui/ResizeComposite.java Line 25: public abstract class ResizeCompositeRequiresResize> Looking at this example, the main advantages I see are that (a) you don't need to define a constructor in this class to enforce its constraints, and (b) we're restricting the type on the one-argument version of initWidget(), so subclassing is a bit less error-prone. But this will be an internal detail of the class that gets constructed. But what if the subclass wants a delegate and RequiresResize at the same time? Then we're enforcing a constraint on the delegate, not the widget. -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 5: re: "We cannot get rid of the mass by ignoring it - and not taking an action is exactly that." Well, not taking action does have the advantage that nobody needs to migrate any code. But I agree with fixing things; I just want to make sure that any change we make works well and isn't a partially-working thing that people need to learn. We should understand it well enough that we can easily explain to other people why they should use it and it won't be "it seemed like a good idea at the time but I don't fully understand it." -- 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: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Jens Nehlmeier has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 4:
@Brian: You can use UiBinder to define a custom Panel easily, the only
downside is that you can not use GWT widgets in the UiBinder as the
UiBinder root element must be a DOM Element, so you can call setElement().
Rough example:
SlidingPanel Headline
SlidingPanel extends ComplexPanel {
@UiField DivElement childContainer;
public SlidingPanel() {
setElement(uiBinder.createAndBindUi(this));
}
@Override
pubic void add(Widget w) {
add(w, childContainer.cast()); //protected method of
ComplexPanel
}
}
So if you want to add some interaction controls like buttons you must do it
using the low level Element API. Thats why you probably always choose
Composite in combination with UiBinder because then you can choose
HTMLPanel as UiBinder root which allows you to add DOM elements as well as
widgets.
The delegate concept of this patch pretty much replicates the protected
ComplexPanel.add(Widget w, Element childContainer) method. But the delegate
concept in this patch can break Widget.getParent() because it operates on
widget level while ComplexPanel.add(Widget w, Element childContainer) does
not as it uses a DOM element as target container.
--
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: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: No
--
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Hello Matthew Dempsky, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/2582 to look at the new patch set (#5). Change subject: Introduces generic Composite widgets. .. Introduces generic Composite widgets. Composite can now be 'safely' extended to add delegation to methods for more specific types. Improves error messaging for incorrect use. Fixes issue 4665, issue 6997. Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Review-Link: https://gwt-review.googlesource.com/#/c/2582/ --- M user/src/com/google/gwt/user/client/ui/Composite.java M user/src/com/google/gwt/user/client/ui/ResizeComposite.java 2 files changed, 83 insertions(+), 32 deletions(-) -- To view, visit https://gwt-review.googlesource.com/2582 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Gerrit-PatchSet: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: 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. This is mostly true, composite will usually only need to expose logical events but I don't think it is always practical. Why an interface / functionality for ValueListBox should be any different than ListBox? If somebody needs focus events for ListBox and then it means they also need it for ValueListBox, this is true for any composite that does single encapsulation. 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. Yes exactly. We are already broken in many places and there is a huge mess. As our own code is broken that means everybody else's code is also broken. We cannot get rid of the mass by ignoring it - and not taking an action is exactly that. There are parts that are fixable (event source) and also there are other parts that is not much practical to fix with current design (getParent symmetry). Parent symmetry also probably broken for any other widget system that uses composition for reuse. But use case is unavoidable as can be seen from our very own SDK (and based on my previos experience with Swing). If we don't provide the right tools then people are causing even more mess like extending a panel instead of composite and only forward one or two methods and missing the rest (like removals or other 'add' methods). What I'm trying to do here is to provide a way for both our own code and user code to do this in less messy way and I think even in it current state it is already an improvement. -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: Since we already have ResizeComposite, I'm not sure that's a good example of how this change is an improvement? It seems like we should have another example. For custom panels, how well does extending Panel work? Suppose you want a fancy panel that's based on a UIBinder template and can be used in a UIBinder template, with a hole where the children should go and some extra widgets that are encapsulated and aren't exposed as children in the panel API. How hard is that to do? What's the best way to make that easy? -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
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 void dispatch(Composite source, GwtEvent event,
H handler) {
Object oldSource = event.getSource();
setSourceOfEvent(event, source);
try {
dispatchEvent(event, handler);
} finally {
setSourceOfEvent(event, oldSource);
}
}
}
// helper method for Composite subclasses
protected void dispatchEvent(GwtEvent 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
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: This looks like exactly what I have proposed; you might have missed some of the discussion. It is never safe to keep a reference to an event as domevent are implemented with flyweight pattern in GWT SDK; so I don't think that is an issue. -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 4:
It might be possible to intercept the events and update the source after
they're generated:
class ClickSourceFixer extends ClickHandler {
ClickHandler wrapped;
Object newSource;
void onClick(Event e) {
Object oldSource = e.getSource();
e.setSource(newSource);
try {
wrapped.onClick(e);
} finally {
e.setSource(oldSource);
}
}
}
When someone calls foo.addEventHandler, the composite could wrap the
handler before passing it to the delegate.
But this isn't transparent because someone might keep a reference to the
event. To fix that we'd have to copy the event. If the composite has its
own set of event handlers then the event only has to be translated once.
Perhaps we could have a ForwardingHandlerManager that does this.
--
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: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: No
--
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: I played with a separate method but I didn't find something I like. Actually, I quite like our current APIs. Question is, if we would like to pay the price of source correction overhead (more code + more complexity) for events. I would expect parent to work as expected in Web Component because that is part of their promise; from outside it should work like any other html tag. I remember from one of their talks that they are correctly handling the events. -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: Fair enough - I don't understand the particular use cases. My point is that we should make sure they work before committing to an API. It seems like for the UIBinder case we're talking about PanelComposite and in that case the delegate is straightforward - it's the place where children are added. (It will still break encapsulation somewhat because when the child asks for its parent it will be different than what you'd expect.) I wonder what web components do? I don't know if we can achieve complete encapsulation given our existing API's. Probably the best to hope for would be to avoid being too surprising. About the current API: do we necessarily want the delegate to be passed to initWidget? Maybe there should be a separate setDelegate() method instead. -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: It doesn't work with UIBinder because you are changing the expected semantics of a widget. So even without UIBinder case, this is still how it suppose to work. You need to think in term of consumption. If a composite has multiple focusable objects and wants to expose focus API, it has different options depending on the specific scenario. In a SuggestBox case, it would use the input the field as the logical reference for Focus from outside point of view even though autocomplete popup might be focusable. For another widget that has multiple inputs and it really wants to really provide a Focus API, it has two options. Either it needs to synthesize it in a way that it looks like a single focusable piece from outside or it will need implement additional APIs in addition to HasFocus. Being said that, I don't ever remember trying to implement Focus interface on a UIBinder composed view. Focus concept is more intended to be used with widgets in contrast to a complex view. -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: Fair enough, it has to work with UiBinder. Though I suppose UiBinder could check for a delegate (or some such interface) as a special case. What do we do when a composite has multiple focusable fields? It seems like the HasFocus interface implies a single field, but we would also want to support UiBinder for composite widgets that have multiple fields? -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Jens Nehlmeier has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: I have once written a widget that does this foo.getDelegate().addXyzHandler() style but it comes with the cost that you can not use @UiHandler with it. If you are used to UiBinder its pretty annoying to not have @UiHandler support for a widget so at the end I actually added foo.addXyzHandler() methods to the widget and forward them to the delegate and obviously keep getDelegate() public so you are able to match against the event source. It works ok'ish once everyone gets used to this style but API wise it just doesn't feel that great. First, the "problem" with making it UiBinder/UiHandler compatible is that developers just don't read the JavaDoc of the forwarding method foo.addXyzHandler() which states that they should use foo.getDelegate() when matching event sources. Thats because they don't call the method on their own when using UiHandler. Secondly, once you have these forwarding methods and a public getDelegate() you end up with two ways of adding handlers. So I doubt that making Composite.getDelegate() public is a good choice. You end up with widgets that are similar to SuggestBox and DateBox that give access to their internals (which may result in deprecated methods in the future like SuggestBox.getTextBox()). -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: Composites are an encapsulation mechanism. For a widget, being a composite is an implementation detail that it doesn't necessarily want to expose outside. That is the reason in many case why they are going into all this trouble in the first place. ValueListBox and SuggestBox from SDK are good examples for that. What you are suggesting both breaks encapsulation and also other things like UiHandler or using Composite with widgets that are implemented using Composites. If a composite want to expose its details, they can easily do that by providing an accessor to the internal widget and people can add handlers to that and the source would be the internal widget as expected. -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 4: We should decide how we're going to delegate events, since delegation is the whole point of this change. On the one hand, when a handler receives a focus event, I would expect the event source to be the actual widget that received focus, and not a container for it. On the other hand, it's weird to add to add an event handler to one widget and have it actually be placed on a different widget. It seems like in the calling code, it would be clearer to be explicit and say foo.getDelegate().addFocusListener(...). Or similarly, if you're calling a method that takes a HasFocusable, you should pass in foo.getDelegate(). This suggests we might not need FocusComposite at all, and instead the subclass should make getDelegate() public, possibly using a forwarding method with a more descriptive name. What was the original use case? -- 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: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 3:
(1 comment)
Agreed. The patch is growing; I will take the FocusComposite and
PanelComposite out but unfortunately that will cause the discussion history
to be lost for future reference.
File user/src/com/google/gwt/user/client/ui/FocusComposite.java
Line 127: return getDelegate().addDoubleClickHandler(handler);
Actually, I was thinking of a simpler route but I still don't like the code
bloat:
public HandlerRegistration addDoubleClickHandler(DoubleClickHandler
handler) {
return getDelegate().addDoubleClickHandler(new DoubleClickHandler() {
@Override
public void onDoubleClick(DoubleClickEvent event) {
Object oldSource = event.getSource();
event.overrideSource(this);
try {
handler.onDoubleClickEvent(event);
} finally {
event.overrideSource(oldSource);
}
}
});
}
Are you sure, this will fire the events twice? I don't see a reason.
It looks like in the SuggestBox case, the event is also delegated from the
handler:
http://gwt-code-reviews.appspot.com/1785803/
--
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
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 3:
(1 comment)
I'd vote for removing FocusComposite from this change, to get it merged
early, and adding it in its own change.
File user/src/com/google/gwt/user/client/ui/FocusComposite.java
Line 127: return getDelegate().addDoubleClickHandler(handler);
Yes, that'd mean tracking, for each event, whether the handler was added to
the delegate widget, something like:
private boolean dblClickHandlerRegistered;
…
public HandlerRegistration addDoubleClickHandler(DoubleClickHandler
handler) {
if (!dblClickHandlerRegistered) {
getDelegate().addDoubleClickHandler(new DoubleClickHandler() {
@Override
public void onDoubleClick(DoubleClickEvent event) {
fireEvent(event);
}
});
dbleClickHandlerRegistered = true;
}
return addHandler(handler, DoubleClickEvent.getType());
}
That's a lot of boilerplate and a bit of runtime overhead; and it doesn't
remove the anonymous handler from the delegate when all the handlers are
removed from the composite; this would require tracking the number of
handlers added –unfortunately I don't think we can use getHandlerCount here
as it doesn't account for queued adds/removes, when adding/removing
handlers from within an event handler–, storing the HandlerRegistration for
the anonymous handler, and wrapping the HandlerRegistration returned from
addHandler() to remove the anonymous handler when the handler count reaches
0.
Note that instead of an anonymous class, we could have an inner class
implementing all the event handlers; similar to SuggestBox. I have no idea
what the compiler would optimize best.
Last, but not least, it'll possibly fire the events twice when
getWidget()==getDelegate(); see issue 3533.
--
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
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Hello Matthew Dempsky, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/2582 to look at the new patch set (#4). Change subject: Introduces generic Composite widgets. .. Introduces generic Composite widgets. Composite can now be 'safely' extended to add delegation to methods for more specific types. Adds FocusComposite and PanelComposite. Improves error messaging for incorrect use. Fixes issue 4665, issue 6997. Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Review-Link: https://gwt-review.googlesource.com/#/c/2582/ --- M user/src/com/google/gwt/user/client/ui/Composite.java A user/src/com/google/gwt/user/client/ui/FocusComposite.java A user/src/com/google/gwt/user/client/ui/PanelComposite.java M user/src/com/google/gwt/user/client/ui/ResizeComposite.java 4 files changed, 384 insertions(+), 32 deletions(-) -- To view, visit https://gwt-review.googlesource.com/2582 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Gerrit-PatchSet: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 3:
(5 comments)
File user/src/com/google/gwt/user/client/ui/Composite.java
Line 32: * calls to the wrapped widget (e.g. HasWidgets APIs). Also,
alternatively, subclasses can choose a
Done
Line 36: * Example {@example
com.google.gwt.examples.CompositeExample}
Yes. I'll send another patch that converts all trivial Composite usages to
Composite.
Line 127:* Provides subclasses access to the widget that higher level
APIs are
I was keeping the wording similar with getWidget. Will update both.
Line 140:*
Done
File user/src/com/google/gwt/user/client/ui/FocusComposite.java
Line 127: return getDelegate().addDoubleClickHandler(handler);
For this to work as expected (i.e. source == composite and ideally with
possible custom addXXXHandler impls at delegate), I will need to create
anonymous class that changes the source for each handler type; I'm not sure
that is what we want.
I will think little bit more on alternatives but I'm not very optimistic
about a good solution...
--
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
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Jens Nehlmeier has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 3: (1 comment) File user/src/com/google/gwt/user/client/ui/FocusComposite.java Line 127: return getDelegate().addDoubleClickHandler(handler); Oh good catch. I think the behavior of event.getSource() should stay the same in all cases. It should be easy to migrate to this new FocusComposite and the usage of FocusComposite should not break your app if you ever decide to switch from initWidget(Widget) to initWidget(Widget, T) because you want to add some decoration. So I guess it should be this.addHandler(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 Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 3:
(4 comments)
Composite looks good. Some javadoc tweaks:
File user/src/com/google/gwt/user/client/ui/Composite.java
Line 32: * calls to the wrapped widget (e.g. HasWidgets APIs). Also,
alternatively, subclasses can choose a
"Also, alternatively," -> "As an alternative,"
Line 36: * Example {@example
com.google.gwt.examples.CompositeExample}
Do we need to update the example?
Line 127:* Provides subclasses access to the widget that higher level
APIs are
"Returns the delegate widget passed to initWidget(). Subclasses may forward
API calls to this delegate."
Line 140:*
"The same widget will be wrapped and used as a delegate. if you want to use
a different delegate, call initWidget(Widget, T) instead."
--
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
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 3: (1 comment) File user/src/com/google/gwt/user/client/ui/FocusComposite.java Line 127: return getDelegate().addDoubleClickHandler(handler); Er (sorry for not catching it earlier), shouldn't these somehow use this.addXxxHandler and fireEvent? As coded here, the event passed to the handlers will have getDelegate() as its getSource(), instead of the FocusComposite. This unfortunately adds some overhead so I could udnerstand that you wouldn't want it, but then it should be clearly documented, as it's likely to surprise a lot of people (all those using the same handler for several widgets –e.g. through @UiHandler– and doing "if te.getSource() == someWidget)" kind of tests) -- 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 Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Hello Matthew Dempsky, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/2582 to look at the new patch set (#3). Change subject: Introduces generic Composite widgets. .. Introduces generic Composite widgets. Composite can now be 'safely' extended to add delegation to methods for more specific types. Adds FocusComposite and PanelComposite. Improves error messaging for incorrect use. Fixes issue 4665, issue 6997. Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Review-Link: https://gwt-review.googlesource.com/#/c/2582/ --- M user/src/com/google/gwt/user/client/ui/Composite.java A user/src/com/google/gwt/user/client/ui/FocusComposite.java A user/src/com/google/gwt/user/client/ui/PanelComposite.java M user/src/com/google/gwt/user/client/ui/ResizeComposite.java 4 files changed, 375 insertions(+), 27 deletions(-) -- To view, visit https://gwt-review.googlesource.com/2582 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Gerrit-PatchSet: 3 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer -- 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 2:
(6 comments)
File user/src/com/google/gwt/user/client/ui/Composite.java
Line 29: * methods. When added to a panel, a composite behaves exactly as
if the widget
Done
Line 37: * @param type of the widget wrapped
Updated docs.
@tbroyer: it is better to defer removal of setWidget to a separate patch.
Line 127: public T getDelegate() {
Opps, good catch! I wasn't intending to make it public.
For the final part, I will keep it non-final due to the
non-final 'getWidget' pair.
Line 148:*
Done
Line 151:*will be delegated (e.g. HasWidget#add)
Done
File user/src/com/google/gwt/user/client/ui/FocusComposite.java
Line 52: HasAllMouseHandlers, HasAllGestureHandlers,
HasAllTouchHandlers {
Done. It looks like I also missed HasEnabled which I added now.
--
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: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 2: (1 comment) File user/src/com/google/gwt/user/client/ui/Composite.java Line 37: * @param type of the widget wrapped Well, setWidget() is a deprecated alias to initWidget, and initWidget(T) is an alias for initWidget(Widget,T) where the argument is passed as both the "root widget" and the "delegate widget"; so while it looks a bit weird (when you look at the signatures of setWidget vs. getWidget), it's actually consistent. I however agree the javadoc could be enhanced to talk about that new concept of 'delegate'. And maybe it's time to delete setWidget(), it's deprecated since the very first public commit more than 6 years ago! -- 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: 2 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Jens Nehlmeier Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Jens Nehlmeier has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 2:
(4 comments)
File user/src/com/google/gwt/user/client/ui/Composite.java
Line 127: public T getDelegate() {
protected final instead of public. I dont think a composite wants to leak
out "T".
Line 148:*
Maybe add an example to illustrate the purpose (for new GWT users).
Line 151:*will be delegated (e.g. HasWidget#add)
I am not a native speaker but "the child widget that higher level API calls
(if any) will be delegated (e.g. HasWidget#add) to" sounds better to me.
File user/src/com/google/gwt/user/client/ui/FocusComposite.java
Line 52: HasAllMouseHandlers, HasAllGestureHandlers,
HasAllTouchHandlers {
What about the remaining interfaces that FocusWidget implements?
HasDoubleClickHandlers, HasAllDragAndDropHandlers?
--
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: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Jens Nehlmeier
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 2: (2 comments) File user/src/com/google/gwt/user/client/ui/Composite.java Line 29: * methods. When added to a panel, a composite behaves exactly as if the widget This javadoc doesn't seem quite true anymore if widget != delegate. Line 37: * @param type of the widget wrapped The way we're using T seems confusing. Widget getWidget(); T getDelegate(); void setWidget(T); So I'm wondering whether T is supposed to be the type of the widget, or the delegate, or both? We should clarify that. -- 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: 2 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 2: That's what I thought it was about; this is great! Just a note that the getDelegate could possibly break some people that defined a getDelegate method in their Composite subclass. I don't think it'll break many people in practice though, so I'm fine with this (same for the overloaded initWidget, with even less impact I guess). -- 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: 2 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 2: Yes, we haven't discussed =) The idea behind is some composite scenarios you want you delegate to something different than your base widget. A good example is following: This is my header This is my footer -- 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: 2 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 2: Code-Review+1 That new 'delegate' concept is interesting. I don't think we discussed it earlier though, so I'll let someone else put the +2. -- 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: 2 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Hello Matthew Dempsky, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/2582 to look at the new patch set (#2). Change subject: Introduces generic Composite widgets. .. Introduces generic Composite widgets. Composite can now be 'safely' extended to add delegation to methods for more specific types. Adds FocusComposite and PanelComposite. Improves error messaging for incorrect use. Fixes issue 4665, issue 6997. Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Review-Link: https://gwt-review.googlesource.com/#/c/2582/ --- M user/src/com/google/gwt/user/client/ui/Composite.java A user/src/com/google/gwt/user/client/ui/FocusComposite.java A user/src/com/google/gwt/user/client/ui/PanelComposite.java M user/src/com/google/gwt/user/client/ui/ResizeComposite.java 4 files changed, 297 insertions(+), 20 deletions(-) -- To view, visit https://gwt-review.googlesource.com/2582 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e Gerrit-PatchSet: 2 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 1:
(2 comments)
Sending a new patch without TypedXYZ.
PTAL.
File user/src/com/google/gwt/user/client/ui/PanelComposite.java
Line 39: this.add(asWidgetOrNull(w));
I think I understand what you are trying to achieve. Done.
File user/src/com/google/gwt/user/client/ui/TypedComposite.java
Line 118: if (widget == null) {
I change how this is asserted in the new code. I took a less conservative
approach and handled cases where it is harder to understand what is going
on.
--
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: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 1:
(6 comments)
Just to be clear, I'm not oppose to add generics to Composite itself. I'm
just concerned about simple case where people don't need any generics
(which is much more common).
I would like to hear what other people think about alternatives.
File user/src/com/google/gwt/user/client/ui/PanelComposite.java
Line 39: this.add(asWidgetOrNull(w));
I'm not sure what are we trying to solve with this alternative?
File user/src/com/google/gwt/user/client/ui/ResizeComposite.java
Line 23: * If you want initWidget/getWidget to be type safe please use
Actually, this class exist for the same reason non-generic Composite exists.
I didn't extend TypeComposite to keep it backward compatible but we can
change that.
File user/src/com/google/gwt/user/client/ui/TypedComposite.java
Line 34: * {@example com.google.gwt.examples.CompositeExample}
Will take a look after we decide how to move forward.
Line 118: if (widget == null) {
Any of the public APIs of the widget can be called from outside without the
widget attached to the dom. When that happens, if initwidget is not called
yet, they will end up with NPE.
Although I used getCheckedWidget in new ones, Composite class itself is
still prone to this issue as it is not using getCheckedWidget itself:
https://code.google.com/p/google-web-toolkit/issues/detail?id=6997
I can send another CL to fix that?
Line 118: if (widget == null) {
Actually I'm totally lost in Renderable part. Wouldn't it also fail with
with NPE when initializeClaimedElement called? Isn't that a better place to
put the check?
File user/src/com/google/gwt/user/client/ui/TypedResizeComposite.java
Line 25: public abstract class TypedResizeCompositeRequiresResize>
I like the idea that it will show up next to Composite but I think that
feels same as deprecating the old one. If we want everybody to use
Composite2 then we can just change Composite itself to add generics - which
is OK with me.
--
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: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 1: Thomas, I sent an email to gwt contributors list that discusses the advantages and disadvantages. -- 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: 1 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Brian Slesinsky Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Matthew Dempsky Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Thomas Broyer has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 1:
(3 comments)
Why couldn't we just make Composite (and ResizeComposite) generic rather
than introducing a new class? All current uses of Composite would be seen
by the compiler as extending the raw class, so all you get is a warning.
Changing ResizeComposite would be a breaking-change API-wise but I doubt
anyone uses a ResizeComposite with a non-RequiresResize widget anyway as it
breaks at runtime, so it shouldn't break anyone in practice.
File user/src/com/google/gwt/user/client/ui/PanelComposite.java
Line 39: this.add(asWidgetOrNull(w));
How about:
T widget = getCheckedWidget();
if (widget instance of HasWidgets.ForIsWidget) {
widget.add(w);
} else {
this.add(getWidgetOrNull(w));
}
in case the wrapped widget defers the call to asWidget(). If all HasWidgets
used in the app are HasWidgets.ForIsWidget (or with a bit of luck, those
used within a PanelComposite; I don't know how smart the compiler can be in
this case), then the if() will be evaluated statically and simplified in
the generated JS.
AFAICT, all "stock" HasWidgets are HasWidgets.ForIsWidget.
File user/src/com/google/gwt/user/client/ui/ResizeComposite.java
Line 23: * If you want initWidget/getWidget to be type safe please use
Should ResizeComposite be deprecated then?
File user/src/com/google/gwt/user/client/ui/TypedComposite.java
Line 118: if (widget == null) {
onAttach will fail already with an NPE.
IMO, render() should check it when renderable==null (if renderable!=null,
then widget should be !=null so there's no need to test, though we could
add an 'assert')
--
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: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Matthew Dempsky
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Brian Slesinsky has posted comments on this change.
Change subject: Introduces generic Composite widgets.
..
Patch Set 1:
(3 comments)
Seems okay in principle, though I think we could use review from other
widget experts since I'm not an expert.
For the record, I talked to Goktug about this and here's some context:
The reason we're doing this is to make it easier to create panel widgets
that you can drop into a UiBinder and also add children to them. This
required either not using a Composite (resulting in a messy API) or writing
a fair amount of boilerplate, and it wasn't easy to introduce a base class
so the boilerplate is in one place. Having the type variable helps to make
this clean.
File user/src/com/google/gwt/user/client/ui/TypedComposite.java
Line 34: * {@example com.google.gwt.examples.CompositeExample}
Do we need to update CompositeExample?
Line 118: if (widget == null) {
Is there somewhere else we can do this check so that it's only done once
and we don't need this method? Could it be done in onAttach()?
File user/src/com/google/gwt/user/client/ui/TypedResizeComposite.java
Line 25: public abstract class TypedResizeCompositeRequiresResize>
Perhaps ResizeComposite2 would actually be a better name? It might seem
ugly at first, but it shows up next to ResizeComposite in an IDE and makes
it very clear that it obsoletes ResizeComposite.
Similarly, perhaps TypedComposite should be Composite2?
--
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: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan
Gerrit-Reviewer: Brian Slesinsky
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Matthew Dempsky
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Matthew Dempsky has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 1: Verified+1 Hoorays, this change passed the build and style presubmit. :D More details at http://gwt-ci.dempsky.org:8080/job/gwt.presubmit/35 -- 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: 1 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Matthew Dempsky Gerrit-HasComments: No -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has posted comments on this change. Change subject: Introduces generic Composite widgets. .. Patch Set 1: One other option (which was my starting point) is just to add generics directly to Composite and ResizeComposite. Disadvantages with that are: 1. In a lot of cases generics will not add value but developers will need to provide a type anyway. 2. Old code will give warnings. 3. Change in the ResizeComposite will be a breaking change as pushes the type check to compile time instead of runtime (which is a good thing but a breaking change) I'm still in-between so would love to hear what you think. Also note that; The previous attempt from rjrjr was failed because it was using IsWidget as boundary type which was changing method signatures after type erasure. Because of that it was a breaking change. This one uses Widget as the boundary type so it is backward compatible (Using 'IsWidget' doesn't add much value to Composite anyway). -- 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: 1 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Goktug Gokdogan Gerrit-Reviewer: Goktug Gokdogan Gerrit-HasComments: No -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "Google Web Toolkit 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.
[gwt-contrib] Change in gwt[master]: Introduces generic Composite widgets.
Goktug Gokdogan has uploaded a new change for review.
https://gwt-review.googlesource.com/2582
Change subject: Introduces generic Composite widgets.
..
Introduces generic Composite widgets.
Fixes issue 4665.
Adds TypedComposite & TypedResizeComposite widgets that are type
safe. Composite can be 'safely' extended to add delegation to methods
for more specific types.
Adds FocusComposite and PanelComposite based on the new Composite
class.
Change-Id: I41e5c07e978d442db7d8402c57605cec1b3ea09e
---
M user/src/com/google/gwt/user/client/ui/Composite.java
A user/src/com/google/gwt/user/client/ui/FocusComposite.java
A user/src/com/google/gwt/user/client/ui/PanelComposite.java
M user/src/com/google/gwt/user/client/ui/ResizeComposite.java
A user/src/com/google/gwt/user/client/ui/TypedComposite.java
A user/src/com/google/gwt/user/client/ui/TypedResizeComposite.java
6 files changed, 469 insertions(+), 165 deletions(-)
diff --git a/user/src/com/google/gwt/user/client/ui/Composite.java
b/user/src/com/google/gwt/user/client/ui/Composite.java
index 257832d..834d4fe 100644
--- a/user/src/com/google/gwt/user/client/ui/Composite.java
+++ b/user/src/com/google/gwt/user/client/ui/Composite.java
@@ -15,181 +15,21 @@
*/
package com.google.gwt.user.client.ui;
-import com.google.gwt.dom.builder.shared.HtmlBuilderFactory;
-import com.google.gwt.dom.builder.shared.HtmlSpanBuilder;
-import com.google.gwt.dom.client.Element;
-import com.google.gwt.event.logical.shared.AttachEvent;
-import com.google.gwt.safehtml.shared.SafeHtml;
-import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
-import com.google.gwt.user.client.DOM;
-import com.google.gwt.user.client.Event;
-
/**
* A type of widget that can wrap another widget, hiding the wrapped
widget's
* methods. When added to a panel, a composite behaves exactly as if the
widget
* it wraps had been added.
- *
*
- * The composite is useful for creating a single widget out of an
aggregate of
- * multiple other widgets contained in a single panel.
- *
- *
+ * If you want initWidget/getWidget to be type safe please use
+ * {@link TypedComposite} instead.
*
* Example
* {@example com.google.gwt.examples.CompositeExample}
*
+ *
+ * @see TypedComposite
*/
-public abstract class Composite extends Widget implements IsRenderable {
-
- private Widget widget;
-
- private IsRenderable renderable;
-
- private Element elementToWrap;
-
- @Override
- public void claimElement(Element element) {
-if (renderable != null) {
- renderable.claimElement(element);
- setElement(widget.getElement());
-} else {
- this.elementToWrap = element;
-}
- }
-
- @Override
- public void initializeClaimedElement() {
-if (renderable != null) {
- renderable.initializeClaimedElement();
-} else {
- elementToWrap.getParentNode().replaceChild(widget.getElement(),
elementToWrap);
-}
- }
-
- @Override
- public boolean isAttached() {
-if (widget != null) {
- return widget.isAttached();
-}
-return false;
- }
-
- @Override
- public void onBrowserEvent(Event event) {
-// Fire any handler added to the composite itself.
-super.onBrowserEvent(event);
-
-// Delegate events to the widget.
-widget.onBrowserEvent(event);
- }
-
- @Override
- public SafeHtml render(RenderableStamper stamper) {
-if (renderable != null) {
- return renderable.render(stamper);
-} else {
- HtmlSpanBuilder spanBuilder = HtmlBuilderFactory.get()
- .createSpanBuilder();
- stamper.stamp(spanBuilder).end();
- return spanBuilder.asSafeHtml();
-}
- }
-
- @Override
- public void render(RenderableStamper stamper, SafeHtmlBuilder builder) {
-if (renderable != null) {
- renderable.render(stamper, builder);
-} else {
- builder.append(render(stamper));
-}
- }
-
- /**
- * Provides subclasses access to the topmost widget that defines this
- * composite.
- *
- * @return the widget
- */
- protected Widget getWidget() {
-return widget;
- }
-
- /**
- * Sets the widget to be wrapped by the composite. The wrapped widget
must be
- * set before calling any {@link Widget} methods on this object, or
adding it
- * to a panel. This method may only be called once for a given composite.
- *
- * @param widget the widget to be wrapped
- */
- protected void initWidget(Widget widget) {
-// Validate. Make sure the widget is not being set twice.
-if (this.widget != null) {
- throw new IllegalStateException("Composite.initWidget() may only be "
- + "called once.");
-}
-
-if (widget instanceof IsRenderable) {
- // In case the Widget being wrapped is an IsRenderable, we save that
fact.
- this.renderable = (IsRenderable) widget;
-}
-
-// Detach the new child.
-widget.removeFromParent();
-
-// Use the contained widget's element as the c
