Reset checkbox selection in ChangeListener under condition

Question

In my JavaFX application I'm using Checkboxes in a TreeView to change the visibility of nodes.

• checkbox selected = some nodes are visible
• checkbox deselected = some nodes are invisible

In a special case, however, the user should be prompted to confirm their selection, because problems can arise when activating a specific checkbox. A dialog window opens in which the user can choose between "Yes" and "No". If the user chooses "Yes", nodes become visible and everything is fine. But if the user chooses "No", the checkbox should be deselected again.

My idea was to check the condition (in this case press "no" in a dialog window) in the ChangeListener and if it's true, set the selected value to false.

But for whatever reason, it didn't work. After that, I figured out that it works with the refresh() method of the TreeView.

Questions

1. Why doesn't it work without the refresh() method, why setSelected() is ignored?
2. Should I use the refresh() method?
3. Is there a better workaround to change the selection status?

Minimal reproducible example

Using the refresh() line will show the desired behavior: The checkbox remains unselected after clicking because 5 > 4 (5>4 simulates for example pressing "no" in a dialog window).

import javafx.application.Application;
import javafx.beans.value.ChangeListener;
import javafx.scene.Scene;
import javafx.scene.control.CheckBoxTreeItem;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
import org.controlsfx.control.CheckTreeView;

public class HelloApplication extends Application {

    enum Names { TEST1, TEST2, TEST3, TEST4 }

    private final CheckTreeView<String> checkTreeView = new CheckTreeView<>();

    @Override
    public void start(Stage stage) {
        VBox vBox = new VBox();
        Scene scene = new Scene(vBox, 500, 500);
        setTreeView();
        vBox.getChildren().add(checkTreeView);
        stage.setScene(scene);
        stage.show();
    }

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


    public void setTreeView() {
        CheckBoxTreeItem<String> rootItem = new CheckBoxTreeItem<>("Root");
        rootItem.setExpanded(true);
        for (Names name : Names.values()) {
            CheckBoxTreeItem<String> item = new CheckBoxTreeItem<>(name.toString(), null);
            item.selectedProperty().addListener(this.onSelectionChanged(item));
            rootItem.getChildren().add(item);
        }
        this.checkTreeView.setRoot(rootItem);
    }

    private ChangeListener<Boolean> onSelectionChanged(CheckBoxTreeItem<String> item) {
        return (observableValue, previousChoice, newChoice) -> {

            if (newChoice) { // if checkbox is selected
                // if anything happens... for example press a "no" button in a dialog window
                if (5 > 4) {
                    System.out.println("reset checkbox status");
                    item.setSelected(previousChoice);
                }
            }

            // it works with refresh:
            // this.checkTreeView.refresh();
        };
    }
}

EDIT:

The solution of @VGR with

Platform.runLater(() -> item.setSelected(previousChoice));

works for the minimal reproducible example, but it doesn't seem to be the best way to do that. As already discussed in the comments, with Platform.runLater() it's

working most of the time

and

there's no guarantee because the exact timing is unspecified, will break f.i. if something in the pending events does also use runlater.

A solution / workaround that always works is desired...

Answer 1

What we are seeing in the OPs example actually is the standard behavior of bidirectionally bound properties, reduced to a simple (no ui, just properties) example.

The scenario:

• there are two boolean properties, representing the data (checkBoxTreeItem) and ui (checkBox) selected properties
• they are wired the same way as in a checkBoxTreeCell, that is the ui is bidirectionally bound to the data
• a listener to the data reverts the data to false on receiving a change from false -> true

Just for showing that - though nor recommended - reverting a the state of the sender in a listener is working: we set the data property to true. At the end, both data and ui are false as we want it.

before data.set(true)
    state of data/ui: false / false
enter listener to ui: true
enter listener to data: true
enter listener to ui: false
enter listener to data: false
 ... returning
listener to data - after reset false
after data.set(true) - state of data/ui: false / false

For the real simulation of the OP's context: set the ui property (user clicks the checkBox) - at the end, the data property is reverted while the ui property is not. Technically, that's done in BidirectionalBinding which ignores incoming changes while updating.

before ui.set(true)
    state of data/ui: false / false
enter listener to data: true
enter listener to data: false
 ... returning
listener to data - after reset false
enter listener to ui: true
after ui.set(true) - state of data/ui: false / true

The example:

public class BidiBindingUpdating {

    public static void main(String[] args) {
        BooleanProperty data = new SimpleBooleanProperty();
        BooleanProperty ui = new SimpleBooleanProperty();
        ui.bindBidirectional(data);

        // listener to item: revert to false
        data.addListener((src, ov, nv) -> {
            System.out.println("enter listener to data: " + nv);
            if (!nv) {
                System.out.println(" ... returning");
                return;
            }
            data.set(ov);
            System.out.println("listener to data - after reset " + data.get());
        });

        // listener to check: logging only
        ui.addListener((src, ov, nv) -> {
            System.out.println("enter listener to ui: " + nv);
        });

        // set item directly:
        System.out.println("before data.set(true)");
        System.out.println("    state of data/ui: " + data.get() + " / " + ui.get());
        data.set(true);
        System.out.println("after data.set(true) - state of data/ui: " + data.get() + " / " + ui.get());

        // set bound property:
        System.out.println("\nbefore ui.set(true)");
        System.out.println("    state of data/ui: " + data.get() + " / " + ui.get());
        ui.set(true);
        System.out.println("after ui.set(true) - state of data/ui: " + data.get() + " / " + ui.get());

    }
}

Answer 2

You are resetting the checkbox item too soon. It hasn’t finished its event processing yet.

You want to wait until it’s finished before you reset it. Use Platform.runLater to make sure you reset it after all pending events have been processed:

Platform.runLater(() -> item.setSelected(previousChoice));

Answer 3

I believe you got all the information about the issue from the previous answers and the comments in this post. So I will not repeat explaining the cause for the issue.

I can suggest one alternate workaround which is similar to @VGR's answer of using Platform.runLater. Though Platform.runLater acts like a master key to most of the issues, mostly it is not recommended regarding its unpredictability and the threading reasons.

The key reason for issue as mentioned in @VGR answer :

You are resetting the checkbox item too soon. It hasn’t finished its event processing yet. You want to wait until it’s finished before you reset it.

and in @kelopatra comment:

I think the culprit is the reverting of item's selected value in the listener

From the above reasons it looks like we can fix the issue, if we run the desired code at the end of the event processing (I mean after all the layout children is finished).

JavaFX has a very powerful class AnimationTimer. You can go through the API of this class, but in simple words, it will allow you to run your desired code at the end of every frame. In constrast to Platform.runLater, here we know when it is going to run. Infact usage of this class is actually recommened in Scene -> addPostLayoutPulseListener API.

Solution:

Create a utility class that accepts a Runnable and runs that code only ONCE at the end of the current scene pulse. By the time this code is executed, the layouting in the scene is already completed, so you will not have any issues.

import javafx.animation.AnimationTimer;

import java.util.concurrent.atomic.AtomicInteger;

/**
 * Utility to run the provided runnable in each scene pulse.
 */
public final class PulseUtil extends AnimationTimer {

    /** Specifies the count of current pulse. */
    private static final int CURRENT_PULSE = 0;

    /** Specifies the count of the timer once it is started. */
    private final AtomicInteger count = new AtomicInteger(0);

    /** Specifies the order at which the runnable need to be executed. */
    private final int order;

    /** Runnable to execute in each frame. */
    private final Runnable callback;

    /**
     * Constructor.
     *
     * @param aCallBack runnable to execute
     * @param aOrder order at which the runnable need to be executed
     */
    private PulseUtil(final Runnable aCallBack, final int aOrder) {
        callback = aCallBack;
        order = aOrder;
    }

    /**
     * Executes the provided runnable at the end of the current pulse.
     *
     * @param callback runnable to execute
     */
    public static void doEndOfPulse(final Runnable callback) {
        new PulseUtil(callback, CURRENT_PULSE).start();
    }

    @Override
    public void handle(final long now) {
        if (count.getAndIncrement() == order) {
            try {
                callback.run();
            } finally {
                stop();
            }
        }
    }
}

And you will call this method in the change listener:

private ChangeListener<Boolean> onSelectionChanged(CheckBoxTreeItem<String> item) {
    return (observableValue, previousChoice, newChoice) -> {
        if (newChoice) { // if checkbox is selected
            // if anything happens... for example press a "no" button in a dialog window
            if (5 > 4) {
                System.out.println("reset checkbox status");
                PulseUtil.doEndOfPulse(()->item.setSelected(previousChoice));
            }
        }
    };
}