8323511: Scrollbar Click jumps inconsistent amount of pixels #1326

FlorianKirmaier · 2024-01-10T12:31:20Z

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: #1194

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires CSR request JDK-8332041 to be approved
Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)

Issues

JDK-8323511: Scrollbar Click jumps inconsistent amount of pixels (Bug - P4)
JDK-8332041: Scrollbar Click jumps inconsistent amount of pixels (CSR)

Reviewers

Andy Goryachev (@andy-goryachev-oracle - Reviewer)
Kevin Rushforth (@kevinrushforth - Reviewer)
Johan Vos (@johanvos - Reviewer)

Contributors

Eduard Sedov <eduard.sdv@web.de>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1326/head:pull/1326
$ git checkout pull/1326

Update a local copy of the PR:
$ git checkout pull/1326
$ git pull https://git.openjdk.org/jfx.git pull/1326/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1326

View PR using the GUI difftool:
$ git pr show -t 1326

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1326.diff

Webrev

Link to Webrev Comment

Making the scrolled height consistent, when abvoe/below the scrollbar is clicked.

bridgekeeper · 2024-01-10T12:31:37Z

👋 Welcome back fkirmaier! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2024-01-10T12:35:54Z

Webrevs

kevinrushforth · 2024-01-10T12:36:39Z

Reviewers: @johanvos @andy-goryachev-oracle

/reviewers 2

openjdk · 2024-01-10T12:36:49Z

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

johanvos · 2024-01-10T12:52:14Z

Thanks for the PR!
Quick observations:

The unit test is indeed clear and at first sight, it looks to me that this is reasonable behavior. One of the hard parts with the VirtualFlow and related concepts, is that different use case scenarios assume different "reasonable behavior". Since we now have more tests than before, I am increasingly confident that a new change to the behavior is not causing regression.
Until we have the exact heights of all cells, the different measurable values (thumb location of scrollbar, position of the cell) can not be set simultaneously to desired values. (That is one of the other hard parts). Hence, changing the behavior of the scrollbar may affect the accuracy of the cell positioning in other scenario's. Again, since we have a growing amount of tests, I'm less worried about this.

I'll look into this more detailed.

andy-goryachev-oracle · 2024-01-10T16:02:54Z

For what it's worth, I just wanted to mention an alternative algorithm that was used in another project:

keep a cache of at least one screenful of laid out cells above and below the view port (a sliding window). this enables correct scrolling when pixels (screen dimensions) are used, as in page up / page down, or when navigating close to the start/end
do not update scroll bar min/max/thumb while the user is scrolling using the scroll bar. do update on MOUSE_RELEASED event. this eliminates flicker when cells with different sizes come into play

Please let me know if you want to see the code.

Maran23 · 2024-01-11T12:17:11Z

Agree, with all the tests added, especially in this area in #1194 and in #1246, it is much easier for us to catch regression. I will also have a look in the next days. I also noted that I got weird scrolling behaviour once before, but could never reproduce it.

andy-goryachev-oracle · 2024-01-12T23:44:14Z

Tested on macOS 14.1.2 with ListView, fixed and variable cell height, using MonkeyTester
https://github.com/andy-goryachev-oracle/MonkeyTest

I do see two problems:

With variable height cells: if I click below the scrollbar thumb, followed by a click above the thumb, I expect the view to go back to the same position exactly, but it does not (see the screenshots). I think I've mentioned this problem before in some other PR.

initial condition:

clicked below the thumb:

clicked above the thumb:

this one is more serious. sometimes it gets into state when clicking below the thumb does not move the scroll bar at all, here is the screenshot:

It seems to happen when the cell height exceeds the viewport height AND the cell is positioned exactly at the top of the viewport.

Can you see it?

andy-goryachev-oracle · 2024-01-12T23:24:19Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

@@ -1545,25 +1545,37 @@ public void scrollTo(int index) {
    }

    // will return true if scroll is successful
-    private boolean tryScrollOneCell(int targetIndex, boolean downOrRight) {
+private boolean tryScrollOneCell(int targetIndex, boolean downOrRight) {


please revert indentation change

removed it!

johanvos · 2024-01-13T08:56:48Z

With variable height cells: if I click below the scrollbar thumb, followed by a click above the thumb, I expect the view to go back to the same position exactly, but it does not (see the screenshots). I think I've mentioned this problem before in some other PR.

This is one of the major difficulties with the VirtualFlow. The word "expect" is very context-dependent. Different scenario's have different expectations. As long as there is nothing specified in the JavaDoc, I don't think one can expect anything.
In this particular case, for example, I can think of a number of reasons why you will not go back to the exact same position.

a cell has been added (outside the viewport).
the contents of a cell (outside the viewport) have changed, leading to different sizes
the estimates for cell dimensions have been updated

The location of the scrollbar thumb does not depend on previous actions, but on the current estimated position in the total sheet, which may vary for a number of reasons. We have the implicit restriction that even though the estimated position may change intermediately, we will not reposition the scrollbar thumb unless there is an explicit user action (scrolling or jumping etc).

For some time, we have been going back and forth to patch VirtualFlow in order to match particular expectations. By making a change, it is often very likely that the expectations in some scenario are violated -- although no contract is violated, hence no bug is introduced.
With the changes in VirtualFlow over the past years, we implicitly specify the expectations by adding regression tests.
In the end, it would be good to somehow have an exhaustive specification document.

reverted accidental indentation chang

Maran23 · 2024-02-05T09:53:23Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

-                cells.addLast(cell);
-                scrollPixels(getCellLength(cell));
+            if (targetIndex < 0) {
+                T cell = getCell(targetIndex);


Is there a reason you call getCell here, but getAvailableCell below?

As I was also involved in fixing this error, I can answer.

The getAvailableCell() method creates a new cell if none exists for the specified index (-1).
The getCell() returns the accumCell in this case, which is completely sufficient for our case, because we only need the dimensions of the cell here. The cell itself will not be added to the scene-graph.

That sounds right.
@Maran23 does this answer your question?

Yes. But is it possible to scroll to an index that is negative? (<0) And what do we estimate then?

But is it possible to scroll to an index that is negative?

Yes, both the negative value (-1) and the positive value, which is greater than the items count (to be precise last item index + 1), are possible and indicate the direction in which the scrolling takes place.

The last item index + 1 is already handled correctly. We only need to fix the handling of -1 index.

Can you explain your second question in more detail?

bridgekeeper · 2024-03-04T12:07:20Z

@FlorianKirmaier This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

johanvos · 2024-03-05T09:54:43Z

@FlorianKirmaier I still think this would be a good addition. I believe there is only one open question (from Marius) so it would be great if you can answer that.

FlorianKirmaier · 2024-03-13T08:55:14Z

@johanvos
Great to see you would like to see it merged!
I've worked on this topic together with @eduardsdv ,
So consider his response to be my answer to the question.

openjdk · 2024-03-13T20:00:39Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

johanvos · 2024-03-17T18:07:35Z

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/VirtualScrollBar.java

@@ -139,11 +139,10 @@ public final BooleanProperty virtualProperty() {
            if (firstVisibleCell != null && firstVisibleCell == lastVisibleCell) {
                int index = firstVisibleCell.getIndex();
                if (newValue < oldValue) {
-                    index = Math.max(0, index - 1);


Why are the boundary checks (Math.max/min) removed here?

The Math.max(0, index - 1) was introduced to fix an IndesOutOfBoundsException (JDK-8311983) for mouse clicks above the thumb, but it also introduced this bug (different scroll amount for clicks above and below thumb).
This change fixes the handling of -1 cell indexes in a different way, but for that we have to pass it to VirtualFlow.scrollTo(int index) and then to VirtualFlow.tryScrollOneCell(int targetIndex, boolean downOrRight).
Therefore it is necessary to remove this Math.max(..) call.

eduardsdv · 2024-03-26T10:18:18Z

Are there any open questions or objections? Can this pull request be merged?

johanvos · 2024-03-26T10:42:02Z

I plan to do an in-depth review later this week.

johanvos · 2024-03-28T19:19:40Z

Intuitively, the added test seems to be the right thing to do. It indeed fails before and succeeds after the patch.
However, I'm not sure about the implementation.

The suggested patch changes the conceptual idea of VirtualFlow.scrollTo(int index) where a negative index is not specified (this is probably what @Maran23 asked at #1326 (comment) .

The way the scrollTo(int index) is modified doesn't sound right to me. In case of the test, where the function is called with index = -1, it will first try to obtain the cell at index -2 (which will always return false), and then it will try to obtain the cell at index 0 (the one we need to have indeed), and then scroll X pixels where X is the height of the cell at index -1. In the test, all cells except the one at 0 have height 100, so the cell at -1 will have 100 as well, so it will use that.
This seems a complex way to deal with the issue. It would need a clear definition of "default size" and it changes the implicit assumption that scrollTo(index) should be called with a valid index (which does not have to be visible in the current viewport).

I believe the first thing that needs to be done, is to extend the "specification" that is currently in comments in the VirtualScrollBar:

            /*
             * Scroll one cell further in the direction the user has clicked if only one cell is shown.
             * Otherwise, a click on the trough would have no effect when cell height > viewport height.
             */

What does this mean if there is no cell further in the direction the user has clicked? Should we them scroll to the top of the current cell (current behavior), or should we use a more gradual scroll (which is the new behavior, which I believe is better indeed)? If the latter is the preferred case, this looks to a behavior that is more similar to the Event that is received when the mousewheel is used (and which invokes VirtualFlow.scrollPixels(double delta))

Maran23 · 2024-03-31T14:24:54Z

The suggested patch changes the conceptual idea of VirtualFlow.scrollTo(int index) where a negative index is not specified (this is probably what @Maran23 asked at #1326 (comment) .

Yes, this is exactly what I mean and where I do not know if this is the right approach.

The way the scrollTo(int index) is modified doesn't sound right to me

Agree, it sounds somewhat weird to me that when scrollTo is called with index = -1, that means we just scroll up more gradually (not to the top of the cell).

If the latter is the preferred case, this looks to a behavior that is more similar to the Event that is received when the mousewheel is used (and which invokes VirtualFlow.scrollPixels(double delta))

I completely agree. This sounds like we may should call scrollPixels directly instead.
As @johanvos mentioned, we also need to change the "specification" in the comment at least.

eduardsdv · 2024-04-03T09:46:15Z

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 scrollbar-track?

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()?

johanvos · 2024-04-10T12:16:59Z

That's a good question. Since there are a number of scenario's that change the current position (offset) of the VirtualFlow, we have to be careful that those scenario's are not conflicting, and are using the same code when appropriate. Hence, duplicating code between VirtualFlow and VirtualScrollBar sounds dangerous (what if code is changed on one place but not on the other?)

Rather than shifting some logic to VirtualScrollBar, I believe it would be safer to have VirtualScrollBar calling into VirtualFlow. For mouse-scrollwheel events, this is already the case so it might be a good idea to do something similar here?

…y one cell is visible

eduardsdv · 2024-05-06T09:23:39Z

I added a new method VirtualFlow.getBlockIncrement(), which returns the amount of pixels by which the position of the VirtualFlow should be adjusted when clicking on the scrollbar track.

Now, if only one cell is visible, the VirtualScrollBar changes the position of the VirtualFlow simply by calling the VirtualFlow.scrollPixels(double)``.

andy-goryachev-oracle · 2024-05-06T16:34:22Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

@@ -1800,6 +1800,16 @@ public T getCell(int index) {
        return accumCell;
    }

+    /**
+     * The amount of pixels by which to adjust the position when the the scroll bar track is clicked.


should it be the height of the viewport, so to speak?

Yes, that would also be my favorite choice for the click on the scrollbar track as well as for the PageUp/PageDown.
But for reasons of backwards compatibility, I didn't want to change it here.

andy-goryachev-oracle · 2024-05-06T16:35:14Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

+     */
+    public double getBlockIncrement() {
+        // For reasons of backward compatibility, we use the cell length of the empty cell (cell index -1)
+        return getCellLength(-1);


How would that work when cell heights are different for each cell?

What if the cell height is much larger than the viewport height (as in, 10 times larger?)

I don't like that either, but as I said I didn't want to change it because of backwards compatibility.

I would like to make it configurable or even make the complete scrolling behavior interchangeable.
Because, the actual scrolling behavior is still not consistent.

If multiple cells are visible, the cells are currently aligned (when clicking on the scrollbar) so that they are either exactly at the top or exactly at the bottom. If only one cell is visible, the scrolling behavior changes to the applying of pixel delta.

For example, if you have two large cells and a small ViewPort so that only one cell is visible.

If you click on the thumb below the scroll bar, the ListView is scrolled by a pixel delta so that two cells can become visible.

If you click below scroll thumb again, the ListView scrolls so that the second cell appears at the top.

If you click above the scroll thumb, the ListView scrolls by pixel delta, as the second cell is also larger than the viewport.

Now the ListView is in a different state than before the click (2).

These two behaviors are each useful for their own situation and should not be mixed.

Align the cell at the top or bottom if you have cells of the same size that are smaller than the viewport.

Scroll by pixel delta if you have cells of different sizes or if the cells are larger than the viewport.

johanvos · 2024-05-09T09:04:54Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

+     *
+     * @return the value in pixels
+     */
+    public double getBlockIncrement() {


I'd rather not add that method but instead make getViewportLength() public. That would benefit other use cases, and it is more behavior-agnostic. The "viewportLength" is easier to define in a contract than a "blockIncrement" and since this is exactly what you return, it sounds like a reasonable thing to do.

Good suggestion. I have made the necessary changes.

…tBlockIncrement()

kevinrushforth

@eduardsdv I left a couple comments on the API docs for the new public method.

Regarding the CSR, if you don't have a JBS account, perhaps you can ask @FlorianKirmaier or @johanvos to create the CSR for you.

And I agree with @andy-goryachev-oracle that you should be added a contributor. @FlorianKirmaier will need to do that, since only the author of the PR can add contributors.

kevinrushforth · 2024-05-09T14:31:56Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

-    double getViewportLength() {
+
+    /**
+     * The length of the viewport portion of the VirtualFlow as computed during the layout pass.


Suggestion: use code case, {@code VirtualFlow}

kevinrushforth · 2024-05-09T14:32:47Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

+     * For a vertical flow, this corresponds to the height and for a horizontal flow to the width of the clip view,
+     * but it does not necessarily have to be the same value.
+     *
+     * @return the viewport length in pixel


That should be "in pixels" (plural).

Also, please add an @since 23 tag after the return tag.

eduardsdv · 2024-05-09T14:46:41Z

I ask @FlorianKirmaier to create a CSR ticket.

johanvos · 2024-05-09T17:34:11Z

I did a number of tests with the new approach. It works as expected, and I don't see scenarios that could lead to a performance issue.
FWIW, this is a rather drastic change to the behavior, but there are no technical reasons to not do it. There are currently no tests that assert the current behavior, and I didn't find it in the public docs either (the idea about mapping scroll events to the top/bottom of a cell is hinted in the code though).
It is very likely that existing application will behave differently after this PR. But I think that is more a UX discussion than a code discussion.

andy-goryachev-oracle · 2024-05-09T17:45:39Z

the idea about mapping scroll events to the top/bottom of a cell is hinted in the code though

could you please point to these places?

I think this change is inline with the modern UIs, at least macOS and Windows. As Eduard correctly pointed out, it makes sense to align on the cell top boundary when changing selection, but mouse navigation via scroll bar should use the view port height. In other words, going back and forward with the mouse should revert the control to the same state (unless it hit the rail).

FlorianKirmaier · 2024-05-10T08:34:38Z

/contributor add @eduardsdv

openjdk · 2024-05-10T08:35:43Z

@FlorianKirmaier eduardsdv was not found in the census.

Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

FlorianKirmaier · 2024-05-10T08:53:49Z

/contributor add @eduardsdv

openjdk · 2024-05-10T08:54:41Z

@FlorianKirmaier @eduardsdv was not found in the census.

Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

FlorianKirmaier · 2024-05-10T09:06:51Z

/contributor add Eduard Sedov eduard.sdv@web.de

openjdk · 2024-05-10T09:07:53Z

@FlorianKirmaier
Contributor Eduard Sedov <eduard.sdv@web.de> successfully added.

FlorianKirmaier · 2024-05-10T09:26:18Z

I've added eduard as a contributor!

CSR
I've created a CSR.
https://bugs.openjdk.org/browse/JDK-8332041
If i remember correctly, the name of the ticket and the CSR must be the same?
Otherwise i could change it to something like "JavaFX, add the method double VirtualFlow.getViewportLength()"

kevinrushforth · 2024-05-10T12:07:04Z

I've added eduard as a contributor!

CSR I've created a CSR. https://bugs.openjdk.org/browse/JDK-8332041

Thanks!

If i remember correctly, the name of the ticket and the CSR must be the same? Otherwise i could change it to something like "JavaFX, add the method double VirtualFlow.getViewportLength()"

Yes, the titles of the JBS bug, the associated CSR, and the PR must all match.

And speaking of the CSR, based on Johan's comment about the behavior change, that should also be documented in the CSR. We don't currently document the behavior of most controls in the spec, but significant behavioral changes should still be listed in the CSR when they change.

FlorianKirmaier · 2024-05-14T13:37:32Z

Ok, I've updated the CSR accrodingly together with Eduard.

johanvos · 2024-05-14T14:44:58Z

the idea about mapping scroll events to the top/bottom of a cell is hinted in the code though

could you please point to these places?

I'm not sure I understand the question, as this is exactly what the PR is doing. The scrollToTop/scrollToBottom is replaced with a scroll of X pixels.

andy-goryachev-oracle · 2024-05-14T18:50:09Z

I'm not sure I understand the question

I thought there were specific comments somewhere in the code. Never mind then. Thank you for responding!

kevinrushforth

The API changes look good. I left one minor wording suggestion on the API docs.

The new behavior feels more consistent to me, so I have no concerns. I'll leave it to @johanvos and @andy-goryachev-oracle to formally review it.

kevinrushforth · 2024-05-16T19:36:00Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

-    double getViewportLength() {
+
+    /**
+     * The length of the viewport portion of the {@code VirtualFlow} as computed during the layout pass.


Suggested change:

"Returns the length of the viewport ..."

kevinrushforth · 2024-05-16T19:51:11Z

@FlorianKirmaier or @eduardsdv can one of you merge in the latest upstream master? The source branch for this PR is a few months (and 134 commits) behind master.

eduardsdv · 2024-05-17T07:25:35Z

I have merged the master into this branch.

kevinrushforth · 2024-05-21T21:55:30Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

+     * For a vertical flow, this corresponds to the height and for a horizontal flow to the width of the clip view,
+     * but it does not necessarily have to be the same value.


@johanvos added a question in the CSR about this last part:

I understand and agree with the goal behind this.

I'm a bit confused though about the following: "...but it does not necessarily have to be the same value." -> can you elaborate a bit about this?

I share this concern. I think that removing that last clause and putting a period after "clip view" is probably the best.

Johan: what do you think?

@johanvos added a question in the CSR about this last part:

I understand and agree with the goal behind this.
I'm a bit confused though about the following: "...but it does not necessarily have to be the same value." -> can you elaborate a bit about this?

I share this concern. I think that removing that last clause and putting a period after "clip view" is probably the best.

Johan: what do you think?

I agree that removing that clause is probably best to avoid confusion. Having open-ended suggestions in javadoc can lead to broad speculation, so I think it either should be explained (like you did in the CSR issue with the rubberband effect example) or removed.

We should probably also avoid the word "corresponds".

/** * Returns the length of the viewport portion of the {@code VirtualFlow} as computed during the layout pass. * For a vertical flow this is based on the height and for a horizontal flow on the width of the clip view. * * @return the viewport length in pixels * @since 23 */

The text explains that depending on the orientation of the view height or width is used in the calculation and the word "based" makes it clear that the value can differ from the respective size of the view.

This version looks good to me. If it is fine for you too, I would check it in.

I'm happy with this wording.

kevinrushforth

The latest version looks good.

JDK-8323511

cbcaefe

Making the scrolled height consistent, when abvoe/below the scrollbar is clicked.

openjdk bot added the rfr Ready for review label Jan 10, 2024

kevinrushforth requested a review from johanvos January 10, 2024 12:36

andy-goryachev-oracle reviewed Jan 12, 2024

View reviewed changes

JDK-8323511

29d610f

reverted accidental indentation chang

Maran23 reviewed Feb 5, 2024

View reviewed changes

johanvos reviewed Mar 17, 2024

View reviewed changes

openjdk bot changed the title ~~8323511 Scrollbar Click jumps inconsistent amount of pixels~~ 8323511: Scrollbar Click jumps inconsistent amount of pixels Apr 10, 2024

JDK-8323511: Fix scrolling when clicking the scrollbar track when onl…

6a02f68

…y one cell is visible

andy-goryachev-oracle reviewed May 6, 2024

View reviewed changes

johanvos reviewed May 9, 2024

View reviewed changes

JDK-8323511: Mage getViewportLength() public and use it instead of ge…

b08aaa5

…tBlockIncrement()

kevinrushforth reviewed May 9, 2024

View reviewed changes

JDK-8323511: Adjust javadoc of the getViewportLength()

645feee

kevinrushforth reviewed May 16, 2024

View reviewed changes

eduardsdv and others added 2 commits May 17, 2024 09:19

Merge branch 'openjdk:master' into JDK-8323511-scrollbar-jump

f16cff9

JDK-8323511: Adjust javadoc of VirtualFlow.getViewportLength()

023613d

andy-goryachev-oracle approved these changes May 17, 2024

View reviewed changes

kevinrushforth self-requested a review May 20, 2024 20:56

kevinrushforth reviewed May 21, 2024

View reviewed changes

JDK-8323511: Adjust javadoc of VirtualFlow.getViewportLength()

adfc3ef

kevinrushforth approved these changes May 24, 2024

View reviewed changes

johanvos approved these changes May 24, 2024

View reviewed changes

andy-goryachev-oracle approved these changes May 24, 2024

View reviewed changes

		* For a vertical flow, this corresponds to the height and for a horizontal flow to the width of the clip view,
		* but it does not necessarily have to be the same value.

8323511: Scrollbar Click jumps inconsistent amount of pixels #1326

Are you sure you want to change the base?

8323511: Scrollbar Click jumps inconsistent amount of pixels #1326

Conversation

FlorianKirmaier commented Jan 10, 2024 • edited by openjdk bot

Progress

Issues

Reviewers

Contributors

Reviewing

Webrev

bridgekeeper bot commented Jan 10, 2024

mlbridge bot commented Jan 10, 2024 • edited

Webrevs

kevinrushforth commented Jan 10, 2024

openjdk bot commented Jan 10, 2024

johanvos commented Jan 10, 2024

andy-goryachev-oracle commented Jan 10, 2024

Maran23 commented Jan 11, 2024 • edited

andy-goryachev-oracle commented Jan 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanvos commented Jan 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridgekeeper bot commented Mar 4, 2024

johanvos commented Mar 5, 2024

FlorianKirmaier commented Mar 13, 2024

openjdk bot commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardsdv commented Mar 26, 2024

johanvos commented Mar 26, 2024

johanvos commented Mar 28, 2024

Maran23 commented Mar 31, 2024

eduardsdv commented Apr 3, 2024 • edited

johanvos commented Apr 10, 2024

eduardsdv commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanvos May 9, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinrushforth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardsdv commented May 9, 2024

johanvos commented May 9, 2024

andy-goryachev-oracle commented May 9, 2024

FlorianKirmaier commented May 10, 2024 • edited

openjdk bot commented May 10, 2024

FlorianKirmaier commented May 10, 2024

openjdk bot commented May 10, 2024

FlorianKirmaier commented May 10, 2024

openjdk bot commented May 10, 2024

FlorianKirmaier commented May 10, 2024

kevinrushforth commented May 10, 2024

FlorianKirmaier commented May 14, 2024

johanvos commented May 14, 2024

andy-goryachev-oracle commented May 14, 2024

kevinrushforth left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinrushforth commented May 16, 2024

eduardsdv commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinrushforth left a comment

Choose a reason for hiding this comment

FlorianKirmaier commented Jan 10, 2024 •

edited by openjdk bot

mlbridge bot commented Jan 10, 2024 •

edited

Maran23 commented Jan 11, 2024 •

edited

eduardsdv commented Apr 3, 2024 •

edited

johanvos May 9, 2024 •

edited

FlorianKirmaier commented May 10, 2024 •

edited

kevinrushforth left a comment •

edited