[Table/EditableCell] Type in a cell to edit it now #1760
Conversation
Type in a cell to edit it nowPreview: documentation | table |
| if (this.state.isEditing) { | ||
| return; | ||
| } | ||
| // setting dirty value to empty string because apparently the text field will pick up the key and write it in there |
There was a problem hiding this comment.
This is 123 characters long. Curious why prettier didn't catch this. Can we line-wrap comments to 80 for readability?
Also, can you elaborate a little more on this case?
There was a problem hiding this comment.
Not sure why it wasn't caught. Basically, apparently, you don't need to insert the character yourself. When switched over to an empty text input (or non-empty one, technically), the bubbling up to the top of the browser will apparently get it to write a character. This is... incredibly weird, but, clearly what is happening, and I'm much happier to let chrome type the character than hope that we got the right one out of the keyboard event.
cmslewis
changed the title
|
Working well. A couple of details that I spotted:
Also, for some reason, I'm getting much much better success rates when entering edit mode with double click... 🎉 |
|
@llorca doing this without focus cell enabled is... unsupported. Unless you think it should be? It is certainly somewhat strange (I would agree) that you can type in it at all without double click with focus cell off, and I can try to figure out how to tweak it to turn this off (not... entierly sure how) -- but regardless I don't think it should be a supported use-case. Can you clarify what you mean by "doesn't work quite well when typing and going to the next cell super fast."? |
|
@gscshoyru yeah I agree, shouldn't be supported, but editable cells without focus cells aren't explicitly disabled right now, which is why it's problematic. What I mean by "doesn't work quite well when typing and going to the next cell super fast." is: typing in stuff in a cell, then quickly press "enter" then quickly type in again. You'll see that the focus cell unsyncs from the cell being edited because of the fast interactions. If that doesn't make sense, I'll make a GIF tomorrow showcasing the issue. In any case, it'd be neat to fix but I suppose it's lower priority |
|
Hmmm... so there's an issue here with the fact that how I'm controlling/have to control focus doesn't work very well with batched rendering. @cmslewis is there a good way to override batching behavior so that it always renders a specific given cell (the focused one) first before anything else? That way focus behavior will behave appropriately, and not have to catch up to itself. |
|
That's not currently possible, no. |
|
Hm, if that's not possible then... I'm not sure how to make this work properly/quickly, with batching. Possibly we need to get cells to keep from constantly re-rendering when they don't need to -- maybe look into the logic some more. Also, @llorca I've disabled the ability to type-to-edit cells if focus is disabled, which is how it should be :) |
gscshoyru
force-pushed
the
gsc/type-to-edit-cells
branch
from
c2da50a
to
7ee50b7
Compare
Disable type-to-edit without focus cellPreview: documentation | table |
packages/table/src/cell/cell.tsx
Outdated
| @@ -79,6 +79,11 @@ export interface ICellProps extends IIntentProps, IProps { | |||
| onKeyUp?: React.KeyboardEventHandler<HTMLElement>; | |||
|
|
|||
| /** | |||
| * Callback invoked when a character-key is pressed | |||
There was a problem hiding this comment.
Need trailing period for consistency.
add periodPreview: documentation | table |
Changes proposed in this pull request:
Typing any character now replaces the cell content with that character and puts it in edit mode, essentially letting you arrow around and type in the correct cell.