On Mon, 11 May 2026 13:26:51 GMT, Christopher Schnick <[email protected]> wrote:
>> This PR adds a missing call to update the display value to the converter >> property listener >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Christopher Schnick has updated the pull request incrementally with one > additional commit since the last revision: > > Add test for list updates Looks good to me. I also like the new javadoc. Some minor test comments. 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` modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java line 373: > 371: comboBox.setValue("ITEM1"); > 372: > 373: TextField field = (TextField) ((ComboBoxListViewSkin<String>) > comboBox.getSkin()) Same here, call `getDisplayNode()` which is defined in this class and cast it to `TextField` 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? 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. ------------- PR Review: https://git.openjdk.org/jfx/pull/2165#pullrequestreview-4265028931 PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220146327 PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220118063 PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220113847 PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220140834
