RFR: 8090123: Items are no longer visible when collection is changed [v7]

[Kevin Rushforth]

On Fri, 3 Mar 2023 07:28:48 GMT, Karthik P K <kpk at openjdk.org> wrote:

>> When a large number of items were scrolled in the `ChoiceBox`, the scrolled offset was carried forward when the list is replaced with small number of items. Hence the scroll up arrow was displayed with empty popup.
>> 
>> Changed code to scroll to top before popup display when content height of `ChoiceBox` is smaller than the scrolled offset.
>> 
>> Added system test to validate the fix.
>
> Karthik P K has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Stabilizing test

The updated test seems stable now. I did leave a couple suggestions.

Have you run the test this on all platforms?

tests/system/src/test/java/test/robot/javafx/scene/ChoiceBoxScrollUpOnCollectionChangeTest.java line 62:

> 60:  * There is 1 test in this file.
> 61:  * Steps for testChoicBoxScrollOnCollectionChange()
> 62:  * 1. Create a ChoiceBox and add 50 items to it.

Minor: the comment is wrong now (you add 150 items).

tests/system/src/test/java/test/robot/javafx/scene/ChoiceBoxScrollUpOnCollectionChangeTest.java line 143:

> 141: 
> 142:         double rowHeight = ContextMenuContentShim.getContextMenuRowHeight(popup);
> 143:         double screenHeight = Screen.getPrimary().getBounds().getHeight();

I think using `getVisualBounds()` would be better.

tests/system/src/test/java/test/robot/javafx/scene/ChoiceBoxScrollUpOnCollectionChangeTest.java line 144:

> 142:         double rowHeight = ContextMenuContentShim.getContextMenuRowHeight(popup);
> 143:         double screenHeight = Screen.getPrimary().getBounds().getHeight();
> 144:         scrollChoiceBox((int) (screenHeight / rowHeight));

This seems to work, but it might be more robust to use `Math.ceil()` before casting to int, especially if you make the change to use the visual bounds.

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

PR: https://git.openjdk.org/jfx/pull/1039