changeset 11130:f593c24b8680

8207942: Add new protected VirtualFlow methods for subclassing Reviewed-by: kcr, nlisker, eryzhikov, aghaisas
author shadzic
date Fri, 14 Dec 2018 17:01:37 -0800
parents d7d399dd111e
children 1f990b4b789a
files modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehaviorBase.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListViewSkin.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableHeaderRow.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkin.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkinBase.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableViewSkin.java modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeViewSkin.java 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/VirtualFlowSubClassTest.java
diffstat 11 files changed, 504 insertions(+), 86 deletions(-) [+]
line wrap: on
line diff
--- a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehaviorBase.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehaviorBase.java	Fri Dec 14 17:01:37 2018 -0800
@@ -399,6 +399,12 @@
     private Runnable onSelectLeftCell;
     public void setOnSelectLeftCell(Runnable r) { onSelectLeftCell = r; }
 
+    private Runnable onFocusRightCell;
+    public void setOnFocusRightCell(Runnable r) { onFocusRightCell = r; }
+
+    private Runnable onFocusLeftCell;
+    public void setOnFocusLeftCell(Runnable r) { onFocusLeftCell = r; }
+
     public void mousePressed(MouseEvent e) {
 //        // FIXME can't assume (yet) cells.get(0) is necessarily the lead cell
 //        ObservableList<? extends TablePositionBase> cells = getSelectedCells();
@@ -522,7 +528,7 @@
         if (fm == null) return;
 
         fm.focusLeftCell();
-        if (onFocusPreviousRow != null) onFocusPreviousRow.run();
+        if (onFocusLeftCell != null) onFocusLeftCell.run();
     }
 
     protected void focusRightCell() {
@@ -533,7 +539,7 @@
         if (fm == null) return;
 
         fm.focusRightCell();
-        if (onFocusNextRow != null) onFocusNextRow.run();
+        if (onFocusRightCell != null) onFocusRightCell.run();
     }
 
     protected void focusPageUp() {
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListViewSkin.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListViewSkin.java	Fri Dec 14 17:01:37 2018 -0800
@@ -570,7 +570,7 @@
      * if this is a horizontal container, then the scrolling will be to the right.
      */
     private int onScrollPageDown(boolean isFocusDriven) {
-        ListCell<T> lastVisibleCell = flow.getLastVisibleCellWithinViewPort();
+        ListCell<T> lastVisibleCell = flow.getLastVisibleCellWithinViewport();
         if (lastVisibleCell == null) return -1;
 
         final SelectionModel<T> sm = getSkinnable().getSelectionModel();
@@ -597,7 +597,7 @@
                 // to be the top-most cell, or at least as far to the top as we can go.
                 flow.scrollToTop(lastVisibleCell);
 
-                ListCell<T> newLastVisibleCell = flow.getLastVisibleCellWithinViewPort();
+                ListCell<T> newLastVisibleCell = flow.getLastVisibleCellWithinViewport();
                 lastVisibleCell = newLastVisibleCell == null ? lastVisibleCell : newLastVisibleCell;
             }
         } else {
@@ -616,7 +616,7 @@
      * if this is a horizontal container, then the scrolling will be to the left.
      */
     private int onScrollPageUp(boolean isFocusDriven) {
-        ListCell<T> firstVisibleCell = flow.getFirstVisibleCellWithinViewPort();
+        ListCell<T> firstVisibleCell = flow.getFirstVisibleCellWithinViewport();
         if (firstVisibleCell == null) return -1;
 
         final SelectionModel<T> sm = getSkinnable().getSelectionModel();
@@ -642,7 +642,7 @@
                 // to be the bottom-most cell, or at least as far to the bottom as we can go.
                 flow.scrollToBottom(firstVisibleCell);
 
-                ListCell<T> newFirstVisibleCell = flow.getFirstVisibleCellWithinViewPort();
+                ListCell<T> newFirstVisibleCell = flow.getFirstVisibleCellWithinViewport();
                 firstVisibleCell = newFirstVisibleCell == null ? firstVisibleCell : newFirstVisibleCell;
             }
         } else {
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java	Fri Dec 14 17:01:37 2018 -0800
@@ -455,7 +455,16 @@
     NestedTableColumnHeader getNestedColumnHeader() { return nestedColumnHeader; }
     void setNestedColumnHeader(NestedTableColumnHeader nch) { nestedColumnHeader = nch; }
 
-    TableHeaderRow getTableHeaderRow() { return tableHeaderRow; }
+    /**
+     * Returns the {@link TableHeaderRow} associated with this {@code TableColumnHeader}.
+     *
+     * @return the {@code TableHeaderRow} associated with this {@code TableColumnHeader}
+     * @since 12
+     */
+    protected TableHeaderRow getTableHeaderRow() {
+        return tableHeaderRow;
+    }
+
     void setTableHeaderRow(TableHeaderRow thr) {
         tableHeaderRow = thr;
         updateTableSkin();
@@ -477,7 +486,14 @@
         }
     }
 
-    TableViewSkinBase<?,?,?,?,?> getTableSkin() {
+    /**
+     * Returns the {@code TableViewSkinBase} in which this {@code TableColumnHeader} is inserted. This will return
+     * {@code null} until the {@code TableHeaderRow} has been set.
+     *
+     * @return the {@code TableViewSkinBase} in which this {@code TableColumnHeader} is inserted, or {@code null}
+     * @since 12
+     */
+    protected TableViewSkinBase<?, ?, ?, ?, ?> getTableSkin() {
         return tableHeaderRow == null ? null : tableHeaderRow.tableSkin;
     }
 
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableHeaderRow.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableHeaderRow.java	Fri Dec 14 17:01:37 2018 -0800
@@ -48,6 +48,7 @@
 import javafx.scene.control.Label;
 import javafx.scene.control.TableColumn;
 import javafx.scene.control.TableColumnBase;
+import javafx.scene.layout.Border;
 import javafx.scene.layout.Pane;
 import javafx.scene.layout.Region;
 import javafx.scene.layout.StackPane;
@@ -276,7 +277,16 @@
      *                                                                         *
      **************************************************************************/
 
-    // --- reordering
+
+    /**
+     * Indicates if a reordering operation of a column is in progress. The value is {@code true} during a column
+     * reordering operation, and {@code false} otherwise. When a column is reordered (for example, by dragging its
+     * header), this property is updated automatically. Setting the value manually should be done when a subclass
+     * overrides the default reordering behavior. Calling {@link #setReorderingRegion(TableColumnHeader)} before setting
+     * this property is required as well.
+     *
+     * @since 12
+     */
     private BooleanProperty reordering = new SimpleBooleanProperty(this, "reordering", false) {
         @Override protected void invalidated() {
             TableColumnHeader r = getReorderingRegion();
@@ -291,13 +301,16 @@
             dragHeader.setVisible(isReordering());
         }
     };
-    final void setReordering(boolean value) {
+
+    public final void setReordering(boolean value) {
         this.reordering.set(value);
     }
-    final boolean isReordering() {
+
+    public final boolean isReordering() {
         return reordering.get();
     }
-    final BooleanProperty reorderingProperty() {
+
+    public final BooleanProperty reorderingProperty() {
         return reordering;
     }
 
@@ -314,9 +327,22 @@
     private final ReadOnlyObjectProperty<NestedTableColumnHeader> rootHeaderProperty() {
         return rootHeader.getReadOnlyProperty();
     }
-    final NestedTableColumnHeader getRootHeader() {
+
+    /**
+     * Returns the root header for all columns. The root header is a {@link NestedTableColumnHeader} that contains the
+     * {@code NestedTableColumnHeader}s that represent each column. It spans the entire width of the {@code TableView}.
+     * This allows any developer overriding a {@code TableColumnHeader} to easily access the root header and all others
+     * {@code TableColumnHeader}s.
+     *
+     * @return the root header
+     * @implNote This design enforces that column reordering occurs only within a single {@code NestedTableColumnHeader}
+     * and only at that level.
+     * @since 12
+     */
+    public final NestedTableColumnHeader getRootHeader() {
         return rootHeader.get();
     }
+
     private final void setRootHeader(NestedTableColumnHeader value) {
         rootHeader.set(value);
     }
@@ -378,9 +404,17 @@
         return snappedTopInset() + headerPrefHeight + snappedBottomInset();
     }
 
-    // used to be protected to allow subclasses to modify the horizontal scrolling,
-    // but made private again for JDK 9
-    void updateScrollX() {
+    /**
+     * Called whenever the value of the horizontal scrollbar changes in order to request layout changes, shifting the
+     * {@code TableColumnHeader}s.
+     * <p>
+     * For example, if custom components are added around a {@code TableColumnHeader} (such as icons above), they will
+     * also need to be shifted. When overriding, calling {@code super()} is required to shift the {@code
+     * TableColumnHeader}s, and it's up to the developer to notify its own custom components of this change.
+     *
+     * @since 12
+     */
+    protected void updateScrollX() {
         scrollX = flow.getHbar().isVisible() ? -flow.getHbar().getValue() : 0.0F;
         requestLayout();
 
@@ -390,9 +424,21 @@
         layout();
     }
 
-    // used to be protected to allow subclass to customise the width, to allow for features
-    // such as row headers, but made private again for JDK 9
-    private void updateTableWidth() {
+
+    /**
+     * Updates the table width when a resize operation occurs. This method is called continuously when the control width
+     * is resizing in order to properly clip this {@code TableHeaderRow}. Overriding this method allows a subclass to
+     * customize the resizing behavior.
+     * <p>
+     * Normally, the {@code TableHeaderRow} is using the full space ({@code TableView} width), but in some cases that
+     * space may be reduced. For example, if a vertical header that will display the row number is introduced, the
+     * {@code TableHeaderRow} would need to be clipped a bit shorter in order not to overlap that vertical header.
+     * Calling {@code super()} first when overriding this method allows {@link #getClip()} to compute the right width in
+     * order apply a transformation.
+     *
+     * @since 12
+     */
+    protected void updateTableWidth() {
         // snapping added for RT-19428
         final Control c = tableSkin.getSkinnable();
         if (c == null) {
@@ -424,7 +470,13 @@
      *                                                                         *
      **************************************************************************/
 
-    TableColumnHeader getReorderingRegion() {
+    /**
+     * Returns the current {@link TableColumnHeader} being moved during reordering.
+     *
+     * @return the current {@code TableColumnHeader} being moved
+     * @since 12
+     */
+    protected TableColumnHeader getReorderingRegion() {
         return reorderingRegion;
     }
 
@@ -432,7 +484,15 @@
         dragHeaderLabel.setText(rc == null ? "" : rc.getText());
     }
 
-    void setReorderingRegion(TableColumnHeader reorderingRegion) {
+    /**
+     * Sets the {@code TableColumnHeader} that is being moved during a reordering operation. This is automatically set
+     * by the {@code TableColumnHeader} when reordering starts. This method should only be called manually if the
+     * default reordering behavior is overridden. Calling {@link #setReordering(boolean)} after the call is required.
+     *
+     * @param reorderingRegion the {@code TableColumnHeader} being reordered
+     * @since 12
+     */
+    protected void setReorderingRegion(TableColumnHeader reorderingRegion) {
         this.reorderingRegion = reorderingRegion;
 
         if (reorderingRegion != null) {
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkin.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkin.java	Fri Dec 14 17:01:37 2018 -0800
@@ -118,16 +118,18 @@
         flow.getHbar().addEventFilter(MouseEvent.MOUSE_PRESSED, ml);
 
         // init the behavior 'closures'
-        behavior.setOnFocusPreviousRow(() -> onFocusPreviousCell());
-        behavior.setOnFocusNextRow(() -> onFocusNextCell());
+        behavior.setOnFocusPreviousRow(() -> onFocusAboveCell());
+        behavior.setOnFocusNextRow(() -> onFocusBelowCell());
         behavior.setOnMoveToFirstCell(() -> onMoveToFirstCell());
         behavior.setOnMoveToLastCell(() -> onMoveToLastCell());
         behavior.setOnScrollPageDown(isFocusDriven -> onScrollPageDown(isFocusDriven));
         behavior.setOnScrollPageUp(isFocusDriven -> onScrollPageUp(isFocusDriven));
-        behavior.setOnSelectPreviousRow(() -> onSelectPreviousCell());
-        behavior.setOnSelectNextRow(() -> onSelectNextCell());
+        behavior.setOnSelectPreviousRow(() -> onSelectAboveCell());
+        behavior.setOnSelectNextRow(() -> onSelectBelowCell());
         behavior.setOnSelectLeftCell(() -> onSelectLeftCell());
         behavior.setOnSelectRightCell(() -> onSelectRightCell());
+        behavior.setOnFocusLeftCell(() -> onFocusLeftCell());
+        behavior.setOnFocusRightCell(() -> onFocusRightCell());
 
         registerChangeListener(control.fixedCellSizeProperty(), e -> flow.setFixedCellSize(getSkinnable().getFixedCellSize()));
 
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkinBase.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkinBase.java	Fri Dec 14 17:01:37 2018 -0800
@@ -34,7 +34,6 @@
 import javafx.collections.MapChangeListener;
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableMap;
-import javafx.geometry.Insets;
 import javafx.scene.AccessibleAttribute;
 import javafx.scene.Node;
 import javafx.scene.control.*;
@@ -49,7 +48,6 @@
 import java.lang.ref.WeakReference;
 import java.util.List;
 import javafx.beans.WeakInvalidationListener;
-import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ObjectProperty;
 import javafx.geometry.HPos;
 import javafx.geometry.VPos;
@@ -508,7 +506,13 @@
      *                                                                         *
      **************************************************************************/
 
-    final TableHeaderRow getTableHeaderRow() {
+    /**
+     * Returns the {@code TableHeaderRow} created using {@link #createTableHeaderRow()}.
+     *
+     * @return the {@code TableHeaderRow} for this {@code TableViewSkinBase}
+     * @since 12
+     */
+    protected TableHeaderRow getTableHeaderRow() {
         return tableHeaderRow;
     }
 
@@ -573,48 +577,116 @@
         tableHeaderRow.updateScrollX();
     }
 
-    void onFocusPreviousCell() {
-        TableFocusModel<M,?> fm = getFocusModel();
+    /**
+     * Called when the focus is set on the cell above the current focused cell in order to scroll to it to make it
+     * visible.
+     *
+     * @since 12
+     */
+    protected void onFocusAboveCell() {
+        TableFocusModel<M, ?> fm = getFocusModel();
         if (fm == null) return;
 
         flow.scrollTo(fm.getFocusedIndex());
     }
 
-    void onFocusNextCell() {
-        TableFocusModel<M,?> fm = getFocusModel();
+    /**
+     * Called when the focus is set on the cell below the current focused cell in order to scroll to it to make it
+     * visible.
+     *
+     * @since 12
+     */
+    protected void onFocusBelowCell() {
+        TableFocusModel<M, ?> fm = getFocusModel();
         if (fm == null) return;
 
         flow.scrollTo(fm.getFocusedIndex());
     }
 
-    void onSelectPreviousCell() {
+    /**
+     * Called when the selection is set on the the cell above the current focused cell in order to scroll to it to make
+     * it visible.
+     *
+     * @since 12
+     */
+    protected void onSelectAboveCell() {
         SelectionModel<S> sm = getSelectionModel();
         if (sm == null) return;
 
         flow.scrollTo(sm.getSelectedIndex());
     }
 
-    void onSelectNextCell() {
+    /**
+     * Called when the selection is set on the cell below the current focused cell in order to scroll to it to make it
+     * visible.
+     *
+     * @since 12
+     */
+    protected void onSelectBelowCell() {
         SelectionModel<S> sm = getSelectionModel();
         if (sm == null) return;
 
         flow.scrollTo(sm.getSelectedIndex());
     }
 
-    void onSelectLeftCell() {
+    /**
+     * Called when the selection is set on the left cell of the current selected one in order to horizontally scroll to
+     * it to make it visible.
+     *
+     * @since 12
+     */
+    protected void onSelectLeftCell() {
         scrollHorizontally();
     }
 
-    void onSelectRightCell() {
+    /**
+     * Called when the selection is set on the right cell of the current selected one in order to horizontally scroll to
+     * it to make it visible.
+     *
+     * @since 12
+     */
+    protected void onSelectRightCell() {
         scrollHorizontally();
     }
 
-    void onMoveToFirstCell() {
+    /**
+     * Called when the focus is set on the left cell of the current selected one in order to horizontally scroll to it
+     * to make it visible.
+     *
+     * @since 12
+     */
+    protected void onFocusLeftCell() {
+        scrollHorizontally();
+    }
+
+    /**
+     * Called when the focus is set on the right cell of the current selected one in order to horizontally scroll to it
+     * to make it visible.
+     *
+     * @since 12
+     */
+    protected void onFocusRightCell() {
+        scrollHorizontally();
+    }
+
+    /**
+     * Called when the selection is set on the first cell of the table (first row and first column) in order to scroll
+     * to it to make it visible.
+     *
+     * @since 12
+     */
+    protected void onMoveToFirstCell() {
         flow.scrollTo(0);
         flow.setPosition(0);
     }
 
-    void onMoveToLastCell() {
+    /**
+     * Called when the selection is set on the last cell of the table (last row and last column) in order to scroll to
+     * it to make it visible.
+     *
+     * @since 12
+     */
+    protected void onMoveToLastCell() {
         int endPos = getItemCount();
         flow.scrollTo(endPos);
         flow.setPosition(1);
@@ -638,16 +710,22 @@
     }
 
     /**
-     * Function used to scroll the container down by one 'page', although
-     * if this is a horizontal container, then the scrolling will be to the right.
+     * Returns the index of the selected (or focused, if {@code isFocusDriven} is {@code true}) cell after a page scroll
+     * operation. If the selected/focused cell is not the last fully visible cell, then the last fully visible cell is
+     * selected/focused. Otherwise, the content is scrolled such that the cell is made visible at the top of the
+     * viewport (and the new last fully visible cell is selected/focused instead).
+     *
+     * @param isFocusDriven {@code true} if focused cell should be considered over selection
+     * @return the new index to select, or to focus if {@code isFocusDriven} is {@code true}
+     * @since 12
      */
-    int onScrollPageDown(boolean isFocusDriven) {
+    protected int onScrollPageDown(boolean isFocusDriven) {
         TableSelectionModel<S> sm = getSelectionModel();
         if (sm == null) return -1;
 
         final int itemCount = getItemCount();
 
-        I lastVisibleCell = flow.getLastVisibleCellWithinViewPort();
+        I lastVisibleCell = flow.getLastVisibleCellWithinViewport();
         if (lastVisibleCell == null) return -1;
 
         int lastVisibleCellIndex = lastVisibleCell.getIndex();
@@ -673,7 +751,7 @@
                 // to be the top-most cell, or at least as far to the top as we can go.
                 flow.scrollToTop(lastVisibleCell);
 
-                I newLastVisibleCell = flow.getLastVisibleCellWithinViewPort();
+                I newLastVisibleCell = flow.getLastVisibleCellWithinViewport();
                 lastVisibleCell = newLastVisibleCell == null ? lastVisibleCell : newLastVisibleCell;
             }
         }
@@ -685,11 +763,17 @@
     }
 
     /**
-     * Function used to scroll the container up by one 'page', although
-     * if this is a horizontal container, then the scrolling will be to the left.
+     * Returns the index of the selected (or focused, if {@code isFocusDriven} is {@code true}) cell after a page scroll
+     * operation. If the selected/focused cell is not the first fully visible cell, then the first fully visible cell is
+     * selected/focused. Otherwise, the content is scrolled such that the cell is made visible at the bottom of the
+     * viewport (and the new first fully visible cell is selected/focused instead).
+     *
+     * @param isFocusDriven {@code true} if focused cell should be considered over selection
+     * @return the new index to select, or to focus if {@code isFocusDriven} is {@code true}
+     * @since 12
      */
-    int onScrollPageUp(boolean isFocusDriven) {
-        I firstVisibleCell = flow.getFirstVisibleCellWithinViewPort();
+    protected int onScrollPageUp(boolean isFocusDriven) {
+        I firstVisibleCell = flow.getFirstVisibleCellWithinViewport();
         if (firstVisibleCell == null) return -1;
 
         int firstVisibleCellIndex = firstVisibleCell.getIndex();
@@ -710,7 +794,7 @@
                 // to be the bottom-most cell, or at least as far to the bottom as we can go.
                 flow.scrollToBottom(firstVisibleCell);
 
-                I newFirstVisibleCell = flow.getFirstVisibleCellWithinViewPort();
+                I newFirstVisibleCell = flow.getFirstVisibleCellWithinViewport();
                 firstVisibleCell = newFirstVisibleCell == null ? firstVisibleCell : newFirstVisibleCell;
             }
         }
@@ -816,18 +900,30 @@
         }
     }
 
-    // Handles the horizontal scrolling when the selection mode is cell-based
-    // and the newly selected cell belongs to a column which is not totally
-    // visible.
-    void scrollHorizontally() {
-        TableFocusModel<M,?> fm = getFocusModel();
+    /**
+     * Scrolls to the column containing the current focused cell.
+     * <p>
+     * Handles the horizontal scrolling when the selection mode is cell-based and the newly selected cell belongs to a
+     * column which is not completely visible.
+     *
+     * @since 12
+     */
+    public void scrollHorizontally() {
+        TableFocusModel<M, ?> fm = getFocusModel();
         if (fm == null) return;
 
         TC col = getFocusedCell().getTableColumn();
         scrollHorizontally(col);
     }
 
-    void scrollHorizontally(TC col) {
+    /**
+     * Programmatically scrolls to the given column. This call will ensure that the column is aligned on the left edge
+     * of the {@code TableView} and also that the columns don't become detached from the right edge of the table.
+     *
+     * @param col the column to scroll to
+     * @since 12
+     */
+    protected void scrollHorizontally(TC col) {
         if (col == null || !col.isVisible()) return;
 
         final Control control = getSkinnable();
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableViewSkin.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableViewSkin.java	Fri Dec 14 17:01:37 2018 -0800
@@ -164,16 +164,18 @@
         flow.getHbar().addEventFilter(MouseEvent.MOUSE_PRESSED, ml);
 
         // init the behavior 'closures'
-        behavior.setOnFocusPreviousRow(() -> onFocusPreviousCell());
-        behavior.setOnFocusNextRow(() -> onFocusNextCell());
+        behavior.setOnFocusPreviousRow(() -> onFocusAboveCell());
+        behavior.setOnFocusNextRow(() -> onFocusBelowCell());
         behavior.setOnMoveToFirstCell(() -> onMoveToFirstCell());
         behavior.setOnMoveToLastCell(() -> onMoveToLastCell());
         behavior.setOnScrollPageDown(isFocusDriven -> onScrollPageDown(isFocusDriven));
         behavior.setOnScrollPageUp(isFocusDriven -> onScrollPageUp(isFocusDriven));
-        behavior.setOnSelectPreviousRow(() -> onSelectPreviousCell());
-        behavior.setOnSelectNextRow(() -> onSelectNextCell());
+        behavior.setOnSelectPreviousRow(() -> onSelectAboveCell());
+        behavior.setOnSelectNextRow(() -> onSelectBelowCell());
         behavior.setOnSelectLeftCell(() -> onSelectLeftCell());
         behavior.setOnSelectRightCell(() -> onSelectRightCell());
+        behavior.setOnFocusLeftCell(() -> onFocusLeftCell());
+        behavior.setOnFocusRightCell(() -> onFocusRightCell());
 
         registerChangeListener(control.rootProperty(), e -> {
             // fix for RT-37853
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeViewSkin.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeViewSkin.java	Fri Dec 14 17:01:37 2018 -0800
@@ -528,7 +528,7 @@
      * Function used to scroll the container down by one 'page'.
      */
     private int onScrollPageDown(boolean isFocusDriven) {
-        TreeCell<T> lastVisibleCell = flow.getLastVisibleCellWithinViewPort();
+        TreeCell<T> lastVisibleCell = flow.getLastVisibleCellWithinViewport();
         if (lastVisibleCell == null) return -1;
 
         final SelectionModel<TreeItem<T>> sm = getSkinnable().getSelectionModel();
@@ -554,7 +554,7 @@
                 // to be the top-most cell, or at least as far to the top as we can go.
                 flow.scrollToTop(lastVisibleCell);
 
-                TreeCell<T> newLastVisibleCell = flow.getLastVisibleCellWithinViewPort();
+                TreeCell<T> newLastVisibleCell = flow.getLastVisibleCellWithinViewport();
                 lastVisibleCell = newLastVisibleCell == null ? lastVisibleCell : newLastVisibleCell;
             }
         } else {
@@ -572,7 +572,7 @@
      * Function used to scroll the container up by one 'page'.
      */
     private int onScrollPageUp(boolean isFocusDriven) {
-        TreeCell<T> firstVisibleCell = flow.getFirstVisibleCellWithinViewPort();
+        TreeCell<T> firstVisibleCell = flow.getFirstVisibleCellWithinViewport();
         if (firstVisibleCell == null) return -1;
 
         final SelectionModel<TreeItem<T>> sm = getSkinnable().getSelectionModel();
@@ -598,7 +598,7 @@
                 // to be the bottom-most cell, or at least as far to the bottom as we can go.
                 flow.scrollToBottom(firstVisibleCell);
 
-                TreeCell<T> newFirstVisibleCell = flow.getFirstVisibleCellWithinViewPort();
+                TreeCell<T> newFirstVisibleCell = flow.getFirstVisibleCellWithinViewport();
                 firstVisibleCell = newFirstVisibleCell == null ? firstVisibleCell : newFirstVisibleCell;
             }
         } else {
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java	Fri Dec 14 17:01:37 2018 -0800
@@ -1690,7 +1690,7 @@
             }
         }
         setCellIndex(accumCell, index);
-        resizeCellSize(accumCell);
+        resizeCell(accumCell);
         return accumCell;
     }
 
@@ -1735,10 +1735,27 @@
      *                                                                         *
      **************************************************************************/
 
-    final VirtualScrollBar getHbar() {
+    /**
+     * Returns the scroll bar used for scrolling horizontally. A developer who needs to be notified when a scroll is
+     * happening could attach a listener to the {@link ScrollBar#valueProperty()}.
+     *
+     * @return the scroll bar used for scrolling horizontally
+     * @since 12
+     */
+    protected final ScrollBar getHbar() {
         return hbar;
     }
-    final VirtualScrollBar getVbar() {
+
+    /**
+     * Returns the scroll bar used for scrolling vertically. A developer who needs to be notified when a scroll is
+     * happening could attach a listener to the {@link ScrollBar#valueProperty()}. The {@link ScrollBar#getWidth()} is
+     * also useful when adding a component over the {@code TableView} in order to clip it so that it doesn't overlap the
+     * {@code ScrollBar}.
+     *
+     * @return the scroll bar used for scrolling vertically
+     * @since 12
+     */
+    protected final ScrollBar getVbar() {
         return vbar;
     }
 
@@ -1855,7 +1872,16 @@
         }
     }
 
-    private void resizeCellSize(T cell) {
+    /**
+     * Resizes the given cell. If {@link #isVertical()} is set to {@code true}, the cell width will be the maximum
+     * between the viewport width and the sum of all the cells' {@code prefWidth}. The cell height will be computed by
+     * the cell itself unless {@code fixedCellSizeEnabled} is set to {@code true}, then {@link #getFixedCellSize()} is
+     * used. If {@link #isVertical()} is set to {@code false}, the width and height calculations are reversed.
+     *
+     * @param cell the cell to resize
+     * @since 12
+     */
+    protected void resizeCell(T cell) {
         if (cell == null) return;
 
         if (isVertical()) {
@@ -1867,12 +1893,28 @@
         }
     }
 
-    private List<T> getCells() {
+    /**
+     * Returns the list of cells displayed in the current viewport.
+     * <p>
+     * The cells are ordered such that the first cell in this list is the first in the view, and the last cell is the
+     * last in the view. When pixel scrolling, the list is simply shifted and items drop off the beginning or the end,
+     * depending on the order of scrolling.
+     *
+     * @return the cells displayed in the current viewport
+     * @since 12
+     */
+    protected List<T> getCells() {
         return cells;
     }
 
-    // Returns last visible cell whose bounds are entirely within the viewport
-    T getLastVisibleCellWithinViewPort() {
+    /**
+     * Returns the last visible cell whose bounds are entirely within the viewport. When manually inserting rows, one
+     * may need to know which cell indices are visible in the viewport.
+     *
+     * @return last visible cell whose bounds are entirely within the viewport
+     * @since 12
+     */
+    protected T getLastVisibleCellWithinViewport() {
         if (cells.isEmpty() || getViewportLength() <= 0) return null;
 
         T cell;
@@ -1894,8 +1936,14 @@
         return null;
     }
 
-    // Returns first visible cell whose bounds are entirely within the viewport
-    T getFirstVisibleCellWithinViewPort() {
+    /**
+     * Returns the first visible cell whose bounds are entirely within the viewport. When manually inserting rows, one
+     * may need to know which cell indices are visible in the viewport.
+     *
+     * @return first visible cell whose bounds are entirely within the viewport
+     * @since 12
+     */
+    protected T getFirstVisibleCellWithinViewport() {
         if (cells.isEmpty() || getViewportLength() <= 0) return null;
 
         T cell;
@@ -1945,7 +1993,7 @@
         while (index >= 0 && (offset > 0 || first)) {
             cell = getAvailableCell(index);
             setCellIndex(cell, index);
-            resizeCellSize(cell); // resize must be after config
+            resizeCell(cell); // resize must be after config
             cells.addFirst(cell);
 
             // A little gross but better than alternatives because it reduces
@@ -2040,7 +2088,7 @@
             }
             T cell = getAvailableCell(index);
             setCellIndex(cell, index);
-            resizeCellSize(cell); // resize happens after config!
+            resizeCell(cell); // resize happens after config!
             cells.addLast(cell);
 
             // Position the cell and update the max pref
@@ -2073,7 +2121,7 @@
                 index--;
                 T cell = getAvailableCell(index);
                 setCellIndex(cell, index);
-                resizeCellSize(cell); // resize must be after config
+                resizeCell(cell); // resize must be after config
                 cells.addFirst(cell);
                 double cellLength = getCellLength(cell);
                 start -= cellLength;
@@ -2110,22 +2158,46 @@
         return filledWithNonEmpty;
     }
 
-    void reconfigureCells() {
+    /**
+     * Informs the {@code VirtualFlow} that a layout pass should be done, and the cell contents have not changed. For
+     * example, this might be called from a {@code TableView} or {@code ListView} when a layout is needed and no cells
+     * have been added or removed.
+     *
+     * @since 12
+     */
+    protected void reconfigureCells() {
         needsReconfigureCells = true;
         requestLayout();
     }
 
-    void recreateCells() {
+    /**
+     * Informs the {@code VirtualFlow} that a layout pass should be done, and that the cell factory has changed. All
+     * cells in the viewport are recreated with the new cell factory.
+     *
+     * @since 12
+     */
+    protected void recreateCells() {
         needsRecreateCells = true;
         requestLayout();
     }
 
-    void rebuildCells() {
+    /**
+     * Informs the {@code VirtualFlow} that a layout pass should be done, and cell contents have changed. All cells are
+     * removed and then added properly in the viewport.
+     *
+     * @since 12
+     */
+    protected void rebuildCells() {
         needsRebuildCells = true;
         requestLayout();
     }
 
-    void requestCellLayout() {
+    /**
+     * Informs the {@code VirtualFlow} that a layout pass should be done and only the cell layout will be requested.
+     *
+     * @since 12
+     */
+    protected void requestCellLayout() {
         needsCellsLayout = true;
         requestLayout();
     }
@@ -2485,13 +2557,23 @@
         }
     }
 
+
     /**
-     * This method is an experts-only method - if the requested index is not
-     * already an existing visible cell, it will create a cell for the
-     * given index and insert it into the sheet. From that point on it will be
-     * unmanaged, and is up to the caller of this method to manage it.
+     * Creates and returns a new cell for the given index.
+     * <p>
+     * If the requested index is not already an existing visible cell, it will create a cell for the given index and
+     * insert it into the {@code VirtualFlow} container. If the index exists, simply returns the visible cell. From that
+     * point on, it will be unmanaged, and is up to the caller of this method to manage it.
+     * <p>
+     * This is useful if a row that should not be visible must be accessed (a row that always stick to the top for
+     * example). It can then be easily created, correctly initialized and inserted in the {@code VirtualFlow}
+     * container.
+     *
+     * @param index the cell index
+     * @return a cell for the given index inserted in the VirtualFlow container
+     * @since 12
      */
-    T getPrivateCell(int index)  {
+    protected T getPrivateCell(int index)  {
         T cell = null;
 
         // If there are cells, then we will attempt to get an existing cell
@@ -2525,7 +2607,7 @@
 
         if (cell != null) {
             setCellIndex(cell, index);
-            resizeCellSize(cell);
+            resizeCell(cell);
             cell.setVisible(false);
             sheetChildren.add(cell);
             privateCells.add(cell);
--- a/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/VirtualFlowShim.java	Fri Dec 14 10:41:34 2018 +0100
+++ b/modules/javafx.controls/src/shims/java/javafx/scene/control/skin/VirtualFlowShim.java	Fri Dec 14 17:01:37 2018 -0800
@@ -28,6 +28,7 @@
 import javafx.collections.ObservableList;
 import javafx.scene.Node;
 import javafx.scene.control.IndexedCell;
+import javafx.scene.control.ScrollBar;
 import javafx.scene.layout.StackPane;
 
 public class VirtualFlowShim<T extends IndexedCell> extends VirtualFlow<T> {
@@ -59,11 +60,11 @@
         return super.getMaxPrefBreadth();
     }
 
-    public VirtualScrollBar shim_getHbar() {
+    public ScrollBar shim_getHbar() {
         return super.getHbar();
     }
 
-    public VirtualScrollBar shim_getVbar() {
+    public ScrollBar shim_getVbar() {
         return super.getVbar();
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowSubClassTest.java	Fri Dec 14 17:01:37 2018 -0800
@@ -0,0 +1,153 @@
+/*
+ * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test.javafx.scene.control.skin;
+
+import javafx.scene.control.IndexedCell;
+import javafx.scene.control.skin.VirtualFlowShim;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests some protected methods of the VirtualFlow when overriding it.
+ */
+public class VirtualFlowSubClassTest {
+
+    // The following 4 vars are used when testing the
+    private VirtualFlowShim.ArrayLinkedListShim<CellStub> list;
+    private CellStub a;
+    private CellStub b;
+    private CellStub c;
+
+    // The VirtualFlow we are going to test. By default, there are 100 cells
+    // and each cell is 100 wide and 25 tall, except for the 30th cell, which
+    // is 200 wide and 100 tall.
+    private SubVirtualFlow<IndexedCell> flow;
+
+    @Before public void setUp() {
+        list = new VirtualFlowShim.ArrayLinkedListShim<CellStub>();
+        a = new CellStub(flow, "A");
+        b = new CellStub(flow, "B");
+        c = new CellStub(flow, "C");
+
+        flow = new SubVirtualFlow();
+//        flow.setManaged(false);
+        flow.setVertical(true);
+        flow.setCellFactory(p -> new CellStub(flow) {
+            @Override
+            protected double computeMinWidth(double height) {
+                return computePrefWidth(height);
+            }
+
+            @Override
+            protected double computeMaxWidth(double height) {
+                return computePrefWidth(height);
+            }
+
+            @Override
+            protected double computePrefWidth(double height) {
+                return flow.isVertical() ? (c.getIndex() == 29 ? 200 : 100) : (c.getIndex() == 29 ? 100 : 25);
+            }
+
+            @Override
+            protected double computeMinHeight(double width) {
+                return computePrefHeight(width);
+            }
+
+            @Override
+            protected double computeMaxHeight(double width) {
+                return computePrefHeight(width);
+            }
+
+            @Override
+            protected double computePrefHeight(double width) {
+                return flow.isVertical() ? (c.getIndex() == 29 ? 100 : 25) : (c.getIndex() == 29 ? 200 : 100);
+            }
+        });
+        flow.setCellCount(100);
+        flow.resize(300, 300);
+        pulse();
+        // Need a second pulse() call is because this parent can be made
+        // "layout" dirty again by its children
+        pulse();
+    }
+
+    private void pulse() {
+        flow.layout();
+    }
+
+    /**
+     * Tests the protected getFirstVisibleCellWithinViewport method returns the right value, especially when scrolling a
+     * bit.
+     */
+    @Test public void test_getFirstVisibleCellWithinViewport() {
+        assertEquals(flow.getFirstVisibleCell(), flow.getFirstVisibleCellWithinViewport());
+        assertEquals(0, flow.getFirstVisibleCellWithinViewport().getIndex());
+        flow.scrollPixels(10);
+        assertFalse(flow.getFirstVisibleCell().equals(flow.getFirstVisibleCellWithinViewport()));
+        assertEquals(1, flow.getFirstVisibleCellWithinViewport().getIndex());
+
+    }
+
+    /**
+     * Tests the protected getLastVisibleCellWithinViewport method returns the right value, especially when scrolling a
+     * bit.
+     */
+    @Test public void test_getLastVisibleCellWithinViewport() {
+        assertEquals(flow.getLastVisibleCell(), flow.getLastVisibleCellWithinViewport());
+        int lastIndex = flow.getLastVisibleCell().getIndex();
+        assertEquals(lastIndex, flow.getLastVisibleCellWithinViewport().getIndex());
+        flow.scrollPixels(10);
+        assertFalse(flow.getLastVisibleCell().equals(flow.getLastVisibleCellWithinViewport()));
+        assertEquals(lastIndex, flow.getLastVisibleCellWithinViewport().getIndex());
+        assertEquals(lastIndex + 1, flow.getLastVisibleCell().getIndex());
+
+    }
+
+    /**
+     * A subClass of the VirtualFlow that give access to some methods for testing purpose.
+     *
+     * @param <T>
+     */
+    class SubVirtualFlow<T extends IndexedCell> extends VirtualFlowShim {
+
+        @Override
+        public IndexedCell getLastVisibleCellWithinViewport() {
+            return super.getLastVisibleCellWithinViewport();
+        }
+
+        @Override
+        public IndexedCell getFirstVisibleCellWithinViewport() {
+            return super.getFirstVisibleCellWithinViewport();
+        }
+    }
+}