On Thu, 27 Oct 2022 18:15:23 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 28 additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/master' into 8290844.skin.install >> - 8290844: review comments >> - Merge remote-tracking branch 'origin/master' into 8290844.skin.install >> - 8290844: review comments >> - Merge remote-tracking branch 'origin/master' into 8290844.skin.install >> - 8290844: javadoc >> - Merge remote-tracking branch 'origin/master' into 8290844.skin.install >> - 8290844: javadoc >> - Merge branch 'openjdk:master' into 8290844.skin.install >> - 8290844: unit tests >> - ... and 18 more: https://git.openjdk.org/jfx/compare/f37cfff0...3235d433 > > modules/javafx.controls/src/main/java/javafx/scene/control/Skin.java line 97: > >> 95: * This method allows a {@link Skin} to implement any logic >> necessary to clean up itself after >> 96: * the {@code Skin} is no longer needed. It may be used to release >> native resources. >> 97: * The methods {@link #getSkinnable()} and {@link #getNode()} > > I don't think this is correct "Disconnects the skin from its skinnable" -- > the control does this. I also think "clean up" covers the "native resources" > bit, and not something that should be mentioned in an interface description. > > May I suggest: > >> Called when a previously installed skin is about to be removed from its >> associated control. This allows the skin to do clean up, like removing >> listeners and bindings, and undo any permanent changes to its control. >> After this method completes, {@link #getSkinnable()} and {@link #getNode()} >> should return {@code null}. I like this too, except for "permanent", which I think is a misnomer. ------------- PR: https://git.openjdk.org/jfx/pull/845
