3

I have a table written in JavaFX, that the user has the option to edit one of its columns. When an editing of a cell takes place, I try to rename some internal object based on the new text. If that renaming fails, I issue a notification + popup an error dialog, and then I'd like to restore the original text into that field. For some reason, this gets me into an infinite loop of error popups.

The code I currently have from the original writer is as follows:

mNameColumn.setOnEditCommit(
    new EventHandler<TableColumn.CellEditEvent<StateDefinition, String>>() {
        @Override
        public void handle(TableColumn.CellEditEvent<StateDefinition, String> event) {
            try {
                someObject.rename(((StateDefinition) event.getTableView().getItems().get(event.getTablePosition().getRow())).getState(), event.getNewValue());
            } catch (MYException ex) {
                MyNotificationCollector.addNotification("Failed renaming: " + ex.getLocalizedMessage(), NotificationType.SYSTEM_WARNING);
                //popup error message code here
                // Need to revert the cell's text to the previous value here...
            }
        }
    });

Any idea on what I do wrong? How do I restore the old value without invoking the cell edit commit event again?

Thanks, Oren

Oren Sarid
  • 143
  • 1
  • 12
  • same problem as an older question: https://stackoverflow.com/questions/20798634/restore-oldvalue-in-tableview-after-editing-the-cell-javafx (closed as duplicate of this) – kleopatra Feb 04 '22 at 15:10

2 Answers2

3

Try to use

TableColumn.CellEditEvent.getOldValue()

For example:

        try {
            someObject.rename(((StateDefinition) event.getTableView().getItems()
               .get(event.getTablePosition().getRow())).getState(), 
                   event.getNewValue());

        } catch (MYException ex) {
            MyNotificationCollector.addNotification("Failed renaming: " 
                + ex.getLocalizedMessage(), NotificationType.SYSTEM_WARNING);

            // popup error message code here
            // Revert the cell's text to the previous value here...

           someObject.rename(((StateDefinition) event.getTableView().getItems()
                .get(event.getTablePosition().getRow())).getState(),
                   event.getOldValue());

            // workaround for refreshing rendered view
            event.getTableView().getColumns().get(0).setVisible(false);
            event.getTableView().getColumns().get(0).setVisible(true);
        }

The last 2 lines are oldy workarounds for refreshing tableview rendered values.

EDIT:

If the backed data model (item fields of tableview) is not changed when the exception is thrown then just update tableview rendering in catch block with:

} catch (MYException ex) {
            MyNotificationCollector.addNotification("Failed renaming: " 
                + ex.getLocalizedMessage(), NotificationType.SYSTEM_WARNING);

            // workaround for refreshing rendered view
            event.getTableView().getColumns().get(0).setVisible(false);
            event.getTableView().getColumns().get(0).setVisible(true);
        }
Uluk Biy
  • 48,655
  • 13
  • 146
  • 153
  • The rename itself failed, so calling rename again is redundant. All I need is the UI (table) to revert to the old value. My problem only happens when the popup is involved, somehow it causes the event to take place again and again and I can't stop it. – Oren Sarid Apr 08 '15 at 13:14
  • So I guess what I want to do is to somehow invoke the cancel edit event and/or force the cell to get its old value back. – Oren Sarid Apr 08 '15 at 13:22
  • @OrenSarid see update. I can't tell anything about your popup, since there is no info about it. – Uluk Biy Apr 08 '15 at 13:40
  • The popup is just a JavaFX dialog I show with an OK button to draw the user's attention. But if I use it - the event keeps showing again and again. Anyway, I tried your workaround and it doesn't help as the value I get is sometimes wrong (I update to a valid name, then an invalid one, and then I get the first one in the refresh). – Oren Sarid Apr 08 '15 at 13:47
  • @OrenSarid, well first of all, comment out the popup logic, to primarily resolve the table problem. Next there are two ways, when validating user committed value. 1) If the committed value is invalid it will not reflected to datamodel so refreshing described above is enough. 2) If the committed value is invalid but reflected, then setting again to the already validated original getOldValue() is enough. – Uluk Biy Apr 08 '15 at 14:00
  • Sorry, but it's not correct. The data model behind the UI is _not_ updated for sure when the value is invalid. Then, removing the popup line doesn't refresh the table to the old value. When the popup line is there, it just keeps on sending the event everytime the popup's OK button is pressed and the popup is dismissed. – Oren Sarid Apr 08 '15 at 14:03
  • On my testing code, the old value is rendered again when the exception is thrown before setting the datamodel. So @OrenSarid, providing a MVCE will be helpful here, to see the problem more closely. – Uluk Biy Apr 08 '15 at 14:11
  • it's a bug https://bugs.openjdk.java.net/browse/JDK-8187314 (on my near-future TODO list ;) – kleopatra Feb 04 '22 at 13:55
1

It's a bug in TableCell (actually, all cells - though not noticeable for all types due to different reasons): happens if a custom commit handler rejects the edited value (or changes it in any way before writing it back to the data). The technical reason is calling updateItem with the newValue

@Override 
public void commitEdit(T newValue) {
    // ... cell book-keeping
    // ... fire editCommitEvent to allow handler to save value
    // update the item within this cell, so that it represents the new value
    updateItem(newValue, false); // <-- assumes that the handler saves the new value as-is

Until the issue is fixed (soon, hopefully ;) we can work around it by overriding the method, let it call super and then force a complete update:

@Override
public void commitEdit(T newValue) {
    super.commitEdit(newValue);
    updateIndex(getIndex());
}
kleopatra
  • 51,061
  • 28
  • 99
  • 211