Setting predicate for FilteredList in ComboBox affects input

Question

I've implemented a ComboBox where its list is filtered by the input in the ComboBox TextField. It works as you might expect a filter of such a control to work. Every item in the list that starts with the input text is shown in the list.

I just have one small issue. If I select an item from the list, and then try to remove the last character in the textfield, nothing happens. If I select an item from the list, and then try to remove any other character than the last, the whole string gets removed. Both of these problems occur only if this is the first thing I do in the ComboBox. If I write something in the combo box first, or if I select an item for the second time, none of the issues described occurs.

What's really strange to me is that these problems seem to be caused by the predicate being set (if I comment out the invoking of setPredicate, everything works fine). This is strange since I think that should only affect the list that the predicate is being set for. It shouldn't affect the rest of the ComboBox.

import javafx.application.Application;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.collections.transformation.FilteredList;
import javafx.scene.Scene;
import javafx.scene.control.ComboBox;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
import javafx.util.StringConverter;

public class TestInputFilter extends Application {
    public void start(Stage stage) {
        VBox root = new VBox();

        ComboBox<ComboBoxItem> cb = new ComboBox<ComboBoxItem>();
        cb.setEditable(true);

        cb.setConverter(new StringConverter<ComboBoxItem>() {

            @Override
            // To convert the ComboBoxItem to a String we just call its
            // toString() method.
            public String toString(ComboBoxItem object) {
                return object == null ? null : object.toString();
            }

            @Override
            // To convert the String to a ComboBoxItem we loop through all of
            // the items in the combobox dropdown and select anyone that starts
            // with the String. If we don't find a match we create our own
            // ComboBoxItem.
            public ComboBoxItem fromString(String string) {
                return cb.getItems().stream().filter(item -> item.getText().startsWith(string)).findFirst()
                        .orElse(new ComboBoxItem(string));
            }
        });

        ObservableList<ComboBoxItem> options = FXCollections.observableArrayList(new ComboBoxItem("One is a number"),
                new ComboBoxItem("Two is a number"), new ComboBoxItem("Three is a number"),
                new ComboBoxItem("Four is a number"), new ComboBoxItem("Five is a number"),
                new ComboBoxItem("Six is a number"), new ComboBoxItem("Seven is a number"));
        FilteredList<ComboBoxItem> filteredOptions = new FilteredList<ComboBoxItem>(options, p -> true);
        cb.setItems(filteredOptions);

        InputFilter inputFilter = new InputFilter(cb, filteredOptions);
        cb.getEditor().textProperty().addListener(inputFilter);

        root.getChildren().add(cb);

        stage.setScene(new Scene(root));
        stage.show();
    }

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

    class ComboBoxItem {

        private String text;

        public ComboBoxItem(String text) {
            this.text = text;
        }

        public String getText() {
            return text;
        }

        @Override
        public String toString() {
            return text;
        }
    }

    class InputFilter implements ChangeListener<String> {

        private ComboBox<ComboBoxItem> box;
        private FilteredList<ComboBoxItem> items;

        public InputFilter(ComboBox<ComboBoxItem> box, FilteredList<ComboBoxItem> items) {
            this.box = box;
            this.items = items;
        }

        @Override
        public void changed(ObservableValue<? extends String> observable, String oldValue, String newValue) {
            String value = newValue;
            // If any item is selected we get the first word of that item.
            String selected = box.getSelectionModel().getSelectedItem() != null
                    ? box.getSelectionModel().getSelectedItem().getText() : null;

            // If an item is selected and the value of in the editor is the same
            // as the selected item we don't filter the list.
            if (selected != null && value.equals(selected)) {
                items.setPredicate(item -> {
                    return true;
                });
            } else {
                items.setPredicate(item -> {
                    if (item.getText().toUpperCase().startsWith(value.toUpperCase())) {
                        return true;
                    } else {
                        return false;
                    }
                });
            }
        }
    }
}

Edit: I've tried to override the key listeners in a desperate attempt to solve the issue:

cb.getEditor().addEventFilter(KeyEvent.KEY_PRESSED, e -> {
    TextField editor = cb.getEditor();
    int caretPos = cb.getEditor().getCaretPosition();
    StringBuilder text = new StringBuilder(cb.getEditor().getText());

    // If BACKSPACE is pressed we remove the character at the index
    // before the caret position.
    if (e.getCode().equals(KeyCode.BACK_SPACE)) {
        // BACKSPACE should only remove a character if the caret
        // position isn't zero.
        if (caretPos > 0) {
            text.deleteCharAt(--caretPos);
        }
        e.consume();
    }
    // If DELETE is pressed we remove the character at the caret
    // position.
    else if (e.getCode().equals(KeyCode.DELETE)) {
        // DELETE should only remove a character if the caret isn't
        // positioned after that last character in the text.
        if (caretPos < text.length()) {
            text.deleteCharAt(caretPos);
        }
    }
    // If LEFT key is pressed we move the caret one step to the left.
    else if (e.getCode().equals(KeyCode.LEFT)) {
        caretPos--;
    }
    // If RIGHT key is pressed we move the caret one step to the right.
    else if (e.getCode().equals(KeyCode.RIGHT)) {
        caretPos++;
    }
    // Otherwise we just add the key text to the text.
    // TODO We are currently not handling UP/DOWN keys (should move
    // caret to the end/beginning of the text).
    // TODO We are currently not handling keys that doesn't represent
    // any symbol, like ALT. Since they don't have a text, they will
    // just move the caret one step to the right. In this case, that
    // caret should just hold its current position.
    else {
        text.insert(caretPos++, e.getText());
        e.consume();
    }

    final int finalPos = caretPos;

    // We set the editor text to the new text and finally we move the
    // caret to its new position.
    editor.setText(text.toString());
    Platform.runLater(() -> editor.positionCaret(finalPos));
});

// We just consume KEY_RELEASED and KEY_TYPED since we don't want to
// have duplicated input.
cb.getEditor().addEventFilter(KeyEvent.KEY_RELEASED, e -> {
    e.consume();
});
cb.getEditor().addEventFilter(KeyEvent.KEY_TYPED, e -> {
    e.consume();
});

Sadly, this doesn't fix the issue either. If I e.g. choose the "Three is a number" item and then try to remove the last "e" in "Three", this is the values that the text property will switch between:

TextProperty: Three is a number
TextPropery: Thre is a number
TextPropery: 

So it removes the correct character at first, but then it removes the whole String for some reason. As mentioned before, this happens only because the predicate has been set, and it only happens when I do the first input after I've selected an item for the first time.

Answer 1

Jonatan,

As Manuel stated one problem is that setPredicate() will trigger your changed() method twice since you are changing the combobox model, however the real problem is that the combobox will overwrite the editor values with whatever values seems fit. Here is the explanation to your symptoms:

If I select an item from the list, and then try to remove the last character in the textfield, nothing happens.

In this case, the deleting of the last char is actually happening however the first call to setPredicate() matches one possible item (exactly the same item that you deleted the last char of) and changes the combobox contents to only one item. This causes a call where the combobox restores the editor value with the current combobox.getValue() string giving the illusion that nothing happens. It also causes a second call to your changed() method, but at this point the editor text is already changed.

Why do this only happen the first time, but then never again?

Good question! This happens only once, because you are modifying the entire underlying model of the combobox once (which as explained before, triggers the second call to the changed() method).

So after the previous scenario happens if you click the dropdown button (right arrow) you will see you only have one item left, and if you try to delete one character again you will still have that same item left, that is, the model (contents of the combobox) hasn't change, because the setPredicate() will still match the same contents, therefore not causing a markInvalid() call in the TextInputControl class since the contents didn't actually change which means not restoring the item string again (If you want to see where the textfield is actually restored the first time see the ComboBoxPopupControl.updateDisplayNode() method with the JavaFX sources).

If I select an item from the list, and then try to remove any other character than the last, the whole string gets removed.

In your second scenario nothing matches the first setPredicate() call (NO items to match your startsWith condition) which deletes all the items int the combobox removing your current selection and the editor string too.

TIP: Try and make sense of this for yourself, toggle a breakpoint inside the changed() method to see how many times this is entering and why (JavaFX source is needed if you want to follow the ComboBox and it's components behaviors)

Solution: If you want to keep using your ChangeListener, you can simply attack your main problem (which is the Editor contents being replaced after the setPredicate call) by restoring the text in the editor after the filtering:

class InputFilter implements ChangeListener<String> {
    private ComboBox<ComboBoxItem> box;
    private FilteredList<ComboBoxItem> items;

    public InputFilter(ComboBox<ComboBoxItem> box, FilteredList<ComboBoxItem> items) {
        this.box = box;
        this.items = items;
    }

    @Override
    public void changed(ObservableValue<? extends String> observable, String oldValue, String newValue) {
        String value = newValue;
        // If any item is selected we get the first word of that item.
        String selected = box.getSelectionModel().getSelectedItem() != null
                ? box.getSelectionModel().getSelectedItem().getText() : null;

        // If an item is selected and the value of in the editor is the same
        // as the selected item we don't filter the list.
        if (selected != null && value.equals(selected)) {
            items.setPredicate(item -> {
                return true;
            });
        } else {
            // This will most likely change the box editor contents
            items.setPredicate(item -> {
                if (item.getText().toUpperCase().startsWith(value.toUpperCase())) {
                    return true;
                } else {
                    return false;
                }
            });

            // Restore the original search text since it was changed
            box.getEditor().setText(value);
        }

        //box.show(); // <-- Uncomment this line for a neat look
    }
}

Personally I've done this before with KeyEvent handlers in the past (to avoid multiple calls to my code in the changed() event), however you can always use a Semaphore or your favorite class from the java.util.concurrent class to avoid any unwanted re-entrance to your method, if you feel you start to need it. Right now, the getEditor().setText() will always tail restore the correct value even if the same method bubbles down two or three times.

Hope this helps!

Answer 2

Setting a predicate will trigger your ChangeListener, because you're changing the ComboBox-Items and therefore the cb-editor's text-value. Removing the Listener and Re-Adding it will prevent those unexpected Actions.

I added three Lines to your change(...) - Method. Try out, if this is a fix for your problem.

Info: I only used your first block of code

@Override
public void changed(ObservableValue<? extends String> observable, String oldValue, String newValue) {
    String value = newValue;
    // If any item is selected we get the first word of that item.
    String selected = box.getSelectionModel().getSelectedItem() != null
            ? box.getSelectionModel().getSelectedItem().getText() : null;

    box.getEditor().textProperty().removeListener(this); // new line #1

    // If an item is selected and the value of in the editor is the same
    // as the selected item we don't filter the list.
    if (selected != null && value.equals(selected)) {
        items.setPredicate(item -> {
            return true;
        });
    } else {
        items.setPredicate(item -> {
            if (item.getText().toUpperCase().startsWith(value.toUpperCase())) {
                return true;
            } else {
                return false;
            }
        });
        box.getEditor().setText(newValue); // new line #2
    }

    box.getEditor().textProperty().addListener(this); // new line #3
}