Skip to content

Conversation

@JostSchenck
Copy link
Contributor

This fixes #2593 . Currently, the code silently assumes that when text is pasted, it covers complete nodes, and always places the caret at the end. However, when only some text without paragraphs is pasted into the middle of a paragraph, the user expects the caret to be placed at the end of the inserted text.

@JostSchenck
Copy link
Contributor Author

Adding to this: The two failing tests are both unrelated.
@matthew-carroll @angelosilvestre If there is anything else I can do to make this acceptable, I'd be happy to do so -- the bug is pretty annoying for the user, at least on all desktop platforms.

Comment on lines 2523 to 2532
// The user only pasted content without any newlines in it. Place the
// caret in the existing node at the end of the pasted text. This is
// guaranteed to be a TextNode.
documentPositionAfterPaste = DocumentPosition(
nodeId: _pastePosition.nodeId,
nodePosition: TextNodePosition(
offset:
pasteTextOffset + (_parsedContent!.first as TextNode).text.length,
),
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know we only support pasting TextNodes now, but I believe it would be better to explicitly check if the node is a TextNode before entering this branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that it's about the pasted node, yes? Alright, I changed that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's about _parsedContent!.first.

Comment on lines 82 to 84
expect(selection, isNotNull);
expect(selection!.isCollapsed, isTrue);
expect((selection.base.nodePosition as TextNodePosition).offset, 24);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of manually checking different properties I believe it would me more consistent with the repo's style if you use the selectionEquivalentTo method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I should get acquainted to those, thanks, done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you forget to push it? It doesn't seem it was changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, pushed to my fork, now synced

Copy link
Collaborator

@angelosilvestre angelosilvestre left a comment

Choose a reason for hiding this comment

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

Hey @JostSchenck. Thanks for the PR. I left a few comments.

… single-node content; slight reformatting to repo standard line length; learnt about selectionEquivalentTo matcher.
Comment on lines +100 to +101
await tester.typeImeText(
" "); // <- manually add a space because Markdown strips it
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change and the following changes seem to be made by the auto formatter. Could you please revert it to remove noise from the PR?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] - Cursor Resets to End of Paragraph After Pasting Text in Middle of Paragraph

2 participants