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

Revert "22803 - Pasting text or link in edit message pastes the text or link in main compose box" #22845

Merged
merged 2 commits into from
Jul 13, 2023
Merged

Revert "22803 - Pasting text or link in edit message pastes the text or link in main compose box" #22845

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33cf4c7
Revert "22803 - Pasting text or link in edit message pastes the text …
francoisl Jul 13, 2023
de52f82
Remove unneeded extra condition
francoisl Jul 13, 2023
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: 1 addition & 5 deletions src/components/Composer/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,7 @@ class Composer extends React.Component {
* @param {ClipboardEvent} event
*/
handlePaste(event) {
if (!this.props.checkComposerVisibility()) {
return;
}

if (['INPUT', 'TEXTAREA'].includes(event.target.nodeName)) {
if (!this.props.checkComposerVisibility() && !this.state.isFocused) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not fix anything

Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add this check also if (this.textInput.id !== event.target.id) { return; }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting we keep if (!this.props.checkComposerVisibility()) { then? I went with a straight revert but I can update if you'd prefer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this bug here seems to be low on priority and better than both!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Collaborator

Choose a reason for hiding this comment

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

So we could create a new issue for that alone while we look for a solution. What do you say?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry we must have posted at the same time and GitHub didn't initially show me this message.

We're discussing 1:1 now to figure out the best solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update for visibility: we looked into a few other options but didn't find one that fully worked. The current plan is to:

  • Go ahead and merge this revert PR so that we can unblock the production deploy (we're kind of in a rush today so we can deploy other important changes)
  • Look for a proper solution later on

return;
}

Expand Down