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(NcRichContenteditable): past content to an empty field fails #6241

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

DorraJaouad
Copy link
Contributor

☑️ Resolves

🖼️ Screenshots

🏚️ Before 🏡 After
pasted-text pasted-text-fixed

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@DorraJaouad DorraJaouad added bug Something isn't working 3. to review Waiting for reviews feature: rich-contenteditable Related to the rich-contenteditable components labels Nov 19, 2024
@DorraJaouad DorraJaouad added this to the 8.20.1 milestone Nov 19, 2024
@DorraJaouad DorraJaouad self-assigned this Nov 19, 2024
Comment on lines -848 to -850
if (!this.isFF || !window.getSelection) {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If you remove this method, isFF is also not needed anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to pasting issue, but noticed:
if input is started or ended with embedded HTML (mention) "@Test01 anyword @Test01":

  • 🐞 in FF cursor isn't moving to the very start or end with ⬅️ ➡️ keys (⬆️ ⬇️ are working)
  • 🐞 in Safari cursor jumps to the very beginning
  • ✅ in Chrome everything is fine

Comment on lines 818 to 810
// Some browsers add a newline at the end of the contenteditable
text = text === '\n' ? '' : text
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment seems to not document the following line. Let's adjust to something like

Suggested change
// Some browsers add a newline at the end of the contenteditable
text = text === '\n' ? '' : text
// Browsers keep <br> after erasing contenteditable
text = text === '\n' ? '' : text
  • Some browsers => browsers (it is not browser-specific. Or is it?)
  • At the end => after erasing (not the ending but entire content is checked)

Copy link
Contributor

Choose a reason for hiding this comment

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

const text = this.parseContent(htmlOrText).replace(/^\n$/, '')

Tried to put it in parseContent, but it's breaking the component due to comparing trimmed values, so should be good here

@SystemKeeper
Copy link

image

Output raw still has a \n at the end? 🤔

@DorraJaouad
Copy link
Contributor Author

Output raw still has a \n at the end? 🤔

We don't remove it when there is text because browser will add it anyway, we remove when it's the only text (browser leftover)

@ShGKme
Copy link
Contributor

ShGKme commented Nov 20, 2024

Output raw still has a \n at the end? 🤔

It doesn't hurt (unlike then it contains only <br>)

@DorraJaouad
Copy link
Contributor Author

/backport to next

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Tested in Chrome, Firefox, Safari

Comment on lines 818 to 810
// Some browsers add a newline at the end of the contenteditable
text = text === '\n' ? '' : text
Copy link
Contributor

Choose a reason for hiding this comment

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

const text = this.parseContent(htmlOrText).replace(/^\n$/, '')

Tried to put it in parseContent, but it's breaking the component due to comparing trimmed values, so should be good here

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
…is content)

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
…e but has one new line (browser dependent)

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad merged commit 3756762 into master Nov 20, 2024
19 checks passed
@DorraJaouad DorraJaouad deleted the fix/4821/pasted-text branch November 20, 2024 12:37
@ShGKme ShGKme changed the title fix(NcRichContenteditable): fix pasted text handling fix(NcRichContenteditable): past content to an empty field fails Nov 20, 2024
@Antreesy Antreesy mentioned this pull request Nov 21, 2024
@Antreesy Antreesy modified the milestones: 8.20.1, 8.21.0 Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: rich-contenteditable Related to the rich-contenteditable components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NcRichContenteditable not accepting clipboard pasted text
4 participants