RFR: 8089589: [ListView] ScrollBar content moves toward-backward during scrolling. [v2]

[Johan Vos]

On Thu, 15 Apr 2021 07:53:50 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Process reviewer comments
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java line 1305:
> 
>> 1303:             // Otherwise, our goal is to leave the index of the cell at the
>> 1304:             // top consistent, with the same translation etc.
>> 1305:             if (position != 0 && position != 1 && (currentIndex >= cellCount)) {
> 
> Comparing a double for equality or inequality is not the best coding practice. Anyway, I see this pattern throughout this file. We can live with it for now.

The comparison was already in the code. I agree though it is brittle in general, although it can probably be proved that in this case, the value of position will never be e.g. 1+\delta with \delta as small as possible.
But it would be good practice for a follow-up issue to remove the equality checks on doubles in this file (and others).

> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java line 1315:
> 
>> 1313:             s1 = sb.getLayoutY();
>> 1314:             newDelta = newPosition - position;
>> 1315:             System.err.println("s0 = "+s0+", s1 = "+s1);
> 
> I suggest to comment out all System.err.println() calls from this loop.
> They might be useful while debugging individual failing test, but keeping them on for every test run will simply flood the log.
> Similar logs were fixed for stdout earlier. Please refer - https://bugs.openjdk.java.net/browse/JDK-8255497

System.err.println()s are removed now.

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

PR: https://git.openjdk.java.net/jfx/pull/398