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(rich-text-editor): DP-116205 fix line break related issues #573

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

braddialpad
Copy link
Contributor

fix(rich-text-editor): DP-116205 fix line break related issues

Obligatory GIF (super important!)

Obligatory GIF

🛠️ Type Of Change

These types will increment the version number on release:

  • Fix

📖 Jira Ticket

https://dialpad.atlassian.net/browse/DP-116205

📖 Description

Changed to a different method to submit line breaks using shift + enter in rich-text-editor

💡 Context

We've been having various issues with the way enter / new lines are handled within rich-text-editor and have to keep putting in workaround hacks to get it to work how we want. (see jira ticket for latest weird issue). HardBreak extension seems to cause various unwanted issues and after browsing around stack overflow it seems the best solution may be to remove it entirely.

I had to fix this already as part of my hackathon project, so I'm moving the code from my hackathon project to the actual component. Appears to work much more reliably as it is using built in tiptap/prosemirror commands. Could be some unknown regression here so I'm going to check with the product teams before merging to ubervoice, and also do more testing on my own.

📝 Checklist

For all PRs:

  • I have ensured no private Dialpad links or info are in the code or pull request description (Dialtone is a public repo!).
  • I have reviewed my changes.
  • I have added all relevant documentation.
  • I have considered the performance impact of my change.

For all Vue changes:

  • I have made my changes in Vue 2 and Vue 3. Note: you may sync your changes from Vue 2 to Vue 3 (or vice versa) using the ./scripts/dialtone-vue-sync.sh script. Read docs here: Dialtone Vue Sync Script

🔮 Next Steps

update in ubervoice, confirm the problems are fixed.

🔗 Sources

ueberdosis/tiptap#2755

@braddialpad braddialpad added the no-visual-test Add this tag when the PR does not need visual testing label Nov 26, 2024
@braddialpad braddialpad self-assigned this Nov 26, 2024
};
},
});
if (!this.allowLineBreaks) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously, if allowLineBreaks was true, shift + enter would still work to generate a soft break, now it doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks 👍. fixed

@ninamarina
Copy link
Contributor

When copying a text that has soft breaks created with shift + enter, the line breaks are removed:

paste

@braddialpad
Copy link
Contributor Author

When copying a text that has soft breaks created with shift + enter, the line breaks are removed:

Good find. I have re-included the "HardBreak" extension but am no longer triggering it when pressing enter so we should still be able to support pasted hard breaks.

@iropolo
Copy link
Contributor

iropolo commented Nov 27, 2024

This could be an improvement of editor but does not fix the issue.
I can still reproduce copy/paste issue of https://dialpad.atlassian.net/browse/DP-116205

@braddialpad
Copy link
Contributor Author

This could be an improvement of editor but does not fix the issue. I can still reproduce copy/paste issue of https://dialpad.atlassian.net/browse/DP-116205

Hmm yeah as soon as I added the hardbreak extension back in the problem came up again... Will look into more tomorrow.

@ninamarina
Copy link
Contributor

Seems like the developers of the extension are fixing this: ueberdosis/tiptap#5679 (comment)

@braddialpad
Copy link
Contributor Author

Seems like the developers of the extension are fixing this: ueberdosis/tiptap#5679 (comment)

oh nice.. yeah I was trying to find a workaround but sounds like they are on top of it.

@braddialpad
Copy link
Contributor Author

Seems like the developers of the extension are fixing this: ueberdosis/tiptap#5679 (comment)

I've implemented the fix they put up in this PR into our own component. This can be removed once the new link-extension release is out but I'm putting it in here right now just for the purposes of fixing this in a timely manner.

Should work correctly now, contains hardbreak extension and allows for pasting links with hardbreaks.

@iropolo
Copy link
Contributor

iropolo commented Nov 29, 2024

console error on implementation

image

@braddialpad
Copy link
Contributor Author

console error on implementation

hmm looks like they have already merged the fix and are preparing a release. I'll just wait for the fix.

@braddialpad braddialpad force-pushed the fix/rich-text-enter branch 2 times, most recently from e8d77dd to 36ef498 Compare December 11, 2024 18:08
@braddialpad
Copy link
Contributor Author

The fix hasn't been released with extension-link yet, however it was released in linkifyjs. Just adding the correct version of that dependency has fixed the issue

@braddialpad
Copy link
Contributor Author

To be clear on the new behaviour:

when allowLineBreaks is true:

  • Enter creates a paragraph
  • Shift + Enter creates a line break
  • Ctrl/Cmd + Enter creates a line break

when allowLineBreaks is false:

  • Enter makes no change and fires an event
  • Shift + Enter creates a paragraph
  • Ctrl/Cmd + Enter creates a line break

@braddialpad braddialpad requested review from ninamarina and removed request for ninamarina December 11, 2024 20:17
@braddialpad
Copy link
Contributor Author

Ready for review.

Copy link

✔️ Deploy previews ready!
😎 Dialtone preview: https://dialtone.dialpad.com/deploy-previews/pr-573/
😎 Dialtone-vue 2 preview: https://dialtone.dialpad.com/vue/deploy-previews/pr-573/
😎 Dialtone-vue 3 the preview: https://dialtone.dialpad.com/vue3/deploy-previews/pr-573/

@juliodialpad
Copy link
Collaborator

Still broken...

Now the link with soft breaks gets split

image

Copy link
Collaborator

@juliodialpad juliodialpad left a comment

Choose a reason for hiding this comment

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

Looking good!

@@ -124,6 +124,7 @@
"date-fns": "2.30.0",
"docopt": "0.6.2",
"emoji-toolkit": "9.0.1",
"linkifyjs": "4.2.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a temp solution while they release the proper fix, right?

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 basically, they just need to update linkifyjs in extension-link and release it, then we can remove this.

@braddialpad braddialpad merged commit a204fd4 into staging Dec 12, 2024
10 checks passed
@braddialpad braddialpad deleted the fix/rich-text-enter branch December 12, 2024 19:01
braddialpad pushed a commit that referenced this pull request Dec 13, 2024
# [9.87.0](dialtone/v9.86.2...dialtone/v9.87.0) (2024-12-12)

### Bug Fixes

* recipes css cleanup ([#581](#581)) ([ba85404](ba85404))
* **Rich Text Editor:** DP-116205 fix line break related issues ([#573](#573)) ([a204fd4](a204fd4))

### Features

* **Callbar Button:** DLT-2211 add passthrough tooltip props ([#578](#578)) ([66cf439](66cf439))
* **Emoji:** NO-JIRA update emoji-toolkit to 9.0 ([#587](#587)) ([9684377](9684377))
* **Rich Text Editor:** DLT-2237 add image extension to editor ([#589](#589)) ([72cff78](72cff78))
braddialpad pushed a commit that referenced this pull request Dec 13, 2024
# [2.168.0](dialtone-vue2/v2.167.1...dialtone-vue2/v2.168.0) (2024-12-12)

### Bug Fixes

* recipes css cleanup ([#581](#581)) ([ba85404](ba85404))
* **Rich Text Editor:** DP-116205 fix line break related issues ([#573](#573)) ([a204fd4](a204fd4))

### Features

* **Callbar Button:** DLT-2211 add passthrough tooltip props ([#578](#578)) ([66cf439](66cf439))
* **Emoji:** NO-JIRA update emoji-toolkit to 9.0 ([#587](#587)) ([9684377](9684377))
* **Rich Text Editor:** DLT-2237 add image extension to editor ([#589](#589)) ([72cff78](72cff78))
braddialpad pushed a commit that referenced this pull request Dec 13, 2024
# [3.161.0](dialtone-vue3/v3.160.1...dialtone-vue3/v3.161.0) (2024-12-12)

### Bug Fixes

* recipes css cleanup ([#581](#581)) ([ba85404](ba85404))
* **Rich Text Editor:** DP-116205 fix line break related issues ([#573](#573)) ([a204fd4](a204fd4))

### Features

* **Callbar Button:** DLT-2211 add passthrough tooltip props ([#578](#578)) ([66cf439](66cf439))
* **Emoji:** NO-JIRA update emoji-toolkit to 9.0 ([#587](#587)) ([9684377](9684377))
* **Rich Text Editor:** DLT-2237 add image extension to editor ([#589](#589)) ([72cff78](72cff78))
@bianca-artola-dialpad
Copy link
Contributor

hey @braddialpad i'm having some issues when using shift + enter on the message input on beta

Screen.Recording.2024-12-17.at.12.41.32.PM.mov
Error: There is no node type named 'listItem'. Maybe you forgot to add the extension?

is there something we need to add on firespotter side to make it work?

@braddialpad
Copy link
Contributor Author

hey @braddialpad i'm having some issues when using shift + enter on the message input on beta

Screen.Recording.2024-12-17.at.12.41.32.PM.mov

Error: There is no node type named 'listItem'. Maybe you forgot to add the extension?

is there something we need to add on firespotter side to make it work?

@bianca-artola-dialpad strangely I'm not getting this at all when I try. Are you still getting it on beta?

@bianca-artola-dialpad
Copy link
Contributor

Not reproducible now on beta but I think is because of the new release process for beta in which we rollbacked to the previous release on Tuesday (so no front-end changes will be displayed for some hours). I'm having this issue when running it locally, but we can wait some hours after beta has the latest front-end changes

@braddialpad
Copy link
Contributor Author

Not reproducible now on beta but I think is because of the new release process for beta in which we rollbacked to the previous release on Tuesday (so no front-end changes will be displayed for some hours). I'm having this issue when running it locally, but we can wait some hours after beta has the latest front-end changes

Ah yeah that must be it.. I'll try locally.

@braddialpad
Copy link
Contributor Author

Found the problem, fix coming shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-visual-test Add this tag when the PR does not need visual testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants