[SuperEditor] Linkify on Enter (Resolves #1600)

[angelosilvestre]

[SuperEditor] Linkify on Enter. Resolves #1600

Currently, we only linkify an URL if the user types a space after it. So, typing an URL and pressing ENTER isn't linkifying the URL.

This PR changes the LinkifyReaction to look for SubmitParagraphIntention and SplitParagraphIntention, so it can linkify the URL on ENTER. As both SubmitParagraphIntention and SplitParagraphIntention don't have the node id, I had to look at the next change in the change list. We could also modify those objects to store the node id.

To also make it work for list items and tasks, this PR adds the SplitListItemIntention and SplitTaskIntention to follow the same pattern.

Gravacao.de.Tela.2023-11-22.as.23.03.38.mov

We have some duplication on the tests because we basically need to run the same test three times, with the only difference being the signal we use to trigger a new line:

1. Simulate an ENTER key press on a hardware keyboard.
2. Simulate a "\n" insertion delta.
3. Simulate a newline action.

[matthew-carroll]

@angelosilvestre can you think of an approach for this that doesn't require repeating the same thing for paragraphs, list items, and tasks? Based on the description of what you had to do, I'm getting the sense that we've made this goal too difficult with the existing approaches.

Perhaps we should run the linkify reaction on all TextNodes where a relevant event takes place? And perhaps we should inspect the change list on every selection change rather than on the intentions? I think there are actually three scenarios where we want to linkify:

• on space after a word
• on enter after a word
• on movement of the caret when it was previously sitting before, within, or just after a word.

Then, one caveat is that it needs to be possible to stop the editor from auto-linkifying a URL. This is similar to when a user composes a tag but doesn't actually want it to be a tag, we don't want to keep trying to turn it into a tag. In the case of tagging, I added an attribution which says the user doesn't want that text to be tagged. Perhaps something similar makes sense here.

What I'm thinking at the moment is that if we auto-linkify a URL, and the user then chooses to remove that link, instead of only removing the link attribution, we should replace the link attribution with some kind of do-not-linkify attribution so that we don't try again.

What do you think about these points?

[angelosilvestre]

Perhaps we should run the linkify reaction on all TextNodes where a relevant event takes place? And perhaps we should inspect the change list on every selection change rather than on the intentions?

Are you suggesting that we should only look for selection changes to apply the link attribution?

For the record, when pressing ENTER at the end of a paragraph, these are the reported changes:

List (6 items)
[0]: SubmitParagraphIntention
[1]: DocumentEdit (DocumentEdit -> NodeChangeEvent (1))
[2]: DocumentEdit (DocumentEdit -> NodeInsertedEvent (4bf6f342-65db-4ac0-a41d-3dca9ddf9b5e))
[3]: SelectionChangeEvent ([SelectionChangeEvent] - New selection: [DocumentSelection] - 
  base: ([DocumentPosition] - node: "4bf6f342-65db-4ac0-a41d-3dca9ddf9b5e", position: (TextPosition(offset: 0, affinity: TextAffinity.downstream))),
  extent: ([DocumentPosition] - node: "4bf6f342-65db-4ac0-a41d-3dca9ddf9b5e", position: (TextPosition(offset: 0, affinity: TextAffinity.downstream))), change type: SelectionChangeType.insertContent)
[4]: SubmitParagraphIntention
[5]: SelectionChangeEvent ([SelectionChangeEvent] - New selection: [DocumentSelection] - 
  base: ([DocumentPosition] - node: "4bf6f342-65db-4ac0-a41d-3dca9ddf9b5e", position: (TextPosition(offset: 0, affinity: TextAffinity.downstream))),
  extent: ([DocumentPosition] - node: "4bf6f342-65db-4ac0-a41d-3dca9ddf9b5e", position: (TextPosition(offset: 0, affinity: TextAffinity.downstream))), change type: SelectionChangeType.insertContent)

What I'm thinking at the moment is that if we auto-linkify a URL, and the user then chooses to remove that link, instead of only removing the link attribution, we should replace the link attribution with some kind of do-not-linkify attribution so that we don't try again.

If we linkify on selection changes, I think it makes sense to add another attribution to prevent the url to be linkified if the user doesn't want it.

If we want to keep using the intentions, we could add a marker Intention which SubmitParagraphIntention, SplitListItemIntention, and others could inherit from, so we just look for this marker Intention in the reaction.

[matthew-carroll]

@angelosilvestre in terms of deciding which direction to go with this, if the user types a URL, leaves the caret immediately after that URL, and then taps somewhere else, should we linkify that URL? I think the answer to that question is important.

[angelosilvestre]

@matthew-carroll As a user, I wouldn't expect that tapping somewhere else, or tapping at an url and then tapping somewhere else would turn the url into a link, but I'm not aware how apps like notion handle this.

[matthew-carroll]

@angelosilvestre can you do a survey of a few relevant apps to see what happens? This is the time to figure out the desired behavior, before we further lock-in to an approach that makes it harder to do that in the future.

[angelosilvestre]

@matthew-carroll I've done a quick research on some apps. Here are the findings:

Notion

[x] Space
[ ] Enter
[ ] Changing selection with arrow key
[ ] Typing and then tapping somewhere else
[ ] Tapping at an url and then tapping somewhere else

Apple Notes

[x] Space
[x] Enter
[ ] Changing selection with arrow key
[ ] Typing and then tapping somewhere else
[ ] Tapping at an url and then tapping somewhere else

Obsidian

As soon as I type "www.go" it already linkifies the text.

Google docs

[x] Space
[x] Enter
[ ] Changing selection with arrow key
[ ] Typing and then tapping somewhere else
[ ] Tapping at an url and then tapping somewhere else

Evernote

[x] Space
[x] Enter
[ ] Changing selection with arrow key
[ ] Typing and then tapping somewhere else
[ ] Tapping at an url and then tapping somewhere else

Microsoft Loop

[ ] Space
[ ] Enter
[ ] Changing selection with arrow key
[ ] Typing and then tapping somewhere else
[ ] Tapping at an url and then tapping somewhere else

I guess we should be fine supporting only enter and space to linkify.

[matthew-carroll]

I guess we should be fine supporting only enter and space to linkify.

Ok, we can continue in that direction. Based on that, do you want me to review this? Or do you have more changes?

[angelosilvestre]

@matthew-carroll You can proceed with a review, please.

[matthew-carroll]

LGTM

[matthew-carroll]

@angelosilvestre can you rebase to get most of the tests passing again before we merge?

[angelosilvestre]

@matthew-carroll Done.