Skip to content
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.

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

[Part 2] Port Auto link #2485

Merged
merged 7 commits into from
Mar 8, 2024
Merged

[Part 2] Port Auto link #2485

merged 7 commits into from
Mar 8, 2024

Conversation

juliaroldi
Copy link
Contributor

This change is part of the effort to port the Auto Link from the legacy ContentEdit feature into AutoFormatPlugin. After this change AutoFormatPlugin will automatically transform a typed url when space key is pressed after the text segment.
AutoLinkTyping

@juliaroldi juliaroldi requested a review from JiuqingSong March 7, 2024 20:14
@juliaroldi juliaroldi marked this pull request as ready for review March 7, 2024 20:14
Comment on lines 8 to 10
if (!unlink) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why need this parameter? If unlink is false, we should not call this function at all

Comment on lines 8 to 10
*/
export function createLinkAfterSpace(editor: IEditor, autoLink: boolean) {
if (!autoLink) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

@@ -5,7 +5,10 @@ import type { IEditor } from 'roosterjs-content-model-types';
/**
* @internal
*/
export function createLink(editor: IEditor) {
export function createLink(editor: IEditor, autoLink: boolean) {
if (!autoLink) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

triggerList(editor, model, listType, styleType, index);
rawEvent.preventDefault();
normalizeContentModel(model);
if (shouldSearchForBullet || shouldSearchForNumbering) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, check outside to reduce unnecessary parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already use these parameter on line 20 and 21, then we cannot remove them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I return this check to outside the function anyway?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. In that case we can keep it

triggerList(editor, model, listType, styleType, index);
rawEvent.preventDefault();
normalizeContentModel(model);
if (shouldSearchForBullet || shouldSearchForNumbering) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. In that case we can keep it

@juliaroldi juliaroldi merged commit 070d8d8 into master Mar 8, 2024
7 checks passed
@juliaroldi juliaroldi deleted the u/juliaroldi/auto-link-2 branch March 8, 2024 19:45
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.

2 participants