Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2369 chat paste bug #2370

Merged
merged 5 commits into from
Oct 7, 2024
Merged

#2369 chat paste bug #2370

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e8a776b
fix the bug re paste action on mobile
SebinSong Oct 4, 2024
9ac7665
remove console.log
SebinSong Oct 4, 2024
4ac1dbd
update the fix
SebinSong Oct 6, 2024
d0e590e
uncomment it
SebinSong Oct 6, 2024
52ad0c4
comment update again
SebinSong Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions frontend/views/containers/chatroom/SendArea.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
@keydown.ctrl='isNextLine'
@keydown='handleKeydown'
@keyup='handleKeyup'
@paste.prevent.stop='handlePaste'
v-bind='$attrs'
)

Expand Down Expand Up @@ -553,6 +554,11 @@ export default ({
this.updateMentionKeyword()
}
},
handlePaste (e) {
const pastedText = e.clipboardData.getData('text')
this.$refs.textarea.value = pastedText
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@corrideat corrideat Oct 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

this.updateTextArea()
},
addSelectedMention (index) {
const curValue = this.$refs.textarea.value
const curPosition = this.$refs.textarea.selectionStart
Expand Down