On Mon, 11 May 2026 16:27:03 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:
> 
>   Update tests

There is one usecase from my testing that is still broken. But I also proposed 
a fix.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxListViewSkin.java
 line 182:

> 180:         });
> 181:         lh.addChangeListener(control.converterProperty(), e -> {
> 182:             updateCellFactory();

Here, I wonder if we just can call `listView.refresh()`. Because we effectively 
want all cells to 'recalculate' their value.

And there is one problem now with the current approach: If we have custom 
`cellFactory` that uses the `converter`, it won't get updated. 

Because in `updateCellFactory()` the `cellFactory` is the same as before, so 
will be a noop and the `ListView` items will not change.

<details>
<summary>Reproducer</summary>


import javafx.application.Application;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.ComboBox;
import javafx.scene.control.ListCell;
import javafx.scene.layout.BorderPane;
import javafx.stage.Stage;
import javafx.util.StringConverter;

public class ComboBoxCustomListCell extends Application {

    private final ObservableList<String> strings = 
FXCollections.observableArrayList("Option 1", "Option 2",
            "Option 3");

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage stage) {
        final ComboBox<String> comboBox = new ComboBox<>();
        comboBox.setConverter(new StringConverter<>() {
            @Override
            public String toString(String object) {
                return "CONVERTER: " + object;
            }

            @Override
            public String fromString(String string) {
                return null;
            }
        });
        comboBox.setItems(FXCollections.observableArrayList(strings));
        comboBox.setCellFactory(_ -> new ListCell<>() {
            @Override
            public void updateItem(String item, boolean empty) {
                super.updateItem(item, empty);

                if (empty || item == null) {
                    setText(null);
                } else {
                    setText(comboBox.getConverter().toString(item));
                }
            }
        });

        BorderPane root = new BorderPane(comboBox);
        Button value = new Button("Change Converter");
        value.setOnAction(event -> {
            comboBox.setConverter(new StringConverter<>() {
                @Override
                public String toString(String object) {
                    return "NEW: " + object;
                }

                @Override
                public String fromString(String string) {
                    return null;
                }
            });
        });
        root.setBottom(value);
        Scene scene = new Scene(root, 600, 400);

        stage.setScene(scene);
        stage.show();
    }
}


</details>

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

Changes requested by mhanl (Committer).

PR Review: https://git.openjdk.org/jfx/pull/2165#pullrequestreview-4265855466
PR Review Comment: https://git.openjdk.org/jfx/pull/2165#discussion_r3220810067

Reply via email to