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

Automatically create a link when selected text is a URL #9067

Merged
merged 3 commits into from
Aug 22, 2018

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Aug 17, 2018

Closes #3513.

When a URL is selected and the Link button is clicked or the Link keyboard shortcut is pressed, a link with that URL set as its href should be instantly created.

auto-link

As part of this, I pulled an existing link regular expression out into its own isURL function that lives in the @wordpress/url package. Not sure if it's actually a useful helper function, though—happy to go back on this decision.

To test:

npm run test-e2e

Or, for those of us who distrust automation:

  1. Highlight a URL
  2. Click Link or press Cmd+K
  3. The selected URL should immediately become a link

@noisysocks noisysocks added [Type] Enhancement A suggestion for improvement. [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable labels Aug 17, 2018
];

forEach( urls, ( url ) => {
expect( isURL( url ) ).toBe( true );
Copy link
Member

Choose a reason for hiding this comment

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

equivalent to: expect( every( urls, isURL ) ).toBe( true );

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 0d218ad

*
* @return {boolean} Whether or not it looks like a URL.
*/
export function isURL( url ) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice util, I think we may be able to use it in:

if ( linkRegExp.test( pastedText ) ) {

Removing a duplicate regex.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've already replaced that regex 🙂

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Nice feature 👍
It is working well I think we have a small bug that we should address before the merge. After we add a link using this method if we click right away on the edit button nothing happens the edit button does not work. If we select other block and then come back and press edit things work well.

@noisysocks
Copy link
Member Author

Good catch @jorgefilipecosta! I've fixed this in 50c9c01.

@noisysocks noisysocks force-pushed the fix/disappearing-link-selection branch from 543da2f to 47d0bd8 Compare August 20, 2018 23:25
@noisysocks noisysocks force-pushed the add/auto-link-selected-urls branch 2 times, most recently from 0f90d62 to beee381 Compare August 21, 2018 03:42
@noisysocks noisysocks changed the base branch from fix/disappearing-link-selection to master August 21, 2018 03:43
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

It is working well in my tests 👍

When a URL is selected and the Link button is clicked or the Link
keyboard shortcut is pressed, a link with that URL set as its href
should be instantly created.
Fixes a bug that prevented the user from selecting a link which has text
that looks like a URL and clicking Edit. We accomplish this by
distinguishing between editing a link (isEditingLink) and adding a new
link (isAddingLink).
@mtias
Copy link
Member

mtias commented Aug 30, 2018

Thanks for this improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants