Skip to content

fix: edit link will break the text styles #7863

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

fix: edit link will break the text styles #7863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea1e0f7
fix: edit link will break the text styles
asjqkkkk Apr 30, 2025
f75d061
Merge branch 'main' into fix/desktop/link
asjqkkkk Apr 30, 2025
4651316
fix: remove key error
asjqkkkk Apr 30, 2025
5709b94
Merge branch 'main' into fix/desktop/link
asjqkkkk May 12, 2025
14447f6
Merge branch 'main' into fix/desktop/link
asjqkkkk May 12, 2025
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
75 changes: 74 additions & 1 deletion frontend/appflowy_flutter/integration_test/desktop/document/document_toolbar_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,8 @@ void main() {
expect(getNodeText(node), link);
});

testWidgets('insert link and clear link name and remove link', (tester) async {
testWidgets('insert link and clear link name and remove link',
(tester) async {
const text = 'edit link', link = 'https://test.appflowy.cloud';
await prepareForToolbar(tester, text);

Expand Down Expand Up @@ -449,5 +450,77 @@ void main() {
expect(getNodeText(node), link);
expect(getLinkFromNode(node), null);
});

testWidgets('edit link text with style', (tester) async {
Attributes getAttribute(Node node, Selection selection) {
Attributes attributes = {};
final ops = node.delta?.whereType<TextInsert>() ?? [];
final startOffset = selection.start.offset;
var start = 0;
for (final op in ops) {
if (start > startOffset) break;
final length = op.length;
if (start + length > startOffset) {
attributes = op.attributes ?? {};
break;
}
start += length;
}

return attributes;
}

const text = 'edit text with style', link = 'https://test.appflowy.cloud';
await prepareForToolbar(tester, text);
final constSelection =
Selection.single(path: [0], startOffset: 0, endOffset: text.length);

final bold = find.byFlowySvg(FlowySvgs.toolbar_bold_m),
italic = find.byFlowySvg(FlowySvgs.toolbar_inline_italic_m),
underline = find.byFlowySvg(FlowySvgs.toolbar_underline_m);
await tester.tapButton(bold);
await tester.tapButton(italic);
await tester.tapButton(underline);

Node node = tester.editor.getNodeAtPath([0]);
Attributes attributes = getAttribute(node, constSelection);
expect(attributes, {
AppFlowyRichTextKeys.bold: true,
AppFlowyRichTextKeys.italic: true,
AppFlowyRichTextKeys.underline: true,
});

/// tap link button to show CreateLinkMenu
final linkButton = find.byFlowySvg(FlowySvgs.toolbar_link_m);
await tester.tapButton(linkButton);

/// search for page and select it
final textField = find.descendant(
of: find.byType(LinkCreateMenu),
matching: find.byType(TextFormField),
);
await tester.enterText(textField, gettingStarted);
await tester.pumpAndSettle();
await tester.simulateKeyEvent(LogicalKeyboardKey.enter);
await tester.simulateKeyEvent(LogicalKeyboardKey.escape);

node = tester.editor.getNodeAtPath([0]);
attributes = getAttribute(node, constSelection);
expect(isPageLink(node), true);
expect(getLinkFromNode(node) == link, false);
expect(attributes[AppFlowyRichTextKeys.bold], true);
expect(attributes[AppFlowyRichTextKeys.italic], true);
expect(attributes[AppFlowyRichTextKeys.underline], true);

/// remove link
await tester.hoverOnWidget(find.byType(LinkHoverTrigger));
await tester.tapButton(find.byFlowySvg(FlowySvgs.toolbar_link_unlink_m));
node = tester.editor.getNodeAtPath([0]);
attributes = getAttribute(node, constSelection);
expect(getLinkFromNode(node) == link, false);
expect(attributes[AppFlowyRichTextKeys.bold], true);
expect(attributes[AppFlowyRichTextKeys.italic], true);
expect(attributes[AppFlowyRichTextKeys.underline], true);
});
});
}
53 changes: 43 additions & 10 deletions ...lib/plugins/document/presentation/editor_plugins/desktop_toolbar/link/link_extension.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,66 @@ extension LinkExtension on EditorState {
if (node == null) {
return;
}
final attributes = _getAttribute(node, selection);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we're not getting all the attributes from the selection. For example:

Before:

http://appflowy.io <- (add one more o here)

After:

http://appflowy.ioo The result should be ioo without extra formatting.

We have a slice attributes function that gets the attributes based on the index. appflowyEditorSliceAttributes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, the logic about getting Attributes is not correct, I need to adjust it

attributes[BuiltInAttributeKey.href] = null;
attributes[kIsPageLink] = null;
final index = selection.normalized.startIndex;
final length = selection.length;
final transaction = this.transaction
..formatText(
node,
index,
length,
{
BuiltInAttributeKey.href: null,
kIsPageLink: null,
},
);
..formatText(node, index, length, attributes);
apply(transaction);
}

void applyLink(Selection selection, LinkInfo info) {
final node = getNodeAtPath(selection.start.path);
if (node == null) return;
final transaction = this.transaction;
final attributes = _getAttribute(node, selection);
attributes.addAll(info.toAttribute());
final linkName = info.name.isEmpty ? info.link : info.name;
transaction.replaceText(
node,
selection.startIndex,
selection.length,
linkName,
attributes: info.toAttribute(),
attributes: attributes,
);
apply(transaction);
}

void removeAndReplaceLink(
Selection selection,
String text,
) {
final node = getNodeAtPath(selection.end.path);
if (node == null) {
return;
}
final attributes = _getAttribute(node, selection);
attributes[BuiltInAttributeKey.href] = null;
attributes[kIsPageLink] = null;
final index = selection.normalized.startIndex;
final length = selection.length;
final transaction = this.transaction
..replaceText(node, index, length, text, attributes: attributes);
apply(transaction);
}

Attributes _getAttribute(Node node, Selection selection) {
Attributes attributes = {};
final ops = node.delta?.whereType<TextInsert>() ?? [];
final startOffset = selection.start.offset;
var start = 0;
for (final op in ops) {
if (start > startOffset) break;
final length = op.length;
if (start + length > startOffset) {
attributes = op.attributes ?? {};
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Consider cloning the attributes to avoid unintended mutation.

Since _getAttribute returns op.attributes by reference, removing keys will mutate shared state. Clone the map first (e.g., with Map.from) before modifying.

Suggested change
attributes = op.attributes ?? {};
attributes = op.attributes != null ? Map.from(op.attributes) : {};

break;
}
start += length;
}

return attributes;
}
}
27 changes: 1 addition & 26 deletions ...ib/plugins/document/presentation/editor_plugins/desktop_toolbar/link/link_hover_menu.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class _LinkHoverTriggerState extends State<LinkHoverTrigger> {
onRemoveLink: (linkinfo) {
final replaceText =
linkinfo.name.isEmpty ? linkinfo.link : linkinfo.name;
onRemoveAndReplaceLink(editorState, selection, replaceText);
editorState.removeAndReplaceLink(selection, replaceText);
},
),
child: child,
Expand Down Expand Up @@ -269,31 +269,6 @@ class _LinkHoverTriggerState extends State<LinkHoverTrigger> {
);
}
}

void onRemoveAndReplaceLink(
EditorState editorState,
Selection selection,
String text,
) {
final node = editorState.getNodeAtPath(selection.end.path);
if (node == null) {
return;
}
final index = selection.normalized.startIndex;
final length = selection.length;
final transaction = editorState.transaction
..replaceText(
node,
index,
length,
text,
attributes: {
BuiltInAttributeKey.href: null,
kIsPageLink: null,
},
);
editorState.apply(transaction);
}
}

class LinkHoverMenu extends StatefulWidget {
Expand Down
Loading