On Thu, Nov 18, 2010 at 11:59 AM, <[email protected]> wrote: > > http://gwt-code-reviews.appspot.com/1121801/diff/1/2 > File > user/src/com/google/gwt/uibinder/elementparsers/HasAlignmentParser.java > (right): > > http://gwt-code-reviews.appspot.com/1121801/diff/1/2#newcode34 > user/src/com/google/gwt/uibinder/elementparsers/HasAlignmentParser.java:34: > public class HasAlignmentParser implements ElementParser { > On 2010/11/18 19:45:35, rjrjr wrote: > >> On 2010/11/18 19:34:50, sbrubaker wrote: >> > On 2010/11/18 18:22:17, rjrjr wrote: >> > > No reason for these to be inner classes. >> > >> > These classes were inlined per your comment at >> > http://gwt-code-reviews.appspot.com/1109801/diff/1/5#newcode1000, >> > > Yes, but they're not as inlined as they sensibly could be. >> > > > but on further >> > thought I think it makes more sense to keep them as separate classes >> > and > >> > register these parsers as well. This solves the alignment issue for >> > any class > >> > that implements HasHorizontalAlignment or HasVerticalAlignment, and >> > there's no > >> > harm done to classes that implement HasAlignment (since the relevant >> attributes >> > are consumed by the first parser pass). >> > > My comment there suggested doing that iff it would actually solve any >> > problems. > >> The fact that you had to create HasAlignment to solve the actual bug >> > at hand > >> made me wonder. >> > > What will be fixed by the existence of HasHorizontalAlignment and >> HasVerticalAlignment? >> > > For any class that implements HasHorizontalAlignment or > HasVerticalAlignment rather than HasAlignment, the horizontalAlignment > and verticalAlignment tags are effectively useless, for the same reason > as HasAlignment. Registering these parsers will fix the parse order for > classes that implement the Has*Alignment classes directly. > > > http://gwt-code-reviews.appspot.com/1121801/show >
We have lots of widgets that implement HasHorizontalAlignment directly. All I'm asking is if you have confirmed that they are broken now, and fixed by this parser. It doesn't look like it to me. (Mmmmm, empiricism.) I take your point that any future panel that imitates the nasty order-dependency of the three HasAlignment panels would be fixed by this. But there aren't any that I can see, and I really dislike speculative coding. rjrjr -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
