JDK-8136404 : Review VirtualFlow API
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: 9
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2015-09-11
  • Updated: 2018-02-11
  • Resolved: 2016-06-02
Related Reports
Relates :  
JDK-8197535 - VirtualFlow: missing scrollToBottom(int)
Relates :  
JDK-8077916 - Complete development work for project one: make UI control skins public API
Description
I created this separate task so that the comment thread at JDK-8077916 does not get too unwieldy. Here are some comments on the VirtualFlow API:

1. VirtualFlow seems like a generally useful layout component, so I think it would make sense for it to be in javafx.scene.layout instead of javafx.scene.control.skin. The only caveat is that its cells are restricted to be IndexedCells (see 2.).

2. I find it unfortunate that VirtualFlow's cells are limited to instances of IndexedCell. It would make perfect sense to me to allow arbitrary Nodes in VirtualFlow.

3. Shouldn't java.util.Function be preferred over javafx.util.Callback for cellFactory?

4. Wouldn't it be better to change

    BooleanProperty	verticalProperty();

to

    Property<javafx.geometry.Orientation> orientationProperty();

?

5. Does it make sense for the vertical (aka. orientation) to be writable? Is it legal to change the orientation after the first layout?

6. Probably just an oversight: there are two versions of scrollToTop, one taking a cell and one taking a cell index, there is only the cell version of scrollToBottom.
Comments
Multi-issue jira issues like this are difficult to deal with, but I'll try my best to respond to each issue below. I'll then close this as "won't fix". If you disagree I think it would be best to file new issues per disagreement, so we can more easily manage the discussion and any necessary code commits. 1, 2) VirtualFlow is not targeted for general purpose layout - it is just targeted to be the underlying container for the existing virtualised UI controls. Making it more generic to handle any type of Node will only inhibit it from its primary goal of being highly performant for its intended purpose. Relocating to the layout package is not in the cards. 3) There doesn't appear to be any benefit of Function over Callback, from what I can tell? We use Callback because historically that was what was available to us (as Function only became available in Java 8), and it is what we expose in the public APIs for ListView, etc. It seems best to retain the use of Callback here. 4) Possibly this is a valid point. The only downside is the need to check for null orientations. Personally I am ok either way, so I would be inclined to leave as-is. 5) It is legal to change the orientation, but in reality I doubt it is something anyone ever does. I would leave it as writable. 6) Not so much an oversight as a "waiting for there to be a feature request".
10-11-2015