remove old link when editing #5690

grnd-alt · 2024-04-15T13:11:24Z

max-nextcloud · 2024-04-17T09:01:33Z

The current change works well when only changing a single link in a paragraph.
There are two more scenarios that would be good to account for:

Link in a paragraph

Find more information on https://nextcloud.com as usual.

-> should only change the link.

Link with custom text

More informarion

-> should only change the href.

max-nextcloud

As I said earlier... this will not work if the link is not the only thing in a paragraph.

@grnd-alt would you be up for picking this up again?

codecov · 2025-02-12T10:23:00Z

Codecov Report

Attention: Patch coverage is 93.18182% with 3 lines in your changes missing coverage. Please review.

Project coverage is 51.90%. Comparing base (bc0916a) to head (08a528b).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
src/marks/Link.js	95.34%	2 Missing ⚠️
src/components/Menu/ActionInsertLink.vue	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5690      +/-   ##
==========================================
+ Coverage   51.84%   51.90%   +0.06%     
==========================================
  Files         475      475              
  Lines       40132    40157      +25     
  Branches      983      989       +6     
==========================================
+ Hits        20805    20845      +40     
+ Misses      19222    19207      -15     
  Partials      105      105

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

max-nextcloud

Looks good. Just one small remark about the test code.

src/tests/marks/Link.spec.js

juliusknorr · 2025-03-07T13:14:50Z

Warning: Failed to restore: getCacheEntry failed: Cache service responded with 500

Looked odd, restarted cypress

max-nextcloud

Just a minor comment. Other than that seems good code wise. Have not tested it though.

max-nextcloud · 2025-03-19T07:07:16Z

src/tests/marks/Link.spec.js

+	it('should insert new link if none at anchor', () => {
+		const editor = createCustomEditor(
+			'<p><a href="nextcloud.com">Test</a> HELLO WORLD</p>',
+			[Link],
+		)
+		editor.commands.setTextSelection(10)
+		expect(editor.getJSON()).toEqual({
+			content: [
+				{
+					content: [
+						{
+							marks: [
+								{ attrs: { href: 'nextcloud.com', title: null }, type: 'link' },
+							],
+							text: 'Test',
+							type: 'text',
+						},
+						{ text: ' HELLO WORLD', type: 'text' },
+					],
+					type: 'paragraph',
+				},
+			],
+			type: 'doc',
+		})
+	})


This test seems incomplete. It does not change anything but just asserts the initial state.

Signed-off-by: grnd-alt <[email protected]>

Signed-off-by: Max <[email protected]>

It is a proper e2e test driving the ui. Signed-off-by: Max <[email protected]>

Signed-off-by: Max <[email protected]>

This way we do not hand in the active state and avoid inconsistencies. Signed-off-by: Max <[email protected]>

Signed-off-by: grnd-alt <[email protected]>

max-nextcloud · 2025-03-19T13:15:37Z

Failing psalm test is unrelated. I'd be okay with force merging.

grnd-alt linked an issue Apr 15, 2024 that may be closed by this pull request

Updating the URL of a link via toolbar menu splits the link #5516

Closed

juliusknorr requested review from elzody and max-nextcloud April 16, 2024 10:37

juliusknorr added bug Something isn't working 3. to review labels Apr 16, 2024

juliusknorr added this to the Nextcloud 30 milestone Apr 16, 2024

max-nextcloud self-assigned this Apr 17, 2024

max-nextcloud force-pushed the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch 2 times, most recently from 10bfdeb to 35f1d07 Compare April 17, 2024 14:17

This was referenced Jul 30, 2024

30.0.0 beta 2 nextcloud/server#46870

Merged

30.0.0 beta 3 nextcloud/server#46946

Merged

This was referenced Aug 5, 2024

30.0.0 beta 4 nextcloud/server#47028

Merged

30.0.0 beta 5 nextcloud/server#47100

Merged

skjnldsv mentioned this pull request Aug 13, 2024

30.0.0 RC1 nextcloud/server#47190

Merged

skjnldsv mentioned this pull request Aug 22, 2024

30.0.0 RC2 nextcloud/server#47411

Merged

44 tasks

This was referenced Aug 29, 2024

30.0.0 RC3 nextcloud/server#47603

Merged

30.0.0 RC4 nextcloud/server#47757

Merged

blizzz modified the milestones: Nextcloud 30, Nextcloud 31 Sep 4, 2024

blizzz mentioned this pull request Jan 8, 2025

31.0.0 beta 2 nextcloud/server#50095

Merged

This was referenced Jan 14, 2025

31.0.0 beta 3 nextcloud/server#50178

Merged

31.0.0 beta 4 nextcloud/server#50216

Merged

This was referenced Jan 21, 2025

31.0.0 beta 5 nextcloud/server#50285

Merged

31.0.0 RC1 nextcloud/server#50329

Merged

blizzz mentioned this pull request Jan 29, 2025

31.0.0 RC2 nextcloud/server#50538

Merged

1 task

blizzz modified the milestones: Nextcloud 31, Nextcloud 32 Jan 29, 2025

max-nextcloud requested changes Feb 3, 2025

View reviewed changes

max-nextcloud removed their assignment Feb 3, 2025

grnd-alt force-pushed the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch 2 times, most recently from d52b7f4 to e8281df Compare February 12, 2025 11:00

max-nextcloud reviewed Feb 12, 2025

View reviewed changes

src/tests/marks/Link.spec.js Outdated Show resolved Hide resolved

grnd-alt force-pushed the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch from e8281df to 3db7595 Compare February 13, 2025 09:15

grnd-alt requested a review from max-nextcloud February 13, 2025 09:16

grnd-alt force-pushed the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch from 3db7595 to a333bb3 Compare February 13, 2025 09:43

grnd-alt force-pushed the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch from a333bb3 to 1813f94 Compare March 7, 2025 09:36

grnd-alt moved this from 🏗️ In progress to 👀 In review in 📝 Office team Mar 12, 2025

max-nextcloud reviewed Mar 19, 2025

View reviewed changes

grnd-alt and others added 9 commits March 19, 2025 13:49

remove old link when editing

9fbd73b

Signed-off-by: grnd-alt <[email protected]>

refactor(links): add insertOrSetLink command

2b1ce9a

Signed-off-by: Max <[email protected]>

refactor(cy): move Links.spec.js into e2e folder

ed3a6d8

It is a proper e2e test driving the ui. Signed-off-by: Max <[email protected]>

test(cy): initial test for the link mark

3c7aee6

Signed-off-by: Max <[email protected]>

test(cy): adjust import path in moved Links.spec.js

6539ff7

Signed-off-by: Max <[email protected]>

refactor(link): use isMarkActive in insertOrSetLink

c1ef403

This way we do not hand in the active state and avoid inconsistencies. Signed-off-by: Max <[email protected]>

fix: replace link correctly

819ea95

Signed-off-by: grnd-alt <[email protected]>

fix: dont overwrite other link commands

0c3bcad

Signed-off-by: grnd-alt <[email protected]>

use snapshots for long test results

08a528b

Signed-off-by: grnd-alt <[email protected]>

grnd-alt force-pushed the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch from 139fce4 to 08a528b Compare March 19, 2025 12:50

max-nextcloud approved these changes Mar 19, 2025

View reviewed changes

grnd-alt merged commit 6e2892d into main Mar 20, 2025
63 of 64 checks passed

grnd-alt deleted the fix/5516-updating-the-url-of-a-link-via-toolbar-menu-splits-the-link branch March 20, 2025 07:57

github-project-automation bot moved this from 👀 In review to ☑️ Done in 📝 Office team Mar 20, 2025

skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

remove old link when editing #5690

remove old link when editing #5690

Uh oh!

grnd-alt commented Apr 15, 2024 •

edited by max-nextcloud

Loading

Uh oh!

max-nextcloud commented Apr 17, 2024

Uh oh!

max-nextcloud left a comment

Uh oh!

codecov bot commented Feb 12, 2025 •

edited

Loading

Uh oh!

max-nextcloud left a comment

Uh oh!

Uh oh!

juliusknorr commented Mar 7, 2025

Uh oh!

max-nextcloud left a comment

Uh oh!

max-nextcloud Mar 19, 2025

Uh oh!

max-nextcloud commented Mar 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

remove old link when editing #5690

remove old link when editing #5690

Uh oh!

Conversation

grnd-alt commented Apr 15, 2024 • edited by max-nextcloud Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

max-nextcloud commented Apr 17, 2024

Link in a paragraph

Link with custom text

Uh oh!

max-nextcloud left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

max-nextcloud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

juliusknorr commented Mar 7, 2025

Uh oh!

max-nextcloud left a comment

Choose a reason for hiding this comment

Uh oh!

max-nextcloud Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

max-nextcloud commented Mar 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

grnd-alt commented Apr 15, 2024 •

edited by max-nextcloud

Loading

codecov bot commented Feb 12, 2025 •

edited

Loading