On Wed, 4 Dec 2024 16:22:18 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> This PR improves the `Tree-/TableRowSkin` code by doing a normal live lookup 
>> for the `fixedCellSize` instead of adding listener just to update 
>> variables(`fixedCellSizeEnabled` and `fixedCellSize`) which can otherwise be 
>> also just lookup'd without the hassle of listeners.
>> 
>> While this is mostly a cleanup, it does improve the state of the 
>> `Tree-/TableRow`, as when the `TableRowSkinBase` constructor is called, the 
>> variables are not yet set.
>> 
>> It is also consistent with the other cells, see also 
>> [JDK-8246745](https://bugs.openjdk.org/browse/JDK-8246745).
>> Helps a bit with [JDK-8185887](https://bugs.openjdk.org/browse/JDK-8185887) 
>> (https://github.com/openjdk/jfx/pull/1644), but as written above, not 
>> required as there is no (visible) effect.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java
>  line 432:
> 
>> 430:     }
>> 431: 
>> 432:     double getFixedCellSize() {
> 
> should this method be `abstract`?

We can do that, but then it should be `protected` as well, so custom code that 
extends from `TableRowSkinBase` can override this. If it is package private, 
custom code can not.

This will require a CSR hovever, so I think it is better as a follow up (there 
might be some more occurrences that should be `protected abstract` in this 
class)

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1645#discussion_r1870977422

Reply via email to