#2369 chat paste bug

[SebinSong]

closes #2369

The textarea element needed a 'paste' event handler.

[cypress]

group-income    Run #3221

Run Properties:   Passed #3221  •  23b81cac9f ℹ️: Merge 52ad0c44789756887af679636c48dea6b3b358c0 into a8cbe02389bc3f56d5c566574d42...

Project	group-income
Branch Review	sebin/task/#2369-chat-paste-bug
Run status	Passed #3221
Run duration	09m 31s
Commit	23b81cac9f ℹ️: Merge 52ad0c44789756887af679636c48dea6b3b358c0 into a8cbe02389bc3f56d5c566574d42...
Committer	Sebin Song
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	10
Skipped	0
Passing	111
View all changes introduced in this branch ↗︎

[corrideat]

This works! However, I wonder if using the input event might be a more generic approach. This seems to work for me: @input='()=>this.updateTextArea()'.

[SebinSong]

@corrideat
If you look more closely in SendArea.vue, capturing what user has typed are all done via either calling this.$refs.textarea.value or through keydown event handler (which is already calling this.updateTextarea). so your solution will lead to calling it twice unnecessarily.

(Don't know if you write Vue code too but) It's more typical/generic approach to use v-model (which inherently uses input event) in a vue component.

But I'm sure the front-end author(s) of this component didn't use neither of them for a reason:
SendArea.vue currently has various UI/UX related logics triggered by user typing action (eg. auto-sizing the textarea element height if the content is multiple lines etc..) and they decided that using keydown event is more suitable to account for all of them.

So, adding input event handler is not only duplicate but also will very likely lead to creating conflicts with these existing logics. dismissing the review.

[taoeffect]

Thanks @SebinSong! There's a critical bug here:

[taoeffect]

This will replace all existing text with the pasted text, overwriting the value.

Instead of that, we want the pasted value to be inserted where the cursor is like normal.

[corrideat]

My suggestion of using the input event doesn't need this handlePaste handler (it needs a much simpler handler, as shown in my example). However, I didn't thoroughly test it.

and they decided that using keydown event is more suitable to account for all of them.

I did see the duplicate logic for triggering the update (actually, it could possibly be duplicate, if you paste without using keys it won't be a duplicate), however it doesn't seem like a big issue considering that it's not expensive to call it.

As for conflicts, I didn't really see anything that would conflict.

[SebinSong]

It does not make sense to add input when there is keyodwn already in place. It has to be either one of them. They are two events that look similar but behave differently in certain cases in my experience. For that reason it's a type of change I would be very cautious to do at this point (Considering this code has been this way for a long while). Also, as I said above, I think other front-end devs chose to use keydown instead of input for a reason.

[SebinSong]

@taoeffect Updated the fix again!

[taoeffect]

Nice work @SebinSong! 👏