Replace first cell content if input while on cell selection

[Andres-CT98]

Description:

Fix of #2023

Writing on a selected cell, or group of cells, does not replace the content of the first cell as expected.

Fix

On the TableCellSelection plugin, the handle key down event used to end the cell selection when typing. Now it makes only the first cell selected and creates a new selection of all its children.

Cases tested:

1. Create a table
2. Select one or more cells
3. Type something

Before: The place where the cursor was placed is where the typed text will be inserted.

After: The first cell in the selection, top left one, will be cleared and the typed text will be inserted.

Warnings and implications

• Keys like the function keys and Caps Lock will end cell selections. This behaviour exits before this change. However for the case of typing, after pressing said keys the full contents of the cell will still be selected, so typing and replacing the content should not be affected.

[JiuqingSong]

Is Input event preventable?

[JiuqingSong]

It seems we just need to handle keydown event, it will be good enough. I tried to comment out the handling code of input event, and it still works well

[Andres-CT98]

There is an issue with undo that is mitigated by using the Input method. It only happens if you type one character and do undo, the inputted character and the last written thing will both be undone. This issue is present today and not introduced by this change.

The reason I clear the full cell is that I do not know if select is enough to cover all possible elements in the cell for deletion. And for clearing I want to make sure it is an input and not other keys like caps lock.

[JiuqingSong]

About the undo issue, can you provide a full end to end repro step? I want to understand more about it. Thanks

[Andres-CT98]

1. Create a table
2. Fill it with values, remembering which one was the last one
3. Make a cell selection
4. Type one character
5. Undo

Notice that the typed character and the last filled value are both undone. The only one undone should be the typed one.

[JiuqingSong]

Ok, in that case we do need to call addUndoSnapshot() when type with table selection, but only need to do once.

[JiuqingSong]

And I think it should be good enough to just select everything inside the table cell when keydown. Because:

1. User made a table selection, then type
2. User selected everything in a table cell, then type

Both of them should get the same result. So making a selection to the first select cell means we convert 2 to 1, so that is the right operation.

[Andres-CT98]

I have been trying both selection types, but each have an issue. editor.select with the Td as parameter has the above issue, while editor.select with the table and coordinates as parameter has an issue of replacing everything in the cell when typing. For example if the cell has multiple lines, only the first one is replaced.

[JiuqingSong]

Actually, when I debug your code, Input event happens after KeyDown event. Since you have already changed selection when KeyDown event, it will fail the check of table selection range type, so will go directly to line 149, so code from line 142 to line 146 is never executed.

[Andres-CT98]

Interesting. I assumed it would retain the selection type.

[JiuqingSong]

Because you have called editor.select(), that will change selection type.

[JiuqingSong]

And because of that, the executing result is correct now. But if line 142-146 are executed, the new typed content will be removed, just because when Input event is triggered, the new typed content is already added into the view.

[Andres-CT98]

I have noticed that, would it work if I did the deletion before the Input event? I would need to add it to PluginEventType.

[JiuqingSong]

My feeling is you just need to keep you current code to select the content of first selected cell, (and call addUndoSnapshot() in a proper place), then let browser handle everything else, so no need to handle Input event at all.

Because Input event is triggered after type, so anything there is too late.

And maybe only need to handle character types (you can use isCharacterValue function to check) so for other keys like caps lock, leave it to browser

[JiuqingSong]

I made the following change locally to make undo work.

1. In handleKeydownEvent.ts:
   

2. In UndoPlugin.ts:
   

The problem is although we added new undo snapshot, but the flag "hasNewContenet" is also cleared. So we need to set it back when there is really input.

[BryanValverdeU]

Using Arrow Keys, Escape or some other keys that do not add content will add aｎ unneeded undo snapshot here.

[JiuqingSong]

Makes sense. In that case, my change in handleKeydownEvent.ts to addUndoSnapshot() is not needed

[Andres-CT98]

I can use isCharacterValue here to only add the snapshot if it is true.