Skip to content

fix: Inserting link removes comment & add comment button click buggy #2620

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.

Sign up for GitHub

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

Merged
matthewlipski merged 12 commits into from
Apr 20, 2026
Merged

fix: Inserting link removes comment & add comment button click buggy #2620

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b89283f
Made inserting links not call `insertText` when text is the same
matthewlipski Apr 2, 2026
5a7a186
test: add e2e test for preserving marks when editing a link (#2573)
miadnguyen Apr 5, 2026
17386a6
Fixed some comments UI issues, updated test and added comments testin…
matthewlipski Apr 14, 2026
50fdda4
Updated test
matthewlipski Apr 14, 2026
437a7d3
Fixed failing link toolbar tests
matthewlipski Apr 14, 2026
46c0600
Small fix
matthewlipski Apr 16, 2026
82c5971
Implemented PR feedback
matthewlipski Apr 20, 2026
9e179fd
Merge branch 'main' into link-overwrite-comments-fix
matthewlipski Apr 20, 2026
3543c94
Fixed example generated `package.json` files
matthewlipski Apr 20, 2026
c58be94
Updated lock file
matthewlipski Apr 20, 2026
703e2fd
Changed example number
matthewlipski Apr 20, 2026
03d90d9
Updated lock file
matthewlipski Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/core/src/editor/managers/StyleManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,12 @@ export class StyleManager<
const { from, to } = tr.selection;

if (text) {
tr.insertText(text, from, to).addMark(from, from + text.length, mark);
const existingText = tr.doc.textBetween(from, to);
if (text !== existingText) {
tr.insertText(text, from, to);
}

tr.addMark(from, from + text.length, mark);
} else {
tr.setSelection(TextSelection.create(tr.doc, to)).addMark(
from,
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/extensions/LinkToolbar/LinkToolbar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ export const LinkToolbarExtension = createExtension(({ editor }) => {
if (!range) {
return;
}
tr.insertText(text, range.from, range.to);

Copy link
Copy Markdown
Contributor

@nperez0111 nperez0111 Apr 13, 2026

Choose a reason for hiding this comment

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

should actually probably use style manager.createLink here no?

Copy link
Copy Markdown
Collaborator Author

@matthewlipski matthewlipski Apr 14, 2026

Choose a reason for hiding this comment

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

Not exactly, since editLink takes an optional position whereas createLink always uses the selection start position to insert the link. This is necessary for hovered links, as the editor selection isn't related to the link position in this case.

We could ofc just update createLink to take an optional position/range, but because it's part of the editor API, I don't think we want to have anything related to ProseMirror positions there. Ideally this would be solved when we finish our own locations API.

const existingText = tr.doc.textBetween(range.from, range.to);
if (text !== existingText) {
tr.insertText(text, range.from, range.to);
}

tr.addMark(
range.from,
range.from + text.length,
Expand Down
Loading