On Mon, Feb 28, 2011 at 3:14 PM, Jeff Larsen <[email protected]> wrote:
> > > On Mon, Feb 28, 2011 at 1:34 PM, John LaBanca <[email protected]> wrote: > >> Let me clarify what I had in mind for replacing the default GWT >> appearance. In the future, we might add a new DefaultAppearance >> implementation, but would leave the existing one. We would probably give it >> some trendy name, like ModernAppearance or DefaultAppearance2013, leaving >> DefaultAppearance. The GWT deferred binding for Appearance would be changed >> to point to DefaultAppearance2013. >> >> Using the default constructor will result in being automatically upgraded >> to the new appearance: >> new ButtonCell(); // Always uses the most recent Apperance. >> >> Using the Resources convenience constructor will use the old Appearance. >> new ButtonCell(myResources); // Uses DefaultAppearance. May be deprecated >> when new appearances are added. >> >> We would add a new convenience constructor for the new Appearance: >> public ButtonCell(DefaultAppearance2013.Resources resources); >> >> There is no way around the fact that DefaultAppearance.Resources are tied >> to DefaultAppearance and won't carry over to the new DefaultAppearance2013. >> > > That wouldn't have to be the case though would it? > > Couldn't we stick the Resource in the Appearance, then if the new > DefaultAppearance2013 for Button doesn't need to add new css definitions, > there then is no need to add an additional constructor or change up the > style definitions. Should something happen where DefaultAppearance2013 needs > additional classes you still have the option of creating a new ctor for that > new theme, you've just given yourself some more options down the road. The > main drawback here is that the Appearance.Resources may have css classes > that are unused in descendant appearances. If that burden seems too high, > then there still is nothing stopping you from implementing the multiple ctor > solution. > Thats actually a big problem. DefaultAppearance.Resources may specify a background gradient image, but in the future we can use CSS to specify a background gradient. So, we end up with an unused image and users are confused about why the gradient doesn't apply. Worse, if we want to add a style name or resources to the interface, thats a breaking change. Still, if DefaultAppearance2013 is a minor change that uses all of the same style names, we could subclass Appearance and use the same Resources. It would depend on how much of a change we make to the DOM structure. > We could add a setter for the Resource and do something like which could do > something like > > public void onAttach(){ > > if(resource == null){ > resource = *getDefaultResource();* > } > resource.ensureInjected(); > } > > thus avoiding injecting unused styles. > > You also could easily be getting into constructor hell here as you get to n > themes, you then require a minimum of n constructors and every time you add > a theme, you have to touch every FooWidget to add the appropriate > constructors. Seems like adding themes could become a nightmare. > > >> >> >> On Mon, Feb 28, 2011 at 2:23 PM, Philippe Beaudoin < >> [email protected]> wrote: >> >>> On Mon, Feb 28, 2011 at 10:30 AM, Jeff Larsen <[email protected]> >>> wrote: >>> > >>> > 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. >>> >>> In its current form, I don't think the constructor accepting a >>> CssResource lets GWT swap the default appearance without impacting the >>> user's code that relies on it. Let's look at your proposed >>> implementation for the constructor in question: >>> // Replace the styles used by this cell instance. >>> public ButtonCell(DefaultAppearance.Resources resources) { >>> this(new DefaultAppearance(resources)); >>> } >>> >>> This constructor is not using deferred binding. Therefore, if you want >>> user's code relying on it to switch to the new appearance you have to >>> roll-out a new implementation of the constructor. Let's say you do >>> that: >>> // Replace the styles used by this cell instance. >>> public ButtonCell(DefaultAppearance.Resources resources) { >>> >> this(new NewDefaultAppearance(resources)); >>> } >>> >>> The problem, here, is that you are passing DefaultAppearance.Resources >>> to NewDefaultAppearance. It means that your new appearance >>> implementation cannot use CSS classes that did not exist in the >>> original CssResource. In fact, in the current implementation, the >>> CssResource is defined by the Appearance _implementation_ (it's >>> DefaultAppearance.Resource, not Appearance.Resource). >>> >>> The confusion therefore is: >>> a) Will ButtonCell(DefaultAppearance.Resources resources) update its >>> appearance automatically when GWT upgrades to a new default >>> appearance; OR >>> b) Will it bind me forever to DefaultAppearance? >>> My proposition of dropping it was assuming you wanted to go with (b), >>> now I understand that you want the behavior of (a) -- which I agree is >>> much preferable -- but the current design does not seem to allow for >>> that goal. Maybe we should discuss ways to decouple the CssResource >>> from the Appearance implementations instead? >>> >>> Also: I agree with the rest of your post and your reasoning. >>> >>> -- >>> http://groups.google.com/group/Google-Web-Toolkit-Contributors >>> >> >> > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
