[REVIEW REQUEST] RT-25325 Controls should be hardcoded to their default skin in case -fx-skin is not specified in CSS

[steve.x.northover at oracle.com]

Answers inline:

On 21/11/2012 4:41 PM, Jasper Potts wrote:
> Answers inline:
> On Nov 21, 2012, at 1:31 PM, steve.x.northover at oracle.com wrote:
>
>> Ok, I should have read the JavaDoc more closely.  Am I correct in saying that this will create a hard reference between control classes and their skins (when there isn't one right now?).  I am not a fan of reflection but it seems weird that I can't ask what the default skin will be (as a String) so that I can use that to set CSS for a control using code?
> Not sure we want people to be able to query it as it may change and would not want people copy and pasting the value into their css files. This would have same issues as today.
I suppose that people could run a program, query the string, print it 
out and use it.  Would you even set -fx-skin in code?
I am wondering whether the hard reference to the skin is a problem.  I 
will ask Daniel what he thinks.
>> The method createDefaultSkin() is normally called exactly once when a skin is needed for the control.  Why not initDefaultSkin() (or a better name) that takes no arguments and set the default skin for the control?  Does it make sense to query the skin in a subclass, then set it somewhere else or call this method then not call setSkin() with the result?
> It might be possible to do this but not 100% sure as using private access to Control to set the skin to value from create method without CSS thinking it is a user set property that would not be overridable.
My point here is that this is a method that gets called once when a skin 
is initialized and I was going to just add an init() sort of method 
rather than one that returned a value that could only be used under 
restricted circumstances.  That's all.

>
> Jasper
>
>
>> Steve
>>
>> On 21/11/2012 4:05 PM, Jasper Potts wrote:
>>> The idea is: if there is no skin specified in CSS rather than logging a error like it does today, we first call createDefaultSkin() to create a skin. If it returns null then we log a error the same as we do today for no skin. So you can carry on using css for specifying or overriding the controls skin but it is not compulsory like it has been.
>>>
>>> Here is a example for Pagination control.
>>>
>>> public class Pagination extends Control {
>>> ...
>>>      @Override protected Skin<?>   createDefaultSkin() {
>>>          return new PaginationSkin(this);
>>>      }
>>> …
>>> }
>>>
>>> Even if we support "default" as css value we still need some way for control to tell CSS what that default is. So we would still need API like this as there is no way to override a CSS properties default value in a subclass.
>>>
>>> I have attached patch for fix to bug http://javafx-jira.kenai.com/browse/RT-25325 . This includes switching all shipping controls over to new method and fixing a couple bugs in control skins that were uncovered by slight change in skin creation timing.
>>>
>>> Jasper
>>>
>>> On Nov 21, 2012, at 12:43 PM, steve.x.northover at oracle.com wrote:
>>>
>>>> So the idea is not to use CSS at all and to reset the default skin?
>>>> Could we support "default" as a value for -fx-skin instead?
>>>> Can you give a simple example of how createDefaultSkin() would be used?
>>>>
>>>> Steve
>>>>
>>>> On 21/11/2012 3:25 PM, Jonathan Giles wrote:
>>>>> Jasper and I have been speaking about this recently, so from my point of view as tech lead inside the UI controls team I believe this is a good move - it speeds up instantiation time (less use of the costly reflection) and it allows for easier styling, both of which Jasper has outlined below. These are both good wins.
>>>>>
>>>>> The other big perk is that we can release more samples code without referring to private API (that is, the skins we use to style our controls).
>>>>>
>>>>> -- Jonathan
>>>>>
>>>>> On 22/11/2012 9:17 a.m., Jasper Potts wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> When building applications we often want to remove and replace all the default styling from a control but still use the default skin. To do this so far you had to respecify the default skin in your css file with something like
>>>>>>
>>>>>> .my-button {
>>>>>>     -fx-skin: "com.sun.javafx.scene.control.skin.ButtonSkin";
>>>>>> }
>>>>>>
>>>>>> The problem with this is the skins are in a com.sun package so not public API and may change in the future. So what I propose doing is adding new protected method to Control class so that sub-classes can create instances of their default skin.
>>>>>>
>>>>>> public abstract class Control{
>>>>>> …...
>>>>>>      /**
>>>>>>       * Create a new instance of the default skin for this control. This is called to create a skin for the control if
>>>>>>       * no skin is provided via CSS {@code -fx-skin} or set explicitly in a sub-class with {@code  setSkin(...)}.
>>>>>>       *
>>>>>>       * @return  new instance of default skin for this control. If null then the control will have no skin unless one
>>>>>>       *          is provided by css.
>>>>>>       */
>>>>>>      protected Skin<?>   createDefaultSkin() ….
>>>>>> ….
>>>>>> }
>>>>>>
>>>>>> The reason for returning a instance rather than class name string is it speeds up start up to not have to do the reflection to lookup and instantiate the class from classname. It would have been nice if this could have been a abstract method but that would have not been backwards compatible.
>>>>>>
>>>>>> All existing code will work as it does not but any css specifying the default skin can now be removed when running on 8.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Jasper