RFR: 8290844: Add Skin.install() method [v9]

Andy Goryachev angorya at openjdk.org
Tue Oct 4 19:27:55 UTC 2022


On Tue, 4 Oct 2022 15:57:28 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 226:
>> 
>>> 224:      * <p>
>>> 225:      * To ensure a one-to-one relationship between a {@code Control} and its {@code Skin},
>>> 226:      * {@link Control#setSkin(Skin)} will check the return value of {@link Skin#getSkinnable()}
>> 
>> strictly speaking, it's not the method setSkin but the property skin ..
>
> so maybe `setting the {@link #skinProperty() skin property}`?

please check the updated comment, I think it sounds weird...

>> modules/javafx.controls/src/main/java/javafx/scene/control/Skin.java line 43:
>> 
>>> 41:  * <li>uninstalling of the old skin via its {@link #dispose()} method
>>> 42:  * <li>installing of the new skin via {@link #install()}
>>> 43:  * </ul>
>> 
>> - this interface is unrelated to any specific implementation of a Skinnable, at least replace control.setSkin with skinnable.setSkin
>> - technically, it can't be the setSkin method to have any workload (as already discussed and fixed elsewhere)
>> 
>> aside from these nit-pickings .. don't we overspecify here (and in the doc of install)? From scratch implementations of a Skinnable/Skin pair might decide to do the complete wiring outside of the property, something like
>> 
>>      Skinnable skinnable = new .. 
>>      Skin skin = new ...
>>      if (skinnable.getSkin() != null) skinnable.getSkin().dispose();
>>      skin.install();
>>      skinnable.setSkin(skin);
>
> I don't think we would want to go out of our way to enable this, so I prefer the tighter definition of the life-cycle that Andy is proposing. It seems better to have the Control always call `dispose` and `install` rather than provide an option where the application would call it.

I agree with @kevinrushforth , it's one of the cases when the method is public, but it should not be called by an application, only by its Control.

Perhaps we should further clarify this fact?

-------------

PR: https://git.openjdk.org/jfx/pull/845


More information about the openjfx-dev mailing list