-1

I have a ComboBox with a custom cell factory, but the rendering is buggy and I don't know what I'm doing wrong.

The ComboBox's data is based on an enum, let's call it MyEnum. So my ComboBox is defined as:

@FXML
private ComboBox<MyEnum> comboBox;

I am setting it up like this:

comboBox.getItems().addAll(MyEnum.values());
comboBox.setButtonCell(new CustomRenderer());
comboBox.setCellFactory(cell -> new CustomRenderer());
comboBox.getSelectionModel().select(0);

My CustomRenderer looks like this:

public class CustomRenderer extends ListCell<MyEnum> {

    @Override
    protected void updateItem(MyEnum enumValue, boolean empty) {
        super.updateItem(enumValue, empty);

        if (enumValue== null || empty) {
            setGraphic(null);
            setText(null);
        } else {
            setGraphic(enumValue.getGraphic());
        }
    }
}

The getGraphic() method of MyEnum returns a HBox which can contain any number of elements, but in my specific case it's just an ImageView:

public enum MyEnum{

       ONE(new ImageView(new Image(MyApp.class.getResourceAsStream("image1.png"),
            15,
            15,
            true,
            true))),

       TWO(new ImageView(new Image(MyApp.class.getResourceAsStream("image2.png"),
            15,
            15,
            true,
            true)));

    private final HBox graphic;

    MyEnum(Node... graphicContents) {
        graphic = new HBox(graphicContents);
        graphic.setSpacing(5);
    }

    public HBox getGraphic() {
        return graphic;
    }
}

Now, when I start the app, the first bug is that the ComboBox doesn't show anything selected, even though I have the comboBox.getSelectionModel().select(0) in the initialization:

enter image description here

When I click on it, the dropdown is correct and shows my two entries with their images:

enter image description here

When I select one of the entries, everything still seems fine:

enter image description here

But when I open the dropdown again, then it looks like this:

enter image description here

So suddenly the selected image is gone from the dropdown.

After I select the other entry where the icon is still displayed, and reopen the dropdown, then both images are gone. The images are still shown in the ButtonCell though, just not in the dropdown.

I first thought maybe it has something to do specifically with ImageViews, but when I replaced them with other nodes, like Labels, it was still the same 2 bugs:

  1. Nothing shown as selected on app start
  2. Everything that I click in the dropdown box is then gone from the dropdown

If a runnable sample is needed, let me know. But maybe someone can already spot my mistake from the given code.

Thx

user3237736
  • 845
  • 1
  • 9
  • 24
  • 2
    You should always create a [mcve] when requesting debugging assistance. – jewelsea Apr 28 '22 at 21:30
  • 2
    it's wrong to use nodes as data, even though you get away with it often except for combobox - please read the javadoc – kleopatra Apr 28 '22 at 21:57
  • and to address your answers: They were both very helpful, I understand the problem now. I will come up with a fix, maybe ask back if the fix is good or whether there's a more elegant way. Thanks so far! – user3237736 Apr 28 '22 at 22:14

1 Answers1

2

Your issue is that a node cannot appear in the scene graph more than once.

From the node documentation:

A node may occur at most once anywhere in the scene graph. Specifically, a node must appear no more than once in all of the following: as the root node of a Scene, the children ObservableList of a Parent, or as the clip of a Node.

If a program adds a child node to a Parent (including Group, Region, etc) and that node is already a child of a different Parent or the root of a Scene, the node is automatically (and silently) removed from its former parent.

You are trying to reuse the same node in both the button for the list selection and in the selection list itself, which is not allowed.

Additionally, as noted in comments by kleopatra: "it's wrong to use nodes as data". One reason for that (among others), is that if you want to have more than one view of the same data visible at the same time, you won't be able to because the node related to the data can only be attached to the scene at a single place at any given time.

I'd especially recommend not placing nodes in enums. In my opinion, that it is really not what enums are for.

What you need to do is separate the model from the view. The cell factory is creating a view of the changing value of the enum which is the model. This view only needs to be created once for the cell and it needs to be updated (by providing the view with the appropriate image) whenever the cell value changes.

Example Code

You can see in the example that, because we have created two separate views of the same Monster (the dragon), the example is able to render both views at the same time. This is because the views are completely different nodes, that provide a view to the same underlying data (an enum value and its associated image).

example

import javafx.application.Application;
import javafx.collections.FXCollections;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.*;
import javafx.scene.image.*;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

public class ChooseYourDoomApp extends Application {

    public static final String CSS = "data:text/css," + // language=CSS
            """
            .root {
                -fx-background-color: lightblue;
                -fx-base: palegreen;
            }
            """;

    @Override
    public void start(Stage stage) throws Exception {
        ComboBox<Monster> choiceOfDoom = new ComboBox<>(
                FXCollections.observableArrayList(
                        Monster.values()
                )
        );

        choiceOfDoom.setButtonCell(new MonsterCell());
        choiceOfDoom.setCellFactory(listView -> new MonsterCell());
        choiceOfDoom.getSelectionModel().select(Monster.Dragon);

        StackPane layout = new StackPane(choiceOfDoom);
        layout.setPadding(new Insets(20));

        Scene scene = new Scene(layout);
        scene.getStylesheets().add(CSS);

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

    public class MonsterCell extends ListCell<Monster> {
        private ImageView imageView = new ImageView();

        @Override
        protected void updateItem(Monster item, boolean empty) {
            super.updateItem(item, empty);

            if (item == null || empty) {
                imageView.setImage(null);
                setGraphic(null);
            } else {
                imageView.setImage(monsterImages.get(item));
                setGraphic(imageView);
            }
        }
    }

    public enum Monster {
        Medusa,
        Dragon,
        Treant,
        Unicorn
    }

    private Map<Monster, Image> monsterImages = createMonsterImages();

    private Map<Monster, Image> createMonsterImages() {
        Map<Monster, Image> monsterImages = new HashMap<>();

        for (Monster monster : Monster.values()) {
            monsterImages.put(
                    monster,
                    new Image(
                            Objects.requireNonNull(
                                    ChooseYourDoomApp.class.getResource(
                                            monster + "-icon.png"
                                    )
                            ).toExternalForm()
                    )
            );
        }

        return monsterImages;
    }

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

Icons are placed in the resource directory under the same hierarchy as the package containing the main application code.

Dragon-icon.png enter image description here

Medusa-icon.png enter image description here

Treant-icon.png enter image description here

Unicorn-icon.png enter image description here

jewelsea
  • 150,031
  • 14
  • 366
  • 406
  • Thanks, I basically did exactly what you posted now in the meantime, based on your previous comment. My enum now contains just the constants, and my renderer has a set of images that it maps to the different constants. But from your code I see I can further optimize my code, I thought I have to create a new ImageView in the updateItem method, when it seems I can re-use a single instance and just use setImage(). Thx! – user3237736 Apr 28 '22 at 22:42
  • @jewelsea, very nice answer, but `createMonsterImages();` in the `start` method appears to be unused. – SedJ601 May 03 '22 at 03:30
  • Removed redundant call. – jewelsea May 03 '22 at 16:37