Can JavaFX's ListChangeListener.Change.getRemoved() return non-contiguous items?
Asked Answered
T

1

17

Problem

When items are removed from an ObservableList, a change event is fired where getFrom() gives the location of the removal and getRemoved() gives a list of items that were removed. The documentation says:

The getRemoved() method returns a list of elements that have been replaced or removed from the list.

It is not stated as such, but I took it to be implied that the list of items is a contiguous sub-list from the original list. I've written a lot of code with that assumption, but am now encountering difficulties with TreeTableView's selection model, which doesn't behave that way.

Example

Take for instance a simple tree table with three "Node" rows. If I select those three rows...

three rows selected

...and then click and select just the middle row...

enter image description here

...the change event fired on treeTableView.getSelectionModel().getSelectedItems() looks like this:

{ [TreeItem [ value: Node 1 ], TreeItem [ value: Node 3 ]] removed at 0,  }

In a single change event it reports that "Node 1" and "Node 3" were removed from index 0 of the selectedItems list.

I would have expected the Change object to have two separate removal events separated by next() calls. The first call to next() would tell me that "Node 1" was removed at index 0, and the second call to next() would tell me that "Node 3" was removed at index 1. But no, I get a single event with both rows listed at once.

Question

Can getRemoved() really return non-contiguous items? Is this a misunderstanding on my part of how list change events work, or is it a bug in TreeTableView?

Normally I'm hesitant to blame standard libraries, but this wouldn't be the first bug I've found in JavaFX, so it's not unthinkable.


Update

If I add a call to setShowRoot(false), the behavior changes. I get what I would expect, the removal split into two pieces:

{ [TreeItem [ value: Node 1 ]] removed at 0, [TreeItem [ value: Node 3 ]] removed at 1,  }

Also, here is my MCVE:

import java.util.*;

import javafx.application.*;
import javafx.beans.property.*;
import javafx.collections.*;
import javafx.scene.*;
import javafx.scene.control.*;
import javafx.stage.*;

public class TreeTableSelectionEvents extends Application {
    public void start(Stage stage) {
        // Root node.
        TreeItem<String> root = new TreeItem<>("Root");

        root.setExpanded(true);

        root.getChildren().setAll(Arrays.asList(
            new TreeItem<>("Node 1"),
            new TreeItem<>("Node 2"),
            new TreeItem<>("Node 3")
        ));

        // Single column.
        TreeTableColumn<String, String> column = new TreeTableColumn<>("Column");

        column.setPrefWidth(150);
        column.setCellValueFactory((TreeTableColumn.CellDataFeatures<String, String> p) -> {
            return new ReadOnlyStringWrapper(p.getValue().getValue());
        });

        // Tree table.
        TreeTableView<String> table = new TreeTableView<>(root);

        table.getColumns().add(column);
        table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
        // table.setShowRoot(false);

        table.getSelectionModel().getSelectedItems().addListener(
            (ListChangeListener.Change<? extends TreeItem<String>> change) -> {
                System.out.printf("item change = %s, list is now %s%n", change, change.getList());
            }
        );

        table.getSelectionModel().getSelectedIndices().addListener(
            (ListChangeListener.Change<? extends Integer> change) -> {
                System.out.printf("index change = %s, list is now %s%n", change, change.getList());
            }
        );

        // Stage.
        stage.setScene(new Scene(table));
        stage.show();
    }
}
Thurman answered 12/12, 2015 at 0:18 Comment(5)
What does getTo() return in that scenario?Leatherman
getTo() = getFrom() = 0.Thurman
That's at least consistent with the documentation then. It seems the answer is simply "Yes, getRemoved() can return non-contiguous items". Is there any reason to believe that's a bug? I don't see anywhere in the documentation that states that's not allowed.Leatherman
Lists can contain duplicate items. If the items in getRemoved() aren't contiguous then I can't unambiguously determine which items were removed. It could be any of the duplicates. I suspect but am not sure that this is a flaw in the documentation and a bug in TreeTableViewSelectionModel.Thurman
Understood. Maybe it's a flaw in the API design...Leatherman
S
2

You're right, the event should contain two separate 'remove' changes. As of 1.8.0_74, TreeTableView's selection model seems hopelessly broken. It's not even consistent with TreeView's selection model, which is also screwy (but less so). There are so many failure modes, existing bugs and regression errors that it's tough to tell if Oracle is aware of problem. I'd suggest filing another bug. Below code provides a decent sandbox for playing with feature.

public class Test extends Application {
    public void start(Stage pStage) {
        pStage.setTitle("Test");

        final TabPane tabPane = new TabPane();

        tabPane.getTabs().addAll(
            Stream.of(true, false).map(
                pIsTreeTable -> {
                    final Tab result = new Tab(pIsTreeTable ? "TreeTableView" : "TreeView");

                    // create tree model
                    final TreeItem<String> root = new TreeItem<>("Root Node");
                    root.setExpanded(true);
                    final Collection<TreeItem<String>> children = IntStream.rangeClosed(
                        1, 5
                    ).mapToObj(pIdx -> new TreeItem<>("Child Node " + pIdx)).collect(
                        Collectors.toList()
                    );

                    // create TreeView or TreeTableView
                    final Control tree;
                    final MultipleSelectionModel<TreeItem<String>> selectionModel;
                    if (pIsTreeTable) {
                        final TreeTableView<String> treeTableView = new TreeTableView<>(
                            root
                        );
                        final TreeTableColumn<String,String> column = new TreeTableColumn<>(
                            "Column"
                        );
                        column.setCellValueFactory(
                            pTreeItem -> new ReadOnlyStringWrapper(
                                pTreeItem.getValue().getValue()
                            )
                        );
                        treeTableView.getColumns().add(column);

                        tree = treeTableView;
                        selectionModel = treeTableView.getSelectionModel();
                    } else {
                        final TreeView<String> treeView = new TreeView<>(root);

                        tree = treeView;
                        selectionModel = treeView.getSelectionModel();
                    }
                    selectionModel.setSelectionMode(SelectionMode.MULTIPLE);

                    // add buttons
                    final ToggleButton childrenBtn = new ToggleButton("Children");
                    childrenBtn.selectedProperty().addListener(
                        (pObservable, pOldVal, pNewVal) -> {
                            if (pNewVal) {
                                root.getChildren().addAll(children);
                            } else {
                                root.getChildren().clear();
                            }
                        }
                    );
                    childrenBtn.setSelected(true);
                    final ToggleButton showRootBtn = new ToggleButton("Show Root");
                    showRootBtn.setSelected(true);
                    (
                        pIsTreeTable ?
                        ((TreeTableView<?>) tree).showRootProperty() :
                        ((TreeView<?>) tree).showRootProperty()
                    ).bind(showRootBtn.selectedProperty());

                    // 'getSelectedItems()' tab
                    final Tab selectedItemsTab = new Tab("getSelectedItems()");
                    final TextArea selectedItemsTextArea = new TextArea();
                    selectionModel.getSelectedItems().addListener(
                        (ListChangeListener<TreeItem<String>>) pChange -> {
                            while (pChange.next()) {
                                if (pChange.getRemovedSize() > 0) {
                                    selectedItemsTextArea.appendText(
                                        "Removed " + pChange.getRemoved() + '\n'
                                    );
                                }
                                if (pChange.getAddedSize() > 0) {
                                    selectedItemsTextArea.appendText(
                                        "Added " + pChange.getAddedSubList() + '\n'
                                    );
                                }
                            }
                            selectedItemsTextArea.appendText(
                                "Selection: " + pChange.getList() + "\n\n"
                            );
                        }
                    );
                    selectedItemsTab.setContent(selectedItemsTextArea);

                    // 'getSelectedItem()' tab
                    final Tab selectedItemTab = new Tab("getSelectedItem()");
                    final TextArea selectedItemTextArea = new TextArea();
                    selectionModel.selectedItemProperty().addListener(
                        (pObservable, pOldVal, pNewVal) -> {
                            selectedItemTextArea.appendText("Selected " + pNewVal + '\n');
                        }
                    );
                    selectedItemTab.setContent(selectedItemTextArea);


                    // display selection data in text area
                    final TabPane selectionTabPane = new TabPane();
                    selectionTabPane.getTabs().addAll(selectedItemsTab, selectedItemTab);

                    final SplitPane splitPane = new SplitPane(
                        tree, new HBox(showRootBtn, childrenBtn), selectionTabPane
                    );
                    splitPane.setOrientation(Orientation.VERTICAL);

                    result.setContent(splitPane);

                    return result;
                }
            ).collect(Collectors.toList())
        );

        pStage.setScene(new Scene(tabPane, 300, 450));
        pStage.show();
    }

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

Related(?) issues:

  1. Depress 'Ctrl' button
  2. Select "Child Node 2"
  3. Select "Child Node 3"
  4. Select "Child Node 1"

ListChangeListener.Change event implies that "Child Node 2" was selected.

  1. Select "Root Node"
  2. De-select "Show Root" button

"Child Node 1" is selected but no selection event is broadcast.

  1. Select "Child Node 2"
  2. De-select "Children" button

'selection' list includes a null.

  1. Select "Child Node 1"
  2. Collapse "Root Node"

ListChangeListener.Change event does includes no removals.

  1. De-select "Children" button
  2. Select "Root Node"
  3. De-select "Show Root" button

Event is broadcast for neither TreeView nor TreeTableView. From there, if "Show Root" button is pressed, TreeView broadcasts event but TreeTableView doesn't.

Shakespeare answered 6/2, 2016 at 19:57 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.