These events are provided so that you can accomplish the same kinds of tasks without having to subclass the Widget, so it seemed natural (well, easier) to put the behavior there. But maybe that is too fragile, I'll try to move it where it belongs.
Another question is why Panel overrides those methods in the first place. I'll delete those, they're noise. Stephen, thanks for catching this, and especially for punching up the test. On Tue, Oct 12, 2010 at 12:37 PM, Joel Webber <[email protected]> wrote: > Hmm... I didn't even realize AttachEvent had been added. John & Ray, you > might want to take a look at this -- in the original design this super > invocation wouldn't have mattered, because widget's onLoad() was empty > (onAttach/Detach() weren't really meant to overridden outside of Panel and > Composite -- onLoad/Unload() were the methods to override). But now it > appears that Widget.onLoad() has actual code in it. Could this not be moved > to onAttach? > > Le 11 octobre 2010 14:05, <[email protected]> a écrit : > > Reviewers: , >> >> Description: >> Panel's onLoad does not call super.onLoad. This is a simple fix that >> just adds the missing onLoad call. >> >> I have another patch set which fires the attach event in >> onAttach/onDetach, given these are more stable methods that user's are >> less prone overriding them. >> >> Please review this at http://gwt-code-reviews.appspot.com/981801/show >> >> Affected files: >> user/src/com/google/gwt/user/client/ui/Panel.java >> user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java >> >> >> Index: user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java >> =================================================================== >> --- user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java >> (revision 8929) >> +++ user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java >> (working copy) >> @@ -121,9 +121,12 @@ >> } >> >> TestPanel tp = new TestPanel(); >> + TestAttachHandler tpa = new TestAttachHandler(); >> + tp.addAttachHandler(tpa); >> + >> TestWidget tw = new TestWidget(); >> - TestAttachHandler ta = new TestAttachHandler(); >> - tw.addAttachHandler(ta); >> + TestAttachHandler twa = new TestAttachHandler(); >> + tw.addAttachHandler(twa); >> >> tp.add(tw); >> RootPanel.get().add(tp); >> @@ -134,9 +137,9 @@ >> assertTrue(tp.onAttachOrder < tp.onLoadOrder); >> assertTrue(tp.onDetachOrder < tp.onUnloadOrder); >> assertTrue(tw.onAttachOrder < tw.onLoadOrder); >> - assertTrue(tw.onLoadOrder < ta.delegateAttachOrder); >> + assertTrue(tw.onLoadOrder < twa.delegateAttachOrder); >> assertTrue(tw.onDetachOrder < tw.onUnloadOrder); >> - assertTrue(tw.onUnloadOrder < ta.delegateDetachOrder); >> + assertTrue(tw.onUnloadOrder < twa.delegateDetachOrder); >> >> // Also trivial. Ensure that the panel's onAttach/onDetach is called >> before >> // its child's onAttach/onDetach. >> @@ -150,6 +153,9 @@ >> // detached/unloaded. >> assertTrue(tp.onUnloadOrder < tw.onUnloadOrder); >> >> + assertTrue(tp.onLoadOrder < tpa.delegateAttachOrder); >> + assertTrue(tp.onUnloadOrder < tpa.delegateDetachOrder); >> + >> // Make sure each widget/panel's elements are actually still attached >> to the >> // DOM during onLoad/onUnload. >> assertTrue(tp.domAttachedOnLoad); >> Index: user/src/com/google/gwt/user/client/ui/Panel.java >> =================================================================== >> --- user/src/com/google/gwt/user/client/ui/Panel.java (revision 8929) >> +++ user/src/com/google/gwt/user/client/ui/Panel.java (working copy) >> @@ -183,6 +183,7 @@ >> */ >> @Override >> protected void onLoad() { >> + super.onLoad(); >> } >> >> /** >> @@ -193,6 +194,7 @@ >> */ >> @Override >> protected void onUnload() { >> + super.onUnload(); >> } >> >> /** >> >> >> -- >> http://groups.google.com/group/Google-Web-Toolkit-Contributors >> > > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
