Virtual Flow enhancements

Tue Sep 13 18:16:28 UTC 2022

I think it would be very good to add high level tests for those general 
use cases -- I've always found the VirtualFlow code quite complicated, 
but most of that comes from the need to allow different heights.

Not sure if this is relevant, but perhaps it is one of those "common 
sense" cases: one thing I missed since the inception of these controls 
is a way to position them in a such a way that they (pixel perfectly) 
show a certain state, tracking a cell or position, to allow me to show 
the control exactly the same as it was last time, or that the control 
tracks the position of a cell that may still be in flux (due to 
background loading of data).

For example, I scrolled my list so that the first Item is half visible 
(at the top), and the fifth item is a quarter visible (at the bottom) 
and I double click the 3rd item to go to a new screen (destroying the 
original list view in the process), I find there is no way to recreate 
the list view in the exact same state as it was before.

[ The part below turned out to be a bit of ramble... I could perhaps 
turn it into a ticket if others agree -- I think it also shows that 
there may not be one correct way to handle the scroll / positioning 
behavior of a list view, and that more user control could be warranted ]:

Preferably, I'd be able to tell the view, show item X (the one I double 
clicked) at the same vertical position as it was before. If the list 
meanwhile changed and there are other items before/after it, then those 
are allowed to be different, but the item I clicked should be in the 
exact same position. The only exception is if there are insufficient 
items before item X now, then it is acceptable to show X in a higher 
position.  The scrollTo function doesn't quite capture that (and has 
been generally unreliable IIRC requiring `Platform.runLater` wrapping 
for example).

Perhaps support something like an anchor position, which is updated 
**only** when there is user interaction (selecting, scrolling, using 
scroll keys). The only other way to change it would be programmatically. 
The anchor would indicate the preferred cell or part of the view to keep 
in an exact stable position and is never changed by the view itself 
unless there was some interaction (ie. size changes of cells and the 
view do not change the anchor, and neither do changes in the underlying 
list of items).  Not even a removal of the anchored index or item should 
update the anchor -- instead the view should fall back to a secondary 
anchor of its own choosing.  If the index or item later is available 
again, it will start using the primary anchor again.

The anchor should support I think three use cases:

- Anchoring an index
- Anchoring an item T
- Anchoring a scroll percentage

Anchoring an index would allow you to simply keep whatever item is at a 
certain index at a stable position.  This could be the first item or the 
current last item, regardless of size changes or new items being 
added/removed.  Using an index of Integer.MAX_VALUE could be supported 
here as an alternative to anchoring the bottom scroll position to the 
bottom of the view.

Similarly, anchoring a specific item T would keep that item in a stable 
position, even if the list is sorted, items around it are added or 
removed or the item itself changes size.

Anchoring a scroll percentage allows you to keep the view at a fixed 
position. Anchoring it at the bottom for example would be suitable for 
lists where the latest added items should be kept in view at all times.

The anchor is changed by the view code when the user selects an item to 
that specific item in that specific position (Item X should appear at 
25% of the control height).  The anchor is also changed when the user 
uses the scroll bar, fixing it at percentage 0 or 100 if the bar was 
moved all the way to the top or bottom, and on the top item otherwise 
(this behavior may need to be selectable).  Similar changes of the 
anchor are allowed for navigation keys, up, down, pg up, pg down, home, end.

To show the 42nd item in the exact center of the view do (and keep it 
there regardless of items appearing/disappearing/growing/shrinking):

      setAnchoredIndex(42, 0.5, 0.5);  // item 42's center is shown at 
the view center

To show the 42nd item's top exactly at 25% of the view:

      setAnchoredIndex(42.0, 0.0, 0.25);  // item 42's top is shown at 
25% of the views height

Item anchors:

      setAnchoredItem(itemX, 0.5, 0.5);    // itemX's center is shown at 
the exact center

Scroll anchors:

     setAnchoredPosition(1.0, 1.0);  // keep bottom of list at all times 
at bottom of view

Note that you could also ensure there is 50% empty space with an anchor 
at the top or bottom, like this:

     setAnchoredPosition(1.0, 0.5);  // keep bottom of list at all times 
at 50% of the view

For a list with images for example, which are often lazily loaded and 
have various sizes, the anchor would allow a user to easily say which 
item should be visible and where it is should appear relatively in the 
view, even if the item is not yet fully loaded; ie. setting the anchor 
with `setAnchoredItem(myLazyImageItem, 0.5, 0.5)` might initially 
display a small place holder cell in the exact center of the view, but 
once the image is loaded it might be much bigger and the view would jump 
to allow this; with the anchor, the image would remain in the exact 
center.  Only if the image is higher than the view itself it would be 
scrolled a bit to show its top.

--John

On 13/09/2022 11:02, Johan Vos wrote:
> Hi,
>
> OpenJFX 17 contains a fix for JDK-8089589  [1] ([ListView] ScrollBar 
> content moves toward-backward during scrolling.) Before this fix, the 
> scrollbar in the VirtualFlow backing List/Table/TreeTableView controls 
> was treating every cell of equal height. That generated an awkward 
> scrolling experience, especially when there are major differences in 
> sizes in the backing list.
>
> The original fix introduced some new issues, and most of these are 
> meanwhile fixed, and tests are added (thanks to e.g. Florian Kirmaier 
> who created a good amount of tests).
>
> However, people have in the past been relying on undocumented behavior 
> while creating apps and libraries, and that behavior is not guaranteed 
> anymore with the fix for JDK-8089589 that went into OpenJFX 17.
> Most of these cases are related to
> (1) assuming that IndexedCell.updateItem() is only called when an item 
> is (about to be) shown and
> (2) assuming that the value returned by the positionProperty of the 
> scrollbar thumb is linearly related to the index of the item in the 
> backinglist being shown.
>
> Both of these assumptions are invalid, but were (more or less) 
> happening before JavaFX 17.
> I have seen a fair amount of creative implementations that somehow get 
> and process information about the position of elements in the 
> controls. Some of these relied on the false assumptions above, and are 
> obviously no longer working.
> Instead of providing fixes per case, I think it would be good to 
> capture the common use cases, and make it easier for developers to 
> achieve what they want to achieve. I didn't yet encounter a use case 
> that can not be implemented with the current approach (using public 
> API's only), but granted, implementations often require too much 
> boilerplate and wiring code.
>
> Crucial in this exercise are tests. There are a fair amount of tests 
> for the list/table/treetableview controls, but only few of them are 
> focused on external access and behavior.
> For all the fixes done so far in VirtualFlow, additional tests have 
> been written which are now excellent regression tests. The last thing 
> we want is that a fix for usecase A is not working anymore after a fix 
> for usecase B.
>
> Hence, I'd like us to do 2 things:
> 1) for issues that are clearly bugs (*violating the JavaDoc*), please 
> create JBS issues as usual (with failing test code).
> 2) for issues that are not violating the specs now, but are "common 
> sense", please create a description and test scenario with expected 
> versus actual outcome.
>
> I am not 100% sure what the best approach is to tackle issues that 
> fall in the second category, but I suggest this: if you have a rather 
> vague, abstract issue (e.g. "listview should always scroll to the 
> bottom of the list IF it was at the bottom before and a new element is 
> added") , it probably makes sense to first discuss it here. If on the 
> other hand you have a very clear issue with a clear failing test, it 
> can be a JBS issue as well -- but we need to keep in mind then that it 
> might be in the category of "this fixes usescase A but blocks usecase B"
>
> Thanks,
>
> - Johan
>
> [1] https://bugs.openjdk.org/browse/JDK-8089589