Skip to content

fix(web): setLink consistency - #774

Open
hejsztynx wants to merge 7 commits into
mainfrom
@ksienkiewicz/fix-web-set-link-serialization-consistency
Open

fix(web): setLink consistency#774
hejsztynx wants to merge 7 commits into
mainfrom
@ksienkiewicz/fix-web-set-link-serialization-consistency

Conversation

@hejsztynx

@hejsztynx hejsztynx commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes #757

There were two issues with setLink on web:

  • the text serialization was inconsistent with what onChangeSelection returned
  • when text="", the method was no-op, whereas on mobile it removes the text (what we want)

Fix addresses both issues.

Added e2e tests for the first serialization related issue.

Added a temporary button in example-web app, so you can see what happens when you call setLink with text="". This will be reverted before the PR's merge.

Test Plan

Experiment with setLink across paragraph bounds and the setLink(text="") button.

Screenshots / Videos

Before:

The serialization differed, so setLink falsely assumed that text it got as an argument, differs from the actual text in the editor. This caused a path, where the content would get forcifully replaced with that "falsely new" text, which caused paragraphs to drop, and a literal \n to get inserted. It visually looks fine, but HTML proves the incorrectness.

Screen.Recording.2026-08-14.at.12.18.10.mov

After:

Screen.Recording.2026-08-14.at.12.19.42.mov

Compatibility

OS Implemented
iOS
Android
Web

Checklist

  • E2E tests are passing
  • Required E2E tests have been added (if applicable)

Copilot AI lite review requested due to automatic review settings August 14, 2026 09:54
Comment on lines +336 to +350
<button
className="btn btn-full"
onClick={() => {
if (!selection) return;
ref.current?.setLink(
selection.start,
selection.end,
'',
'swmansion.com'
);
}}
>
setLink(text="")
</button>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is for manual test purposes only. Will be removed on merge

@hejsztynx
hejsztynx marked this pull request as draft August 14, 2026 09:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes setLink behavior on web to match the native text-coordinate model and mobile semantics, ensuring link text serialization is consistent with onChangeSelection and allowing setLink(text="") to remove the selected range.

Changes:

  • Use nativeLeafText(...) for link text serialization in onLinkDetected and setLink comparisons to ensure consistent newline/leaf handling.
  • Update setLink on web to delete the selected range when text === "" (instead of being a no-op).
  • Add Playwright e2e coverage for cross-paragraph selection round-tripping and add example-web UI hooks to exercise the behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/web/useOnLinkDetected.ts Aligns emitted link text with native leaf-text serialization.
src/web/formats/EnrichedLink.ts Fixes setLink semantics (delete on empty text) and uses native leaf-text for equality checks.
apps/example-web/src/testScreens/TestLinks.tsx Captures and exposes onChangeSelection payload; adds a button to apply setLink from the captured selection.
apps/example-web/src/App.tsx Adds a temporary manual-test button to call setLink with text="".
.playwright/tests/links.spec.ts Adds e2e tests validating selection text round-trips through setLink across block boundaries (including inline marks).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/example-web/src/testScreens/TestLinks.tsx
Comment thread apps/example-web/src/App.tsx
@hejsztynx
hejsztynx marked this pull request as ready for review August 14, 2026 10:26
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.

[web] setLink corrupts multi-block selections; onChangeSelection text isn't round-trip safe

2 participants