On Tue, 4 Oct 2022 15:57:28 GMT, Kevin Rushforth <[email protected]> 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
