Skip to content

Commit 237c889

Browse files
hejsztynxhejsztynx
authored
fix(web): setLink consistency (#774)
# 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. https://github.com/user-attachments/assets/c54443bb-54d7-4525-a5bd-83cab884ebef After: https://github.com/user-attachments/assets/99ee4812-7b5f-439b-b708-26661b46c8c2 ## Compatibility | OS | Implemented | | ------- | :---------: | | iOS | ❌ | | Android | ❌ | | Web | ✅ | ## Checklist - [x] E2E tests are passing - [x] Required E2E tests have been added (if applicable)
1 parent 55002b1 commit 237c889

5 files changed

Lines changed: 107 additions & 7 deletions

File tree

‎.playwright/tests/links.spec.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ const sel = {
2929
selectionStart: '[data-testid="test-links-selection-start"]',
3030
selectionEnd: '[data-testid="test-links-selection-end"]',
3131
applySelection: '[data-testid="test-links-apply-selection-button"]',
32+
applySetLinkFromSelection:
33+
'[data-testid="test-links-apply-setlink-from-selection-button"]',
34+
selectionPayload: '[data-testid="test-links-selection-payload"]',
3235
onLinkDetectedPayload: '[data-testid="on-link-detected-payload"]',
3336
editorInner: '[data-testid="test-links-editor"] .eti-editor',
3437
editorScreenshot: '[data-testid="test-links-editor"]',
@@ -251,6 +254,61 @@ test.describe('test-links setLink table', () => {
251254
}
252255
});
253256

257+
test.describe('test-links setLink round-trips onChangeSelection text', () => {
258+
test('linking a selection across a block boundary keeps both paragraphs', async ({
259+
page,
260+
}) => {
261+
await gotoTestLinks(page);
262+
await setTestLinksEditorHtml(page, '<html><p>hello</p><p>world</p></html>');
263+
264+
await page.fill(sel.selectionStart, '3');
265+
await page.fill(sel.selectionEnd, '8');
266+
await page.fill(sel.setLinkUrl, 'https://swmansion.com');
267+
await page.click(sel.applySelection);
268+
269+
await expect
270+
.poll(async () => page.locator(sel.selectionPayload).textContent())
271+
.toBe(JSON.stringify({ start: 3, end: 8, text: 'lo\nwo' }));
272+
273+
await page.click(sel.applySetLinkFromSelection);
274+
275+
await expect
276+
.poll(async () => getTestLinksSerializedHtml(page))
277+
.toContain(
278+
'<p>hel<a href="https://swmansion.com">lo</a></p>' +
279+
'<p><a href="https://swmansion.com">wo</a>rld</p>'
280+
);
281+
});
282+
283+
test('linking a selection across a block boundary preserves inline marks', async ({
284+
page,
285+
}) => {
286+
await gotoTestLinks(page);
287+
await setTestLinksEditorHtml(
288+
page,
289+
'<html><p>hel<b>lo</b></p><p>world</p></html>'
290+
);
291+
292+
await page.fill(sel.selectionStart, '3');
293+
await page.fill(sel.selectionEnd, '8');
294+
await page.fill(sel.setLinkUrl, 'https://swmansion.com');
295+
await page.click(sel.applySelection);
296+
297+
await expect
298+
.poll(async () => page.locator(sel.selectionPayload).textContent())
299+
.toBe(JSON.stringify({ start: 3, end: 8, text: 'lo\nwo' }));
300+
301+
await page.click(sel.applySetLinkFromSelection);
302+
303+
await expect
304+
.poll(async () => getTestLinksSerializedHtml(page))
305+
.toContain(
306+
'<p>hel<a href="https://swmansion.com"><b>lo</b></a></p>' +
307+
'<p><a href="https://swmansion.com">wo</a>rld</p>'
308+
);
309+
});
310+
});
311+
254312
test.describe('test-links removeLink table', () => {
255313
const cases: {
256314
name: string;

‎apps/example-web/src/testScreens/TestLinks.tsx‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
EnrichedTextInput,
44
type EnrichedInputStyle,
55
type EnrichedTextInputInstance,
6+
type OnChangeSelectionEvent,
67
type OnLinkDetected,
78
} from 'react-native-enriched-html';
89
import { WEB_DEFAULT_HTML_STYLE } from '../defaultHtmlStyle';
@@ -34,6 +35,8 @@ export function TestLinks() {
3435
const [selEndInput, setSelEndInput] = useState('0');
3536
const [lastOnLinkDetected, setLastOnLinkDetected] =
3637
useState<OnLinkDetected | null>(null);
38+
const [lastSelection, setLastSelection] =
39+
useState<OnChangeSelectionEvent | null>(null);
3740

3841
useEffect(() => {
3942
setLinkRegexError('');
@@ -68,6 +71,9 @@ export function TestLinks() {
6871
onLinkDetected={(e) => {
6972
setLastOnLinkDetected(e);
7073
}}
74+
onChangeSelection={(e) => {
75+
setLastSelection(e.nativeEvent);
76+
}}
7177
linkRegex={appliedLinkRegex}
7278
/>
7379
</div>
@@ -223,8 +229,27 @@ export function TestLinks() {
223229
>
224230
setSelection
225231
</button>
232+
<button
233+
type="button"
234+
data-testid="test-links-apply-setlink-from-selection-button"
235+
onClick={() => {
236+
if (!lastSelection) return;
237+
ref.current?.setLink(
238+
lastSelection.start,
239+
lastSelection.end,
240+
lastSelection.text,
241+
linkUrlInput
242+
);
243+
}}
244+
>
245+
setLink from selection
246+
</button>
226247
</div>
227248

249+
<pre data-testid="test-links-selection-payload">
250+
{JSON.stringify(lastSelection)}
251+
</pre>
252+
228253
<pre data-testid="on-link-detected-payload">
229254
{JSON.stringify(lastOnLinkDetected)}
230255
</pre>

‎docs/docs/api-reference/enriched-text-input.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,12 @@ details.
844844
- `text: string` - displayed text of the link
845845
- `url: string` - URL of the link
846846

847+
:::note
848+
849+
Using this method with `text=""` will result in removal of the text marked by the `start` and `end` indexes.
850+
851+
;;;
852+
847853
### `.removeLink()`
848854

849855
```ts

‎src/web/formats/EnrichedLink.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ import Link, { type LinkOptions } from '@tiptap/extension-link';
22
import { mergeAttributes, type CommandProps } from '@tiptap/core';
33
import type { Editor } from '@tiptap/react';
44

5-
import { nativePosToTiptapPos } from '../nativeMappers/positionMapping';
5+
import {
6+
nativeLeafText,
7+
nativePosToTiptapPos,
8+
} from '../nativeMappers/positionMapping';
69
import { isLinkBlocked } from './formatRules';
710
import { findAutolinkRangesInWord } from '../pmPlugins/AutolinkPlugin/autolinkRegex';
811

@@ -138,9 +141,6 @@ export function setLink(
138141
text: string,
139142
url: string
140143
) {
141-
if (url.length === 0 || text.length === 0) {
142-
return;
143-
}
144144
const { state } = editor;
145145
const doc = state.doc;
146146
const from = nativePosToTiptapPos(doc, start);
@@ -150,6 +150,14 @@ export function setLink(
150150
return;
151151
}
152152

153+
if (text.length === 0 && from !== to) {
154+
editor.chain().focus().deleteRange({ from, to }).run();
155+
}
156+
157+
if (text.length === 0 || url.length === 0) {
158+
return;
159+
}
160+
153161
const linkType = state.schema.marks.link;
154162
if (!linkType) return;
155163
const linkMark = linkType.create({ href: url });
@@ -162,7 +170,7 @@ export function setLink(
162170
const marksWithLink = linkMark.addToSet(marksAtRangeStart);
163171
tr.insert(from, s.schema.text(text, marksWithLink));
164172
} else {
165-
const currentText = doc.textBetween(from, to);
173+
const currentText = nativeLeafText(doc, from, to);
166174

167175
if (text !== currentText) {
168176
const marksAtRangeStart = doc.resolve(from).marks();

‎src/web/tiptapWatchers/useOnLinkDetected.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import { getMarkRange, getMarksBetween } from '@tiptap/core';
44
import type { EditorState } from '@tiptap/pm/state';
55
import type { MarkType } from '@tiptap/pm/model';
66
import { emitLinkDetected, type LinkEmitterRef } from './emitLinkDetected';
7-
import { tiptapPosToNativePos } from '../nativeMappers/positionMapping';
7+
import {
8+
nativeLeafText,
9+
tiptapPosToNativePos,
10+
} from '../nativeMappers/positionMapping';
811

912
function findLinkRangeAt(
1013
state: EditorState,
@@ -53,7 +56,7 @@ export const useOnLinkDetected = (
5356
if (!linkMark) return;
5457

5558
emitLinkDetected(ref.current, {
56-
text: state.doc.textBetween(range.from, range.to, '\n'),
59+
text: nativeLeafText(state.doc, range.from, range.to),
5760
url: (linkMark.attrs.href as string | undefined) ?? '',
5861
start: tiptapPosToNativePos(state.doc, range.from),
5962
end: tiptapPosToNativePos(state.doc, range.to),

0 commit comments

Comments
 (0)