On Mon, 11 May 2026 15:33:43 GMT, Marius Hanl <[email protected]> wrote:
>> Christopher Schnick has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add test for list updates
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java
> line 341:
>
>> 339:
>> 340: @Test
>> 341: public void testCellUpdateOnStringConverterChange() {
>
> Minor: Maybe name it `testDisplayNodeUpdateOnStringConverterChange`
I updated the name to testButtonCell... to indicate that we are testing the
non-editable case here. Display node could mean both button cell or text field
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java
> line 411:
>
>> 409: comboBox.getItems().add("ITEM3");
>> 410:
>> 411: ListView<String> list = (ListView<String>)
>> ((ComboBoxListViewSkin<String>) comboBox.getSkin())
>
> can't you call `getListView()`, which is defined here in this class?
Done
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java
> line 413:
>
>> 411: ListView<String> list = (ListView<String>)
>> ((ComboBoxListViewSkin<String>) comboBox.getSkin())
>> 412: .getPopupContent();
>> 413: list.setSkin(new ListViewSkin<>(list));
>
> This is needed because the `ComboBox` is not inside a `Stage`.
> Not needed when you call:
> `sl =new StageLoader(comboBox);`
>
> Just FYI, is probably fine as is. But most of the time you rather want to
> simulate a real scenario.
Done
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220478431
PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220472687
PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220473619
