Interesting ideas. I'm wondering, do you switch skins often enough that you
are worried about performance (and thus care about reusability of skins)?
Because I don't see how reusability of skins saves you lines of code -
whether the code is in the constructor or in the initialize() method, it is
there somewhere. In my opinion, reusing objects is more complicated than
creating fresh instances and the only justification for it is performance.

I agree with you on the problem of separation of skin initialization from
setSkin(). Another way to address this could be deprecating the original
setSkin() method and introducing

    setSkin(SkinProvider<C> skinProvider);

where SkinProvider is a SAM interface and thus the above method could be
called like this:

    setSkin(control -> new MySkin(control));

I know this defeats reusability of skins altogether, so you (and others)
may disagree.

Just my 2 cents.
Tomas


On Tue, Jan 7, 2014 at 11:30 AM, John Hendrikx <hj...@xs4all.nl> wrote:

> Hi List,
>
> I'm wondering if Skins in their current form are "finished" or that more
> changes are still likely to come.  The Skin interface is public, and
> SkinBase is as well.  Unfortunately, it seems that even though SkinBase is
> treated differently by JavaFX from regular Skins, the opportunity was not
> used to give SkinBase an initialize method and make Skins truly reusable.
>
> The reason I'm asking is because I'm developing a control that would
> greatly benefit from customizable and switchable skins, which differs quite
> a bit from the use patterns I've seen so far for Controls in JavaFX (I
> don't think any of them has more than one Skin out of the box).  My control
> would need:
>
> - Multiple different layouts, each with potentially different (CSS)
> properties to customize the chosen layout further (the properties would
> only make sense for the given layout (or skin)).
> - Easy switching between different layouts
> - Highly customizable (possibility to override methods in the layouts or
> provide custom code for certain parts of the rendering)
>
> I've been looking into providing this with Skins, but I keep bumping into
> odd choices made, especially related to their resuability.  As far as I
> know, a Skin is created with a reference to the control it is supposed to
> Skin -- at this time the Skin (if it is a SkinBase) apparently has done all
> kinds of stuff to the control, whether setSkin is called or not.... ehr??
>  Then later you are supposed to call setSkin on the control with the newly
> created Skin... which is apparently an almost optional step, unless it is
> not a SkinBase...?
>
> - What happens when I create the Skin but donot call setSkin?  -->
> Undefined, depends on whether or not it is extended from SkinBase (?)...
> SkinBase skins seem to make this step almost optional.
> - What happens when I call setSkin on a different control with this Skin?
> --> Depends on whether or not SkinBase is extended.  Would break controls
> in most situations -- no check is done if getSkinnable matches AFAICS.
> - What happens when a disposed skin is setSkin'ned again? --> Seems to
> break most controls, no check is done if getSkinnable is null AFAICS.
>
> This arrangement where the Skin class must be constructed with some
> parameter that must later match with the setSkin feels incredibly fragile
> to me.  It would be far nicer if a Skin was simply given an initialize
> method (which would also nicely match the dispose method) that takes the
> Skinnable as a parameter.  This initialization can be enforced in the
> setSkin method.  There is even a remark in the code about this:
>
>                 // Note I do not remove any children here, because the
>                 // skin will have already configured all the children
>                 // by the time setSkin has been called. This is because
>                 // our Skin interface was lacking an initialize method
> (doh!)
>                 // and so the Skin constructor is where it adds listeners
>                 // and so forth. For SkinBase implementations, the
>                 // constructor is also where it will take ownership of
>                 // the children.
>
> Would it not have been a better idea to add an #initialize method to
> SkinBase if the code has two paths for "legacy" Skins and SkinBase skins
> anyway?  Or a Skin2 interface with the extra method + instanceof check?  Or
> Java8 default #initialize method which does nothing by default?
>
> Any of those would open up Skins to be far easier to use by making them
> reusable.  I really want to be able to do something like this:
>
>     ComboBox<Skin> comboBox = new ComboBox<>(FXCollections.
> observableArrayList(
>         new Skin1(),
>         new Skin2(),
>         new Skin3(),
>         new SubClassOfSkin1(Color.RED)),
>         new Skin2() {
>            // with some methods overriden
>         }
>     ));
>
> Then bind those simply with:
>
>     myControl.skinProperty().bind(comboBox.getSelectionModel().
> selectedItemProperty());
>
> This would allow a ComboBox to control the Skin of another Control,
> because skins are reusable AND donot require knowledge of the control they
> are Skinning at construction time.  This is currently not possible without
> creating somekind of SkinFactory and a ChangeListener on the ComboBox that
> does something like:
>
>     myControl.setSkin(skinFactory.create(myControl));
>
> This problem gets worse when you also want to customize the Skins a bit.
>  All the customization must be done in the SkinFactory part, every time the
> Skin is switched.  With reusable skins, they can just be configured once.
>
> So I'm left with a bit of a choice.  Do I make multiple Skins and a bunch
> of Factories?  Or do I simply create just one fixed Skin that just
> delegates everything it does to a second light-weight Skinning mechanism
> that is easier to switch, customize and extend?
>
> --John
>

Reply via email to