On Mon, 15 Jan 2024 08:31:59 GMT, Florian Kirmaier <fkirma...@openjdk.org> 
wrote:

>> As seen in the unit test of the PR, when we click on the area above/below 
>> the scrollbar the position jumps - but the jump is now not always consistent.
>> In the current version on the last cell - the UI always jumps to the top. In 
>> the other cases, the assumed default cell height is used.
>> 
>> With this PR, always the default cell height is used, to determine how much 
>> is scrolled.
>> This makes the behavior more consistent.
>> 
>> Especially from the unit-test, it's clear that with this PR the behavior is 
>> much more consistent.
>> 
>> This is also related to the following PR: 
>> https://github.com/openjdk/jfx/pull/1194
>
> Florian Kirmaier has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8323511
>   reverted accidental indentation chang

I understand your objections. I will create a new pull request. We can then 
discuss which solution we use.

But I already have a few questions about it.

In the new pull request, the ``VirtualScrollBar.adjustValue(double pos)`` 
method will then use the ``VirtualFlow.scrollPixels(double delta)`` method as 
follows.


public class VirtualScrollBar extends ScrollBar {

    @Override
    public void adjustValue(double pos) {
        if (isVirtual()) {
            IndexedCell<?> cell = flow.getCell(-1);
            double pixels = flow.getFixedCellSize() > 0
                            ? flow.getFixedCellSize()
                            : flow.isVertical()
                              ? cell.getLayoutBounds().getHeight()
                              : cell.getLayoutBounds().getWidth();
            if (pos > getValue()) {
                flow.scrollPixels(pixels);
            } else {
                flow.scrollPixels(-pixels);
            }
        } else {
            super.adjustValue(pos);
        }
    }
}


The main question is how far should we scroll when the user clicks on the thumb?

For backwards compatibility reasons, I have used the same logic to calculate 
the delta pixels as is currently used.
Therefore, to calculate the delta pixels, we probably need to change the 
visibility of the method ``VirtualFlow.getCellLength(T cell)`` to ``public``. 
Currently I copied the content of it.
Furthermore, the method ``VirtualFlow.getCell(int index)`` is always called 
with the index ``-1`` that always returns the ``accumCell``. 
Can we somehow shortcut this or should we use a different logic here?

Should we move the entire delta pixel calculation logic into a new method 
``VirtualFlow.getBlockIncrement()``?

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

PR Comment: https://git.openjdk.org/jfx/pull/1326#issuecomment-2034084290

Reply via email to