JavaFX ListView showing old Cells, not clickable

Question

im currently coding a todo app.

Part of it is showing the notes and todos in a ListView, where the user can interact with them. However I have a toggle to determine between archived notes and active ones. When toggling the ObservableList gets updated and the ListView Cells as well, but somehow there end up some old notes which are not interactable anymore.

The top two notes are right, the bottom ones are left overs and not clickable.

I extend my NoteCell that gets displayed in the ListView from ListCell<>

import javafx.scene.control.*;

public class NoteCell extends ListCell<Note> {

    @FXML
    void initialize() { //Event Handlers}


    @Override
    protected void updateItem(Note note, boolean empty) {
        super.updateItem(note,empty);

        if (empty || note == null) {
            setText(null);
            setGraphic(null);
        } else {
            if (fxmlLoader == null) {
                fxmlLoader = new FXMLLoader(getClass().getResource("/view/NoteCells.fxml"));
                fxmlLoader.setController(this);
                try {
                    fxmlLoader.load();
                } catch (IOException e) {
                    e.printStackTrace();
                    logger.error("IOException: " + e);
                }
            }

            Platform.runLater(new Runnable() {
                @Override
                public void run() {
                    cellNoteTitle.setText(note.getTitle());
                    cellNoteDescription.setText(note.getContent());
                    cellNoteDescription.setWrapText(true);
                    cellNoteDescription.maxWidth(394);
                    cellNoteDescription.minWidth(394);
                    cellNoteDate.setText(String.valueOf(note.getCreationDate()));

                    setText(null);
                    setGraphic(rootPane);
                }
            });
        }
    }

I was thinking about a threading problem, but I did not get a clue. When changing to the archived view, I am fetching a parallel stream of Notes, so I figured they should get synchronized, but I don't know how I can achieve this. So I tried a normal stream but still the same issue.

Any help would be appreciated

The code of my overview controller, which has the ListView. Mainly interesting should be the toggleArchive method.

package mainpackage.controller;

import com.jfoenix.controls.*;
import javafx.application.Platform;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.layout.AnchorPane;
import javafx.scene.text.Font;
import javafx.stage.FileChooser;
import javafx.stage.Stage;
import mainpackage.ListManager;
import mainpackage.Main;
import mainpackage.animation.FadeIn;
import mainpackage.exceptions.UnsupportedCellType;
import mainpackage.model.Note;
import mainpackage.model.Task;
import mainpackage.threads.ClockThread;
import mainpackage.threads.SaveThread;

import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.util.*;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;


import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

/**
 * Main view after log in. Shows three different views of the created tasks.
 */

public class Overview {

    @FXML
    private ResourceBundle resources;
    @FXML
    private URL location;
    @FXML
    private AnchorPane rootPane;
    @FXML
    private Label dateLabel;
    @FXML
    private Label timeLabel;
    @FXML
    private ImageView overviewAddItemImage;
    @FXML
    private ImageView overviewAddNoteImage;
    @FXML
    private ImageView overviewCalendarImage;
    @FXML
    private ListView<Note> noteListView;
    @FXML
    private ImageView overviewExport;
    @FXML
    private JFXTextField noteListSearchField;
    @FXML
    private JFXComboBox<String> sortNoteListDropdown;
    @FXML
    private JFXToggleButton toggleArchiveButton;
    @FXML
    private ListView<Task> taskListView;
    @FXML
    private JFXTextField taskListSearchField;
    @FXML
    private JFXComboBox<String> sortTaskListDropdown;

    private static final Logger logger = LogManager.getLogger(Main.class.getName());
    private final ListManager listManager = new ListManager();
    private final ObservableList<Task> usersTasks = FXCollections.synchronizedObservableList(FXCollections.observableArrayList());
    private final ObservableList<Task> usersTasksSearch = FXCollections.synchronizedObservableList(FXCollections.observableArrayList());
    private final ObservableList<Note> usersNotes = FXCollections.synchronizedObservableList(FXCollections.observableArrayList());
    private final ObservableList<Note> usersNotesSearch = FXCollections.synchronizedObservableList(FXCollections.observableArrayList());
    private final ClockThread clock = new ClockThread();
//    private static final Logger log = LogManager.getLogger(Overview.class);

    @FXML
    synchronized void initialize() {


        logger.info("Overview initializing");
        //listManager.getNoteList().forEach(usersNotes::add);
        //listManager.getTaskList().forEach(usersTasks::add);
        //setLists();

        overviewCalendarImage.setOnMouseClicked(mouseEvent -> loadCalendar());
        overviewAddItemImage.setOnMouseClicked(mouseEvent -> loadAddTask());
        overviewAddNoteImage.setOnMouseClicked(mouseEvent -> loadAddNote());
        overviewExport.setOnMouseClicked(mouseEvent -> export());

        toggleArchiveButton.selectedProperty().addListener((arg0, arg1, arg2) -> {
            if(toggleArchiveButton.isSelected()) {
                noteListSearchField.clear();
                toggleArchive();
            }
            else {
                noteListSearchField.clear();
                toggleActive();
            }
        });

        noteListSearchField.textProperty().addListener((observable, oldValue, newValue) -> {
            //debugLogger.debug("Value Changed from: " + oldValue + " to " + newValue);

            if (!newValue.trim().isEmpty() && usersNotes.size() > 0) {
                usersNotesSearch.setAll(search(noteListSearchField.getText(), usersNotes));
                noteListView.setItems(usersNotesSearch);
            } else {
                noteListView.setItems(usersNotes);
            }

            //debugLogger.debug("Search");
            //debugLogger.info("TaskListView Size: " + todolistTaskList.getItems().size());
            //debugLogger.info("TaskList Size: " + taskListView.size());
            //debugLogger.info("tasks Arraylist Size: " + tasks.getTasks().size());
        });

        sortNoteListDropdown.setOnAction(event -> sortNotes(sortNoteListDropdown.getValue()));

        sortNoteListDropdown.setValue("Sort by date (newest to oldest)");

        //Initializing clock
        clock.setLabels(timeLabel, dateLabel);
        clock.setDaemon(true);
        clock.start();

        ExecutorService ex = Executors.newCachedThreadPool();
        ex.execute(this::setNotes);
        ex.execute(this::setTasks);
        ex.shutdown();

        sortNotes(sortNoteListDropdown.getValue());
    }

    /**
     * Sorting notes depending on selected String in sortNoteListDropdown (dropdown menu to sort notes in overview)
     * @param choice selected String in DropDown
     */
    private void sortNotes(String choice) {
        switch (choice) {
            case "Sort by date (newest to oldest)":
                sortDateDesc(usersNotes);
                break;
            case "Sort by date (oldest to newest)":
                sortDateAsc(usersNotes);
                break;
            case "Sort alphabetically (A-Z)":
                sortTitleAsc(usersNotes);
                break;
            case "Sort alphabetically (Z-A)":
                sortTitleDesc(usersNotes);
                break;
        }
    }


    /**
     * Clearing list of user's note and adding only archived notes.
     * Result: only archived notes are shown when toggleArchiveButton is selected
     */
    private synchronized void toggleArchive() {
        usersNotes.clear();
        listManager.getNoteList().filter(n -> n.getState()==2).forEach(usersNotes::add);
        sortNotes(sortNoteListDropdown.getValue());
    }

    /**
     * Clearing list of user's note and adding only archived notes.
     * Result: only active notes are shown when toggleArchiveButton is not selected
     */
    private void toggleActive() {
        usersNotes.clear();
        listManager.getNoteList().forEach((n) -> {
            if (n.getState() == 0) {
                usersNotes.add(n);
            }
        });
        sortNotes(sortNoteListDropdown.getValue());
    }

    /**
     * Sorting list of user's notes by date (descending)
     * @param usersNotes list of user's notes
     */
    private void sortDateDesc(ObservableList<Note> usersNotes) {
            usersNotes.sort((t1, t2) -> t2.getCreationDate().compareTo(t1.getCreationDate()));
            //debugLogger.info("List " + list.toString() + "  sorted by takdates in descending order.");
    }

    /**
     * Sorting list of user's notes by date (ascending)
     * @param usersNotes list of user's notes
     */
    private void sortDateAsc(ObservableList<Note> usersNotes) {
        usersNotes.sort(Comparator.comparing(Note::getCreationDate));
        //debugLogger.info("List " + list.toString() + "  sorted by takdates in descending order.");

    }

    /**
     * Sorting list of user's notes alphabetically (ascending)
     * @param usersNotes list of user's notes
     */
    private void sortTitleAsc(ObservableList<Note> usersNotes) {
        usersNotes.sort(Comparator.comparing(n -> n.getTitle().toUpperCase()));
        //debugLogger.info("List " + list.toString() + "  sorted by title in ascending order.");
    }

    /**
     * Sorting list of user's notes alphabetically (descending)
     * @param usersNotes list of user's notes
     */
    private void sortTitleDesc(ObservableList<Note> usersNotes) {
        usersNotes.sort((n1, n2) -> n2.getTitle().toUpperCase().compareTo(n1.getTitle().toUpperCase()));
        //debugLogger.info("List " + list.toString() + "  sorted by title in descending order.");
    }

    /**
     * Exporting notes and tasks into a .txt file on user's computer
     */
    private void export() {
        FileChooser fileChooser = new FileChooser();
        FileChooser.ExtensionFilter extFilter = new FileChooser.ExtensionFilter("TXT files (*.txt)", "*.txt");
        fileChooser.getExtensionFilters().add(extFilter);
        fileChooser.setInitialFileName("Orga-Exports.txt");
        File file = fileChooser.showSaveDialog(rootPane.getScene().getWindow());
        if (file != null) {
            SaveThread save = new SaveThread(file);
            save.setDaemon(true);
            save.start();
        }
    }

    public synchronized void setNotes() {

        // Placeholder if user has no notes
        Label noNotes = new Label("No notes yet!");
        noNotes.setFont(new Font(20));
        noteListView.setPlaceholder(noNotes);

        usersNotes.clear();
        listManager.getNoteList().filter(t->t.getState()==0).forEach(usersNotes::add);
        CellFactory cellFactory = new CellFactory();
        noteListView.setCellFactory(NoteCell -> {
            try {
                return cellFactory.createCell("note");
            } catch (UnsupportedCellType unsupportedCellType) {
                unsupportedCellType.printStackTrace();
                return new JFXListCell<>();
            }
        });
        noteListView.setItems(usersNotes);
        logger.info("Notes loaded to overview listview");
    }

    public synchronized void setTasks() {

        // Placeholder if user has no tasks
        Label noTasks = new Label("No tasks yet!");
        noTasks.setFont(new Font(20));
        taskListView.setPlaceholder(noTasks);

        usersTasks.clear();
        listManager.getTaskList().filter(t->t.getState()==0||t.getState()==1).forEach(usersTasks::add);
        CellFactory cellFactory = new CellFactory();
        taskListView.setCellFactory(TaskCell -> {
           try {
               return cellFactory.createCell("task");
           } catch (UnsupportedCellType unsupportedCellType) {
               unsupportedCellType.printStackTrace();
               return new JFXListCell<>();
           }
        });
        taskListView.setItems(usersTasks);

        logger.info("Tasks loaded to overview listview");

    }

    private void loadAddNote() {

        FXMLLoader loader = new FXMLLoader();
        loader.setLocation(getClass().getResource("/view/CreateNotes.fxml"));

        try {
            loader.load();
        } catch (IOException e) {
            e.printStackTrace();
        }

        Parent root = loader.getRoot();
        Stage stage = new Stage();
        stage.setScene(new Scene(root));
        stage.setResizable(false);
        stage.getIcons().add(new Image("icon/Logo organizingTool 75x75 blue.png"));
        overviewAddNoteImage.setDisable(true);
        stage.showAndWait();
        if (!toggleArchiveButton.isSelected()) {
            usersNotes.add(listManager.getLatestNote());
        }
        sortNotes(sortNoteListDropdown.getValue());
        overviewAddNoteImage.setDisable(false);

    }

    private void loadAddTask() {

        FXMLLoader loader = new FXMLLoader();
        loader.setLocation(getClass().getResource("/view/CreateTask.fxml"));
        loader.setController(new CreateTask());

        try {
            loader.load();
        } catch (IOException e) {
            e.printStackTrace();
        }

        Parent root = loader.getRoot();
        Stage stage = new Stage();
        stage.setScene(new Scene(root));
        stage.setResizable(false);
        stage.getIcons().add(new Image("icon/Logo organizingTool 75x75 blue.png"));
        overviewAddItemImage.setDisable(true);
        stage.showAndWait();
        overviewAddItemImage.setDisable(false);

    }

    private void loadCalendar() {

        Stage stage = (Stage) rootPane.getScene().getWindow();
        stage.setTitle("Calendar");

        AnchorPane calendar = null;
        FXMLLoader loader = new FXMLLoader();
        loader.setLocation(getClass().getResource("/view/Calendar.fxml"));
        try {
            calendar = loader.load();
        } catch (IOException e) {
            e.printStackTrace();
        }
        new FadeIn(rootPane).play();
        rootPane.getChildren().clear();
        rootPane.getChildren().setAll(calendar);

    }

    private ArrayList<Note> search(String filter, ObservableList<Note> list) {

        //debugLogger.info("Searching for the filter : " + filter + "in list " + list.toString());
        ArrayList<Note> searchResult = new ArrayList<>();
            if (!filter.isEmpty() && !filter.trim().equals("")) {
                //debugLogger.info("Searching for a task containing the filter: '" + filter + "'.");
                for (Note t : list) {
                    if (t.getTitle().toLowerCase().contains(filter.toLowerCase()) || t.getContent().toLowerCase().contains(filter.toLowerCase()) || t.getCreationDate().toString().contains(filter.toLowerCase())) {
                        searchResult.add(t);
                    }
                }
                return searchResult;
            } else if (searchResult.isEmpty()) {
                // debugLogger.info("No task found containing the filter: '" + filter + "'.");
            } else {
                searchResult.addAll(list);
            }
            return searchResult;

    }

    @FXML
    void logout(ActionEvent event) {

        ListManager.wipe();

        AnchorPane login = null;
        try {
            login = FXMLLoader.load(getClass().getResource("/view/Login.fxml"));
        } catch (IOException e) {
            e.printStackTrace();
        }
        rootPane.getChildren().setAll(login);
        new FadeIn(login).play();

    }
}

Answer 1

There are significant issues with your use of threads in the code you posted, which I won't address here since they're not the topic of the question. However, the updateItem() method in a cell subclass is always called on the FX Application Thread, so any use of Platform.runLater() there is redundant at best.

Calling Platform.runLater() from the FX Application Thread will place the supplied Runnable into a queue to be run on the same thread at a later time (essentially when all pending things the FX Application Thread have been completed).

The updateItem() method can be called quite frequently, especially when the ListView is first displayed, and when the user is scrolling. There is (deliberately) no defined order in which specific cells have their updateItem() methods invoked, and with which parameters. Thus a cell may become empty or non-empty at essentially arbitrary times.

If the ListView implementation chooses to temporarily make a cell non-empty and then immediately make it empty, your updateItem() method will be called twice in rapid succession on the FX Application Thread. The first call will schedule a runnable to be run later that sets the graphic to the content of the FXML file. The second call will set the graphic to null. If the second call happens before the runnable posted to the queue is executed, the cell which is supposed to be empty will display the content, because the calls to setGraphic() happen in the wrong order.

Simply remove the Platform.runLater(...) from updateItem().

@Override
protected void updateItem(Note note, boolean empty) {
    super.updateItem(note,empty);

    if (empty || note == null) {
        setText(null);
        setGraphic(null);
    } else {
        if (fxmlLoader == null) {
            fxmlLoader = new FXMLLoader(getClass().getResource("/view/NoteCells.fxml"));
            fxmlLoader.setController(this);
            try {
                fxmlLoader.load();
            } catch (IOException e) {
                e.printStackTrace();
                logger.error("IOException: " + e);
            }
        }

        cellNoteTitle.setText(note.getTitle());
        cellNoteDescription.setText(note.getContent());
        cellNoteDescription.setWrapText(true);
        cellNoteDescription.maxWidth(394);
        cellNoteDescription.minWidth(394);
        cellNoteDate.setText(String.valueOf(note.getCreationDate()));

        setText(null);
        setGraphic(rootPane);

    }
} 

Your current threading simply doesn't work: you are accessing shared data from multiple threads and updating UI elements from background threads. I would recommend removing all the background threads; if there really are tasks that need to be run in a background thread, you need to learn some of the JavaFX concurrency material. Read this post and this tutorial as a start.

Briefly, though, an asynchronous implementation of your toggleArchive() method might look something like this:

// move this to an instance field:
private ExecutorService ex = Executors.newCachedThreadPool();

private void toggleArchive() {
    final String choice = sortNoteListDropdown.getValue();
    Task<List<Note>> getNotesTask = new Task<>() {
        @Override
        public List<Note> call() {
            List<Note> notes = listManager.getNoteList()
                .filter(n -> n.getState()==2)
                .collect(Collectors.toList());
            // sort notes here based on choice
            // (note you could sort the stream after the filter
            // operation instead)
            return notes ;
        }
    };
    getNotesTask.setOnSucceeded(e -> userNotes.setAll(getNotesTask.getValue()));
    ex.submit(getNotesTask);
}

Here the potentially long-running task of retrieving and sorting the notes is done in a background thread, and operates solely on a separate list, without affecting the UI or any of the data on which it relies. When the task completes, the onSucceeded handler, which is called on the FX Application Thread, updates the ListView's items with the new data.

You need to make similar refactoring for all the asynchronous calls. Also remove all the low-level synchronized keywords, which, at least after this refactoring, will be unneccessary.