changeset 11275:366a34279559 13+9

8197536: TableView, ListView: unexpected scrolling behaviour on up/down keys Reviewed-by: kcr, aghaisas
author tsayao
date Wed, 12 Jun 2019 11:43:32 -0700
parents 981cff5755bc
children b384d028b5f6
files modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java modules/javafx.controls/src/shims/java/javafx/scene/control/skin/VirtualFlowShim.java modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java
diffstat 3 files changed, 81 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java	Tue Jun 04 14:53:59 2019 -0700
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java	Wed Jun 12 11:43:32 2019 -0700
@@ -1458,12 +1458,46 @@
         if (cell != null) {
             scrollTo(cell);
         } else {
+            // see JDK-8197536
+            if (tryScrollOneCell(index, true)) {
+                return;
+            } else if (tryScrollOneCell(index, false)) {
+                return;
+            }
+
             adjustPositionToIndex(index);
             addAllToPile();
             requestLayout();
         }
     }
 
+    // will return true if scroll is successful
+    private boolean tryScrollOneCell(int targetIndex, boolean downOrRight) {
+        // if going down, cell diff is -1, because it will get the target cell index and check if previous
+        // cell is visible to base the position
+        int indexDiff = downOrRight ? -1 : 1;
+
+        T targetCell = getVisibleCell(targetIndex + indexDiff);
+        if (targetCell != null) {
+            T cell = getAvailableCell(targetIndex);
+            setCellIndex(cell, targetIndex);
+            resizeCell(cell);
+            setMaxPrefBreadth(Math.max(getMaxPrefBreadth(), getCellBreadth(cell)));
+            cell.setVisible(true);
+            if (downOrRight) {
+                cells.addLast(cell);
+                scrollPixels(getCellLength(cell));
+            } else {
+                // up or left
+                cells.addFirst(cell);
+                scrollPixels(-getCellLength(cell));
+            }
+            return true;
+        }
+
+        return false;
+    }
+
     /**
      * Adjusts the cells such that the cell in the given index will be fully visible in
      * the viewport, and positioned at the very top of the viewport.
--- a/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/VirtualFlowShim.java	Tue Jun 04 14:53:59 2019 -0700
+++ b/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/VirtualFlowShim.java	Wed Jun 12 11:43:32 2019 -0700
@@ -52,6 +52,11 @@
     }
 
     @Override
+    public double getCellPosition(T cell) {
+        return super.getCellPosition(cell);
+    }
+
+    @Override
     public void recreateCells() {
         super.recreateCells();
     }
--- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java	Tue Jun 04 14:53:59 2019 -0700
+++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java	Wed Jun 12 11:43:32 2019 -0700
@@ -1126,6 +1126,48 @@
         assertMinimalNumberOfCellsAreUsed(flow);
         assertEquals(flow.getViewportLength()-25.0, VirtualFlowShim.<IndexedCell>cells_getLast(flow.cells).getLayoutY(), 0.0);
     }
+
+    private void assertLastCellInsideViewport(boolean vertical) {
+        flow.setVertical(vertical);
+        flow.resize(400, 400);
+
+        int total = 10000;
+        flow.setCellCount(total);
+        pulse();
+
+        int count = 9000;
+        flow.setPosition(0d);
+        pulse();
+        flow.setPosition(((double)count) / total);
+        pulse();
+
+        //simulate 500 right key strokes
+        for (int i = 0; i < 500; i++) {
+            count++;
+            flow.scrollTo(count);
+            pulse();
+        }
+
+        IndexedCell vc = flow.getCell(count);
+
+        double cellPosition = flow.getCellPosition(vc);
+        double cellLength = flow.getCellLength(count);
+        double viewportLength = flow.getViewportLength();
+
+        assertEquals("Last cell must end on viewport size", viewportLength, (cellPosition + cellLength), 0.1);
+    }
+
+    @Test
+    // see JDK-8197536
+    public void testScrollOneCell() {
+        assertLastCellInsideViewport(true);
+    }
+
+    @Test
+    // see JDK-8197536
+    public void testScrollOneCellHorizontal() {
+        assertLastCellInsideViewport(false);
+    }
 }
 
 class CellStub extends IndexedCellShim {