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

fix: prohibit paste text/html content into email editor, allow paste … #5478

Open
wants to merge 4 commits into
base: next
Choose a base branch
from

Conversation

YugfeldID
Copy link

fix for #5445

What changed? Why was the change needed?

Blocks paste text/html content into the email editor (allow paste only text/plain). Problem: when the user copies text from the rich text editor (like the editor from the In-App channel) all styles will be copied, it leads to an inability to process variables and clear formatting.

Screenshots

Without fix pasted content can look like this (with the inability to change the font, clear background, remove list items, and bold formatting):
Снимок экрана 2024-04-30 в 16 59 29

Special notes for your reviewer

If this approach is okay, I will add a test

@YugfeldID YugfeldID requested a review from a team as a code owner April 30, 2024 14:12
Copy link

netlify bot commented Apr 30, 2024

👷 Deploy request for dev-web-novu pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 689c162

Copy link

netlify bot commented Apr 30, 2024

Deploy Preview for novu-design ready!

Name Link
🔨 Latest commit 689c162
🔍 Latest deploy log https://app.netlify.com/sites/novu-design/deploys/664b03e31238f60008b54426
😎 Deploy Preview https://deploy-preview-5478--novu-design.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@LetItRock LetItRock left a comment

Choose a reason for hiding this comment

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

hello @YugfeldID! 👋 thanks for the contribution! 🤝

I think that this is the right approach for now, but I noticed some problems with the current implementation:

  • variables are not recognized
  • the undo (cmd + Z) works weird

My feelings is that the inserted text node is not recognized somehow as a text change that's why the weird behaviour with undo. The another solution is to use document.execCommand('insertText', false, pastedData); but its deprecated and not recommended.

@YugfeldID
Copy link
Author

hello @YugfeldID! 👋 thanks for the contribution! 🤝

I think that this is the right approach for now, but I noticed some problems with the current implementation:

  • variables are not recognized
  • the undo (cmd + Z) works weird

My feelings is that the inserted text node is not recognized somehow as a text change that's why the weird behaviour with undo. The another solution is to use document.execCommand('insertText', false, pastedData); but its deprecated and not recommended.

Hello @LetItRock, thanks for feedback.

I pushed some changes:

  • methods.setValue(``${stepFormPath}.template.content.${blockIndex}.content``, ref.current?.innerHTML ?? '') should trigger changes -> variables will be parsed and placeholder will be gone
  • you are right, it seems like the only way to make undo works properly is using document.execCommand('insertText', false, pastedData);, because only it interacts with browser's undo manager

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants