-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[lexical-link] Bug Fix: Enable autolink matching when it unlinked #8165
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
Open
levensta
wants to merge
6
commits into
facebook:main
Choose a base branch
from
levensta:check-valid-unlinked-autolink
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+203
−3
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b0d32d4
enable autolink matching when it unlinked
levensta a5a7b75
one more test
levensta 8455fae
try to fix legacy-events windows test
levensta 475c316
Merge branch 'main' into check-valid-unlinked-autolink
levensta ad35986
skip test in legacy_events mode
levensta aad3c2d
Merge branch 'main' into check-valid-unlinked-autolink
levensta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import { | |
| focusEditor, | ||
| html, | ||
| initialize, | ||
| LEGACY_EVENTS, | ||
| pasteFromClipboard, | ||
| pressInsertLinkButton, | ||
| test, | ||
|
|
@@ -734,8 +735,7 @@ test.describe.parallel('Auto Links', () => { | |
| ); | ||
|
|
||
| await click(page, 'span:has-text("http://www.example.com")'); | ||
|
|
||
| pressInsertLinkButton(page); | ||
| await pressInsertLinkButton(page); | ||
|
|
||
| await assertHTML( | ||
| page, | ||
|
|
@@ -753,6 +753,206 @@ test.describe.parallel('Auto Links', () => { | |
| ); | ||
| }); | ||
|
|
||
| test('Unlinked the autolink should not destruct if add non-spacing text in front or right after it', async ({ | ||
| page, | ||
| isPlainText, | ||
| }) => { | ||
| test.skip(isPlainText || LEGACY_EVENTS); | ||
|
Comment on lines
+756
to
+760
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a result, the current changes in this PR do not bring anything new to this scenario. It already fails in prod in legacy_events mode, so I added a condition to skip the test #8165 (comment) Tests directly related to the new behavior are written below. |
||
|
|
||
| await focusEditor(page); | ||
| await page.keyboard.type('http://example.com'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <a href="http://example.com"> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </a> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| await focusEditor(page); | ||
| await click(page, 'a[href="http://example.com"]'); | ||
| await click(page, 'div.link-editor div.link-trash'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| // Add non-url text after the link | ||
| await moveToLineEnd(page); | ||
| await page.keyboard.type('!'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </span> | ||
| <span data-lexical-text="true">!</span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| await page.keyboard.press('Backspace'); | ||
|
|
||
| // Add non-url text before the link | ||
| await moveToLineBeginning(page); | ||
| await page.keyboard.type('!'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span data-lexical-text="true">!</span> | ||
| <span> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
| await page.keyboard.press('Backspace'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
| }); | ||
|
|
||
| test('Can destruct unlinked the autolink if add an invalid character inside', async ({ | ||
| page, | ||
| isPlainText, | ||
| }) => { | ||
| test.skip(isPlainText); | ||
|
|
||
| await focusEditor(page); | ||
| await page.keyboard.type('http://example.com'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <a href="http://example.com"> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </a> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| await focusEditor(page); | ||
| await click(page, 'a[href="http://example.com"]'); | ||
| await click(page, 'div.link-editor div.link-trash'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| // break autolink | ||
| await moveToLineEnd(page); | ||
| await page.keyboard.press('ArrowLeft'); | ||
| await page.keyboard.type('['); | ||
| // plain text without wrapper | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span data-lexical-text="true">http://example.co[m</span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
| }); | ||
|
|
||
| test('Can destruct unlinked the autolink if add emoji inside', async ({ | ||
| page, | ||
| isPlainText, | ||
| }) => { | ||
| test.skip(isPlainText); | ||
|
|
||
| await focusEditor(page); | ||
| await page.keyboard.type('http://example.com'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <a href="http://example.com"> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </a> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| await focusEditor(page); | ||
| await click(page, 'a[href="http://example.com"]'); | ||
| await click(page, 'div.link-editor div.link-trash'); | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span> | ||
| <span data-lexical-text="true">http://example.com</span> | ||
| </span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
|
|
||
| // type emoji | ||
| await moveToLineEnd(page); | ||
| await page.keyboard.press('ArrowLeft'); | ||
| await page.keyboard.type(':)'); | ||
| // ':', ')' — is valid chars for link but inserting an emoji | ||
| // should break the link by splitting it into two text nodes | ||
| await assertHTML( | ||
| page, | ||
| html` | ||
| <p dir="auto"> | ||
| <span data-lexical-text="true">http://example.co</span> | ||
| <span class="emoji happysmile" data-lexical-text="true"> | ||
| <span class="emoji-inner">🙂</span> | ||
| </span> | ||
| <span data-lexical-text="true">m</span> | ||
| </p> | ||
| `, | ||
| undefined, | ||
| {ignoreClasses: true}, | ||
| ); | ||
| }); | ||
|
|
||
| test('Pressing Enter inside an AutoLinkNode does not insert extra paragraph', async ({ | ||
| page, | ||
| isPlainText, | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.