JavaFX ListView showing old Cells, not clickable
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(); } }
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.