On Thu, 16 Jan 2025 19:51:29 GMT, Andy Goryachev <[email protected]> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java
>> line 221:
>>
>>> 219: /** {@inheritDoc} */
>>> 220: @Override protected void layoutChildren(double x, double y, double
>>> w, double h) {
>>> 221: Node disclosureNode = getDisclosureNode();
>>
>> I take these changes (related to disclosure node) are not really related to
>> the initial issue of this PR, and I wonder if
>> [JDK-8346824](https://bugs.openjdk.org/browse/JDK-8346824) should not be
>> part of this PR. Do the added tests verify these changes and if the issue is
>> solved with them?
>
> I second @jperedadnr concern about combining separate issues into one PR.
> Small focused PRs will get tested, reviewed, and integrated almost always
> faster than the big complicated ones.
Completely agree, but in this case, the fix for
[JDK-8276326](https://bugs.openjdk.org/browse/JDK-8276326) is also the same for
[JDK-8346824](https://bugs.openjdk.org/browse/JDK-8346824) - that is the
`VirtualFlow` changes.
It is not really possible to separate those issues.
What I could try is to cherrypick the changes from the `VirtualFlow`, leaving
out the virtualization optimizations. Although this was not the original point
here.
The reason those are together is that they are very closely related. Fixing the
virtualization without the `VirtualFlow` will still break applications and
tests because you can somewhat randomly get empty cells.
Fixing the `VirtualFlow` first might work, but if you scroll enough you will
get empty cells because of broken virtualization.
So this PR got bigger in order to have a working virtualization and also
scrolling behavior with no empty cells.
If it really helps, I can split them up, as mentioned above.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1644#discussion_r1919325356
