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

22803 - Pasting text or link in edit message pastes the text or link in main compose box #22817

Merged
merged 1 commit into from
Jul 13, 2023
Merged

22803 - Pasting text or link in edit message pastes the text or link in main compose box #22817

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 5 additions & 1 deletion src/components/Composer/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,11 @@ class Composer extends React.Component {
* @param {ClipboardEvent} event
*/
handlePaste(event) {
if (!this.props.checkComposerVisibility() && !this.state.isFocused) {
if (!this.props.checkComposerVisibility()) {
return;
}

if (['INPUT', 'TEXTAREA'].includes(event.target.nodeName)) {
Comment on lines 308 to +313
Copy link
Collaborator

Choose a reason for hiding this comment

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

@robertKozik This is flawed in the root itself. this.props.checkComposerVisibility() always fails for edit message items as we didn't pass checkComposerVisibility

Copy link
Collaborator

Choose a reason for hiding this comment

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

This event is called twice one from each instance. We should perform the operation only when it's from the textInput

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah you're right. I'm also looking for the another fix for this one. Did you check if pasting to the edited comment works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that paste event should be trigger only from focused input. The "parent" issue and PR #21583 was introducing the ability to paste when input is not composed

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, it didn't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

This snippet addresses both issues, but now if I edit an existing message and paste from the clipboard, the cursor remains at the beginning of the pasted message:

Screen.Recording.2023-07-13.at.1.37.54.PM.mov

I'd say let's revert this for now, and then work on a proper fix without having to rush.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes agreed lets do that first!

Copy link
Collaborator

@Santhosh-Sellavel Santhosh-Sellavel Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This one comes out of nowhere.

Please revert this first and lets work on the other issue alone!

Copy link
Contributor

Choose a reason for hiding this comment

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

Revert open here #22845, feel free to review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@robertKozik about the cursor issue

The contributor here found the PR #22443 which causes it

return;
}

Expand Down