Esc key doesn't cancel the cell change

[alonshmiel]

Steps to reproduce:

1. Double click on a cell in order to make it editable
2. Change the cell value
3. Click on Escape key.

Current result: The cell value is updated with the new value.

Expected result: The cell value should remain the same.

[alonshmiel]

I tried to find the cause and it happens because onCellChanged gets commit (boolean). We call it on blur with commit=true.

It looks like we need to set commit=true only if the user didn't press on Escape. There is a skipped test for that.

I will make a PR of fixing this issue: #97

As I know, the react event is syntheticBaseEvent so we can't use event.key or event.keyCode in order to know the pressed key. That's why I used 'any' type for the event in order to use its 'view' property (it's DOMAbstractView).

[samkuehn]

Another option that might be useful would be to add a "reason" to the changes property on the onCellsChanged event.

[
    {
        "previousCell": {
            "type": "text",
            "text": "foo",
            "value": null,
            "placeholder": ""
        },
        "newCell": {
            "type": "text",
            "text": "bar",
            "value": null,
            "placeholder": ""
        },
        "type": "text",
        "rowId": 1,
        "columnId": "foo",
        "reason": "esc"  // <--- reason prop (escape, arrow, enter, mouseClick, etc.)
    }
]

This way you could choose what to do in the onCellsChanged handler - discard, persist... whatever.

You could also just attach the actual event that trigged the onCellChanged event.

[sittaendah]

You can create custom cell template and add the desired behaviour on handleKeyDown for ESCAPE key. I tried and it works

[alon-shmiel-sage]

@sittaendah , yes, this is what I did when I created "currency" cell. The number cell allows to enter 0----0 or 2....7, etc.

Anyway, thank you very much for your suggestion! :)

[DLowHP]

I believe the bug is solved, so I'm going to close the issue. If you would still like to request a "cell change reason" feature please create a new issue, thanks! Take care!