I have a table and I need to add a context menu for rows. The problem is that context menu is shown also for empty rows. To fix it I remove and add context menu using listener on empty property. But taking into consideration that virtual rows are updated very often I am not sure that it is a performance-efficient solution.
This is my code:
public class NewMain1 extends Application {
public static class Person {
private final StringProperty firstName;
private final StringProperty lastName;
public Person(String firstName, String lastName) {
this.firstName = new SimpleStringProperty(firstName);
this.lastName = new SimpleStringProperty(lastName);
}
public StringProperty firstNameProperty() { return firstName; }
public StringProperty lastNameProperty() { return lastName; }
}
@Override
public void start(Stage primaryStage) {
TableView<Person> tableView = new TableView<>();
tableView.setEditable(true);
TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
firstNameCol.setCellValueFactory(cellData -> cellData.getValue().firstNameProperty());
firstNameCol.setCellFactory(TextFieldTableCell.forTableColumn());
TableColumn<Person, String> lastNameCol = new TableColumn<>("Last Name");
lastNameCol.setCellValueFactory(cellData -> cellData.getValue().lastNameProperty());
tableView.getColumns().addAll(firstNameCol, lastNameCol);
ObservableList<Person> data = FXCollections.observableArrayList(
new Person("John", "Smith"),
new Person("Emily", "Johnson"),
new Person("Michael", "Williams"),
new Person("Sarah", "Brown")
);
tableView.setItems(data);
tableView.setRowFactory(tv -> {
TableRow<Person> row = new TableRow<>();
var contextMenu = new ContextMenu(new MenuItem("Show Salary"));
row.emptyProperty().addListener((ov, oldV, newV) -> {
if (newV) {
row.setContextMenu(null);
} else {
row.setContextMenu(contextMenu);
}
});
return row;
});
VBox root = new VBox(tableView);
Scene scene = new Scene(root, 400, 300);
primaryStage.setTitle("Editable Names Table");
primaryStage.setScene(scene);
primaryStage.show();
}
public static void main(String[] args) {
launch(args);
}
}
Could anyone say if there is a better solution not to show context menu for empty rows?
I have a table and I need to add a context menu for rows. The problem is that context menu is shown also for empty rows. To fix it I remove and add context menu using listener on empty property. But taking into consideration that virtual rows are updated very often I am not sure that it is a performance-efficient solution.
This is my code:
public class NewMain1 extends Application {
public static class Person {
private final StringProperty firstName;
private final StringProperty lastName;
public Person(String firstName, String lastName) {
this.firstName = new SimpleStringProperty(firstName);
this.lastName = new SimpleStringProperty(lastName);
}
public StringProperty firstNameProperty() { return firstName; }
public StringProperty lastNameProperty() { return lastName; }
}
@Override
public void start(Stage primaryStage) {
TableView<Person> tableView = new TableView<>();
tableView.setEditable(true);
TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
firstNameCol.setCellValueFactory(cellData -> cellData.getValue().firstNameProperty());
firstNameCol.setCellFactory(TextFieldTableCell.forTableColumn());
TableColumn<Person, String> lastNameCol = new TableColumn<>("Last Name");
lastNameCol.setCellValueFactory(cellData -> cellData.getValue().lastNameProperty());
tableView.getColumns().addAll(firstNameCol, lastNameCol);
ObservableList<Person> data = FXCollections.observableArrayList(
new Person("John", "Smith"),
new Person("Emily", "Johnson"),
new Person("Michael", "Williams"),
new Person("Sarah", "Brown")
);
tableView.setItems(data);
tableView.setRowFactory(tv -> {
TableRow<Person> row = new TableRow<>();
var contextMenu = new ContextMenu(new MenuItem("Show Salary"));
row.emptyProperty().addListener((ov, oldV, newV) -> {
if (newV) {
row.setContextMenu(null);
} else {
row.setContextMenu(contextMenu);
}
});
return row;
});
VBox root = new VBox(tableView);
Scene scene = new Scene(root, 400, 300);
primaryStage.setTitle("Editable Names Table");
primaryStage.setScene(scene);
primaryStage.show();
}
public static void main(String[] args) {
launch(args);
}
}
Could anyone say if there is a better solution not to show context menu for empty rows?
Share Improve this question asked Mar 31 at 12:28 SilverCubeSilverCube 6403 silver badges12 bronze badges 3 |3 Answers
Reset to default 4Your idea is not bad in itself. But there are 2 points where you could optimize:
- Create only one context menu, use it throughout the rows
- Override the row's
updateItemmethod instead of using a listener on theemptyproperty
public class NewMain1 extends Application {
public static class Person {
private final StringProperty firstName;
private final StringProperty lastName;
public Person(String firstName, String lastName) {
this.firstName = new SimpleStringProperty(firstName);
this.lastName = new SimpleStringProperty(lastName);
}
public StringProperty firstNameProperty() { return firstName; }
public StringProperty lastNameProperty() { return lastName; }
}
@Override
public void start(Stage primaryStage) {
TableView<Person> tableView = new TableView<>();
tableView.setEditable(true);
TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
firstNameCol.setCellValueFactory(cellData -> cellData.getValue().firstNameProperty());
firstNameCol.setCellFactory(TextFieldTableCell.forTableColumn());
TableColumn<Person, String> lastNameCol = new TableColumn<>("Last Name");
lastNameCol.setCellValueFactory(cellData -> cellData.getValue().lastNameProperty());
tableView.getColumns().addAll(firstNameCol, lastNameCol);
ObservableList<Person> data = FXCollections.observableArrayList(
new Person("John", "Smith"),
new Person("Emily", "Johnson"),
new Person("Michael", "Williams"),
new Person("Sarah", "Brown")
);
tableView.setItems(data);
final var contextMenu = new ContextMenu(new MenuItem("Show Salary"));
tableView.setRowFactory(tv -> {
TableRow<Person> row = new TableRow<>() {
@Override
public void updateItem(Person item, boolean empty) {
super.updateItem(item, empty);
if (empty || item == null) {
setContextMenu(null);
} else {
setContextMenu(contextMenu);
}
}
};
return row;
});
VBox root = new VBox(tableView);
Scene scene = new Scene(root, 400, 300);
primaryStage.setTitle("Editable Names Table");
primaryStage.setScene(scene);
primaryStage.show();
}
public static void main(String[] args) {
launch(args);
}
}
I think this can be fixed easily. Override the method show() of ContextMenu and decide whether to show or not based on the row empty.
TableRow<Person> row = new TableRow<>();
ContextMenu contextMenu = new ContextMenu(new MenuItem("Show Salary")){
@Override
protected void show() {
if(!row.isEmpty()) {
super.show();
}
}
};
row.setContextMenu(contextMenu);
I'm a firm believer in not optimizing for performance unless there is a demonstrated issue, and trying foremost to write code that is clean and easy to read. The code in the OP is unlikely to cause performance issues, and is "standard" JavaFX code in that it uses standard features of the API (cell factories, bindings) in the way they are intended. Also note that the event handler for the menu item (which isn't shown in the OP) needs to access the item associated with the table row. This is much easier and cleaner if each row has its own context menu; of course this incurs a performance penalty as you create multiple context menus.
That said, here is (yet) another solution. Like julien.giband's solution, this creates a single context menu that is shared by all (non-empty) rows. This avoids changing the row's context menu property entirely; instead of setting that property, a listener for contextMenuRequested is registered with the row which displays the context menu only if the row is non-empty.
As with any solution using a single context menu for all rows, the code for the handlers for the menu items becomes more complex than using a different context menu for each row. To emphasize, under the principle of not complexifying code for performance reasons unless it's demonstrated that it's necessary, I would probably err on the side of using a different context menu for each row for this reason, unless profiling indicated it was necessary. I show one way to manage the menu item event handler here.
Here is the relevant code:
var contextMenu = new ContextMenu() {
private Person person;
void show(Node node, double x, double y, Person person) {
this.person = person;
show(node, x, y);
}
};
MenuItem salaryItem = new MenuItem("Show Salary");
salaryItem.setOnAction( _ ->
System.out.println("Salary for "+contextMenu.person.firstNameProperty().get())
);
contextMenu.getItems().add(salaryItem);
tableView.setRowFactory(tv -> {
TableRow<Person> row = new TableRow<>();
row.setOnContextMenuRequested(evt -> {
if (! row.isEmpty()) {
contextMenu.show(row, evt.getScreenX(), evt.getScreenY(), row.getItem());
}
});
return row;
});
and the complete example:
public class NewMain1 extends Application {
public static class Person {
private final StringProperty firstName;
private final StringProperty lastName;
public Person(String firstName, String lastName) {
this.firstName = new SimpleStringProperty(firstName);
this.lastName = new SimpleStringProperty(lastName);
}
public StringProperty firstNameProperty() { return firstName; }
public StringProperty lastNameProperty() { return lastName; }
}
@Override
public void start(Stage primaryStage) {
TableView<Person> tableView = new TableView<>();
tableView.setEditable(true);
TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
firstNameCol.setCellValueFactory(cellData -> cellData.getValue().firstNameProperty());
firstNameCol.setCellFactory(TextFieldTableCell.forTableColumn());
TableColumn<Person, String> lastNameCol = new TableColumn<>("Last Name");
lastNameCol.setCellValueFactory(cellData -> cellData.getValue().lastNameProperty());
tableView.getColumns().addAll(firstNameCol, lastNameCol);
ObservableList<Person> data = FXCollections.observableArrayList(
new Person("John", "Smith"),
new Person("Emily", "Johnson"),
new Person("Michael", "Williams"),
new Person("Sarah", "Brown")
);
tableView.setItems(data);
var contextMenu = new ContextMenu() {
private Person person;
void show(Node node, double x, double y, Person person) {
this.person = person;
show(node, x, y);
}
};
MenuItem salaryItem = new MenuItem("Show Salary");
salaryItem.setOnAction( _ ->
System.out.println("Salary for "+contextMenu.person.firstNameProperty().get())
);
contextMenu.getItems().add(salaryItem);
tableView.setRowFactory(tv -> {
TableRow<Person> row = new TableRow<>();
row.setOnContextMenuRequested(evt -> {
if (! row.isEmpty()) {
contextMenu.show(row, evt.getScreenX(), evt.getScreenY(), row.getItem());
}
});
return row;
});
VBox root = new VBox(tableView);
Scene scene = new Scene(root, 400, 300);
primaryStage.setTitle("Editable Names Table");
primaryStage.setScene(scene);
primaryStage.show();
}
public static void main(String[] args) {
launch(args);
}
}
TableRowinstances anyway, no matter how many items there are in the table. You could profile this, but I doubt you will see any performance issues at all. – James_D Commented Mar 31 at 13:50