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.

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

[MM-26675] Copy paste applies l ink markdown in edit_textbox #27097

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cyrusjc
Copy link

@cyrusjc cyrusjc commented May 22, 2024

Summary:

Resolves part 1 of ticket #26675.
Fixes bug where the link markdown was not applied if copy and paste was done inside of the text box.

Ticket Link:
Fixes #26675

Screenshots / Gifs
Before Animation2

After Animation

Release Note:

Fixes pasting behaviour to match text_box in the edit_textbox. Will now correctly apply link markdown

@mm-cloud-bot
Copy link

@cyrusjc: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@mattermost-build
Copy link
Contributor

Hello @cyrusjc,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@mm-cloud-bot mm-cloud-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed labels May 22, 2024
@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@amyblais amyblais added 2: Dev Review Requires review by a developer 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Jun 6, 2024
@unified-ci-app
Copy link
Contributor

unified-ci-app bot commented Jun 6, 2024

E2E test triggered successfully for PR #27097. The corresponding commit's status check will be available shortly.

Copy link

github-actions bot commented Jun 6, 2024

E2E test run is starting for commit 089474079a2d18224c437b4cae16e040c9d581bd.
You can check its progress by either:

Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

A minor question.

@@ -157,6 +161,35 @@ const EditPost = ({editingPost, actions, canEditPost, config, channelId, draft,
// just a helper so it's not always needed to update with setting both properties to the same value
const setCaretPosition = (position: number) => setSelectionRange({start: position, end: position});

useEffect(() => {
function onPaste(event: ClipboardEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a pasteHandler in webapp/channels/src/utils/paste.tsx.

Is this something we can reuse in any way? or are there strong reasons to keep this one different? (I see it is for the most part the same, but missing some things).

Copy link
Author

@cyrusjc cyrusjc Jun 13, 2024

Choose a reason for hiding this comment

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

there's no major differences besides the fact that the post textbox uses some props that don't exist in webapp/channels/src/components/edit_post/edit_post.tsx which is passed as arguments to pasteHandler(). An alternative method is to modify pasteHandler() to replace this logic which I originally didn't want to do but it does make sense for the logic for the post textbox and the edit textbox to be the same.

How would you want to approach this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go with updating the behavior in pasteHandler. Without having looked at it much, what I would do is pass a special location from this component (maybe add it to the Locations constants as "EDIT"), and handle the differences based on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a developer 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review Contributor Lifecycle/1:stale release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pasting a URL using ctrl+V should create a link, even when editing an existing message
5 participants