RFR: JDK-8187314: All Cells: must show backing data always

[Andy Goryachev]

On Thu, 19 Oct 2023 06:58:26 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> Before, the `updateItem` method was called with the new value that was committed via `commitEdit()`.
>> This is problematic as developers may setup a commit handler via `setOnEditCommit`, which may reject the edit (or change the value otherwise).
>> We therefore do call the `updateItem(-1)` which will also call `updateItem` but with the real underlying value.
>> 
>> Changed and added tests for all 4 cells.
>
> As I said, everything is basically the same, but changed to `TreeItem`:
> 
> 
> import javafx.application.Application;
> import javafx.collections.FXCollections;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.TreeItem;
> import javafx.scene.control.TreeTableColumn;
> import javafx.scene.control.TreeTablePosition;
> import javafx.scene.control.TreeTableView;
> import javafx.scene.control.cell.TextFieldTreeTableCell;
> import javafx.scene.control.cell.TreeItemPropertyValueFactory;
> import javafx.scene.layout.BorderPane;
> import javafx.stage.Stage;
> 
> public class Test extends Application {
> 
>     TreeTablePosition<Dummy, String> editPosition;
>     private Object editValue;
>     @Override
>     public void start(Stage primaryStage) {
>         TreeTableView<Dummy> table = new TreeTableView<>();
>         table.setRoot(new TreeItem<>());
>         for (Dummy dummy : Dummy.dummies()) {
>             TreeItem<Dummy> dummyItem = new TreeItem<>(dummy);
> 
>             table.getRoot().getChildren().add(dummyItem);
>         }
> 
>         table.setEditable(true);
> 
>         TreeTableColumn<Dummy, String> first = new TreeTableColumn<>("Text");
>         first.setCellFactory(TextFieldTreeTableCell.forTreeTableColumn());
>         first.setCellValueFactory(new TreeItemPropertyValueFactory<>("dummy"));
> 
>         first.setOnEditStart(t -> editPosition = t.getTreeTablePosition());
>         first.addEventHandler(TreeTableColumn.editCommitEvent(), t -> {
>             editValue = t.getNewValue();
>             System.out.println("doing nothing");
> 
>         });
> 
>         table.getColumns().addAll(first);
> 
>         Button button = new Button("Check value");
>         button.setOnAction(e -> {
>             if (editPosition == null) return;
>             String value = editPosition.getTableColumn().getCellObservableValue(editPosition.getRow()).getValue();
>             System.out.println(
>                     "value in edited cell must represent backing data: " + value + " not the edited " + editValue);
>         });
>         BorderPane root = new BorderPane(table);
>         root.setBottom(button);
>         Scene scene = new Scene(root, 300, 250);
> 
>         primaryStage.setTitle("Hello World!");
>         primaryStage.setScene(scene);
>         primaryStage.show();
>     }
> 
>     public static void main(String[] args) {
>         launch(args);
>     }
> 
>     public static class Dummy {
>         private String dummy;
>         public Dummy(String dummy) {
>             this.dummy = du...

Thank you so much, @Maran23 !  It makes it so much easier to review.

Since the master progressed a bit since the last review, may I trouble you to merge in the latest master please?

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1197#issuecomment-1771855455