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

Inserting image into line of text will add line break two chars after #3427

Closed
max-nextcloud opened this issue Nov 15, 2022 · 2 comments · Fixed by #5095
Closed

Inserting image into line of text will add line break two chars after #3427

max-nextcloud opened this issue Nov 15, 2022 · 2 comments · Fixed by #5095

Comments

@max-nextcloud
Copy link
Collaborator

This is a follow up to #3282.

We currently the cursor two steps further after inserting the image and before inserting the hard break (if the image is not inserted as first element, which needs other special treatment). This is necessary because per default the cursor is in front of the image after inserting it, but we want to add the hard break afterwards.

If you insert an image in between text, this results in the linebreak being inserted two characters further for the moment. I think that's an acceptable tradeoff given that images are block nodes now and most likely it will not happen that often that people insert them in the middle of text anyway.

Here's the relevant code:

const selection = this.$editor.view.state.selection
if (!selection.empty) {
// If inserted image is first element, it is selected and would get overwritten by
// subsequent editor inserts (see tiptap#3355). So unselect the image by placing
// the cursor at the end of the selection.
this.$editor.commands.focus(selection.to)
} else {
// Place the cursor after the inserted image node
this.$editor.commands.focus(selection.to + 2)
}

If you have a quick idea how to detect that the cursor (i.e. the focus, this.$editor.view.state.selection) is in the middle of some text and not at the end of a paragraph, we could add this as another special handling. But instead of waiting longer and spending another bunch of hours on this minor issue, my suggestion would be to merge this PR as is now 😉

Originally posted by @mejo- in #3282 (comment)

@max-nextcloud
Copy link
Collaborator Author

@max-nextcloud
Copy link
Collaborator Author

This is still an issue. We should really use gapcursor instead of adding empty lines.

@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team Jun 16, 2023
@mejo- mejo- moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📝 Office team Jul 25, 2023
mejo- added a commit that referenced this issue Dec 6, 2023
@mejo- mejo- moved this from 📄 To do (~10 entries) to 🏗️ In progress in 📝 Office team Dec 6, 2023
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 📝 Office team Dec 6, 2023
mejo- added a commit that referenced this issue Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants