Just a few remarks relating to the design changes... *RequiresLayout.java* : We expose now animation routines in this interface. With those new animation methods this interface becomes more constraining when you have to provide a subclass. What about a panel provider that would not want to implement any animation stuff ? Maybe something like RequiresLayoutWithAnimation and RequiresLayoutWithoutAnimation or design something that would lead to optional animation would be nice.
*LayoutPanel.java* : ProvidesLayout is replaced by both interfaces RequiresResize and ProvidesResize. IMHO this split seems to be redundant. Why this separation if at the end onLayout() calls onResize() and onResize() calls onResize() for all its children ? *ProvidesLayout.java:* This class is removed and replaced by ProvideResize but in the providers side, we have two different interfaces RequiresLayout and RequiresResize ... Joel, would be very interested to understand the limitations you got in the previous design …. Sami On Thu, Sep 3, 2009 at 6:34 AM, <rj...@google.com> wrote: > > Still LGTM, some nits. > > > http://gwt-code-reviews.appspot.com/63801/diff/1/2 > File user/javadoc/com/google/gwt/examples/DockLayoutPanelExample.java > (right): > > http://gwt-code-reviews.appspot.com/63801/diff/1/2#newcode49 > Line 49: rp.layout(); > So this doesn't call its children's layout methods? That seems like a > shame. If it does, then the p.layout method above is redundant, right? > > http://gwt-code-reviews.appspot.com/63801/diff/1/3 > File user/javadoc/com/google/gwt/examples/LayoutExample.java (right): > > http://gwt-code-reviews.appspot.com/63801/diff/1/3#newcode35 > Line 35: // percentages. > Might as well make this the class comment. > > Worth mentioning that most app developers won't do this (right?), and > what they'd do instead. > > http://gwt-code-reviews.appspot.com/63801/diff/1/3#newcode43 > Line 43: Element topChild = doc.createDivElement(), bottomChild = > doc.createDivElement(); > long line > > http://gwt-code-reviews.appspot.com/63801/diff/1/8 > File user/src/com/google/gwt/layout/client/UserAgent.java (right): > > http://gwt-code-reviews.appspot.com/63801/diff/1/8#newcode24 > Line 24: public class UserAgent { > should file an issue to make sure you remember to hit this TODO before > 2.0 ships (or make this not public again), or we'll be living with this > for a long time... > > http://gwt-code-reviews.appspot.com/63801 > > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
