> On Saturday, February 26, 2011 9:14:37 PM UTC+1, Philippe Beaudoin wrote: >> 1) The ButtonCell(DefaultAppearance.Resources) constructor can be >> confusing, I think it should be dropped. >> 2) Providing a custom DefaultAppearance.Resources lead to somewhat smelly >> code. > > -1 on both the above > (note: ClientBundle only reads the annotations from the method definition, > it does not look at the overridden methods) Agreed on (2), still not sure about (1) even after the discussing it with Jeff. I'd like to see his reply on this. Also, I'd love to hear your thoughts on why the extra constructor is not confusing in the use case I proposed (and why it is needed).
By forcing the user to do new DefaultAppearance(Resource) you're removing their ability to globally change the appearance. You've now introduced a much tighter coupling than was there previously. As a for instance, lets say I wanted different button styles, I go ahead and extend the ClientBundle and apply then everything gets styled to my liking. Now lets say there is some html5 new button hotness that we want to have access to, or we need to add different Aria support etc. I can swap out the Appearance class globally with deferred binding by keeping both constructors in its current form. By getting rid of the constructor, I have to find every instance of my classes and change them programmatically to not use DefaultAppearance, but the new appearance. Now I'm good and I'm in a even more difficult refactor if I need to change the appearance based on locale/browser etc. I really think people will grasp injecting the ClientBundle as that is how the new CellWidgets work. The fact that it is Appearance.Resource does add an extra layer of complexity, but the flexibility/decoupling it gives you seems to be more than worth the complexity. The use case for changing styles is high. Many/most people will do that on most/all widgets if they're developing to a style guide. So by forcing you to new up an Appearance every time you change the style, you're more or less going to remove the ability to globally change the Appearance classes on your widgets. Having both constructors gives me extra flexibility which I always like :). As long as there is example code I think people will get the hang of how to change styles, especially since that pattern has already been introduced with the data presentation widgets. PS: GEP = Google Eclipse Plugin :) -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
