Insert post title instead of URL, when adding a link to an existing post

[pwkip]

Fixes #11930
Make the link anchor also retrieve the title of the post/page

Description

• New behavior: If you insert a new link to an existing post via the inline-link modal, the post title is automatically inserted as link text.
• Previous behavior: If you insert a new link to an existing post via the inline-link modal, the actual URL is inserted as the link text.

How has this been tested?

Tested on local development server. Tested the following cases:

• insert new link with modal --> start typing --> select post from list. Result: Post title is inserted as link text
• select some text --> insert link with modal --> link remains unchanged
• insert new link with modal --> insert a link to an extarnal page --> The actual URL is inserted as link text

Screenshots

Types of changes

New feature

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation. (I think the code is obvious by itself)
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR.

[aduth]

This seems like a sensible change to me. I know there's been some history with the suggestion title and wanting to avoid some of its applications (#19735, #19739), but in terms of a default text for a new link, it seems like a good option to use as a default. My only concern would be if a manually entered URL (as in a user pasting a URL to the input field) would be selected and have an associated title, it's not clear to me if the user would expect the title to be used, or the URL. I don't know that it's currently the case that a title is assigned in these instances, however.

[aduth]

There's unfortunately not any useful unit tests associated with this code. However, there is a pretty exhaustive suite of end-to-end tests for links:

https://github.com/WordPress/gutenberg/blob/master/packages/e2e-tests/specs/editor/various/links.test.js

It would be good if we could account for this new behavior somewhere in those tests.

Depending on whether you're using the default development environment, it could be a little bit of extra work to get the end-to-end tests running in your environment:

https://github.com/WordPress/gutenberg/blob/master/docs/contributors/testing-overview.md#end-to-end-testing

https://github.com/WordPress/gutenberg/blob/master/packages/scripts/README.md#test-e2e

Is that something you might be interested in taking on? Let me know if I can help.

[getdave]

Thank for submitting this PR. It's very much appreciated 👍

I agree with @aduth that this seems like a sensible change. We should be aware of edge cases and try and include these in the e2e tests he references.

I did spot one bug. If you create some Pages with long titles then when the link is created only part of the title gets marked as the hyperlink. I'm not sure if that's a bug with this or a wider bug that exists in master.

[pwkip]

I did spot one bug. If you create some Pages with long titles then when the link is created only part of the title gets marked as the hyperlink. I'm not sure if that's a bug with this or a wider bug that exists in master.

@getdave Nice find. fixed it. Looks like the formatting was still applied to the length of the URL, I changed it to the length of the inner text now. Also did a quick find for other places where the newUrl was used. I think we're good now.

@aduth I'm using the default npm run dev command for the development server. I'll look into the links you shared and see if I can figure out what I need to do.

[pwkip]

@aduth I might need some help. All tests keep failing with this message:
["Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in %s.%s the componentWillUnmount method

I'll look into the error tomorrow. Probably something stupid. Maybe you know a quick fix?

[aduth]

@pwkip I believe this issue may be because the built copy of Gutenberg is generated from npm run dev, and because there are presently many warnings in development mode of the editor, the tests fail, because there are mechanisms in place which ensure that no console.error or console.warn is allowed to occur in the end-to-end test environment:

Related:

• Fixing up StrictMode errors in the e2e tests. #17355
• Testing: Use SCRIPT_DEBUG in end-to-end test environment #14845

• gutenberg/packages/e2e-tests/config/setup-test-framework.js

  Lines 117 to 206 in 981a45d

  	/**
  	* Adds a page event handler to emit uncaught exception to process if one of
  	* the observed console logging types is encountered.
  	*/
  	function observeConsoleLogging() {
  	page.on( 'console', ( message ) => {
  	const type = message.type();
  	if ( ! OBSERVED_CONSOLE_MESSAGE_TYPES.hasOwnProperty( type ) ) {
  	return;
  	}
  	let text = message.text();
  	// An exception is made for _blanket_ deprecation warnings: Those
  	// which log regardless of whether a deprecated feature is in use.
  	if ( text.includes( 'This is a global warning' ) ) {
  	return;
  	}
  	// A chrome advisory warning about SameSite cookies is informational
  	// about future changes, tracked separately for improvement in core.
  	//
  	// See: https://core.trac.wordpress.org/ticket/37000
  	// See: https://www.chromestatus.com/feature/5088147346030592
  	// See: https://www.chromestatus.com/feature/5633521622188032
  	if (
  	text.includes( 'A cookie associated with a cross-site resource' )
  	) {
  	return;
  	}
  	// Viewing posts on the front end can result in this error, which
  	// has nothing to do with Gutenberg.
  	if ( text.includes( 'net::ERR_UNKNOWN_URL_SCHEME' ) ) {
  	return;
  	}
  	// Network errors are ignored only if we are intentionally testing
  	// offline mode.
  	if (
  	text.includes( 'net::ERR_INTERNET_DISCONNECTED' ) &&
  	isOfflineMode()
  	) {
  	return;
  	}
  	// As of WordPress 5.3.2 in Chrome 79, navigating to the block editor
  	// (Posts > Add New) will display a console warning about
  	// non - unique IDs.
  	// See: https://core.trac.wordpress.org/ticket/23165
  	if ( text.includes( 'elements with non-unique id #_wpnonce' ) ) {
  	return;
  	}
  	// As of WordPress 5.3.2 in Chrome 79, navigating to the block editor
  	// (Posts > Add New) will display a console warning about
  	// non - unique IDs.
  	// See: https://core.trac.wordpress.org/ticket/23165
  	if ( text.includes( 'elements with non-unique id #_wpnonce' ) ) {
  	return;
  	}
  	const logFunction = OBSERVED_CONSOLE_MESSAGE_TYPES[ type ];
  	// As of Puppeteer 1.6.1, `message.text()` wrongly returns an object of
  	// type JSHandle for error logging, instead of the expected string.
  	//
  	// See: https://github.com/GoogleChrome/puppeteer/issues/3397
  	//
  	// The recommendation there to asynchronously resolve the error value
  	// upon a console event may be prone to a race condition with the test
  	// completion, leaving a possibility of an error not being surfaced
  	// correctly. Instead, the logic here synchronously inspects the
  	// internal object shape of the JSHandle to find the error text. If it
  	// cannot be found, the default text value is used instead.
  	text = get(
  	message.args(),
  	[ 0, '_remoteObject', 'description' ],
  	text
  	);
  	// Disable reason: We intentionally bubble up the console message
  	// which, unless the test explicitly anticipates the logging via
  	// @wordpress/jest-console matchers, will cause the intended test
  	// failure.
  	// eslint-disable-next-line no-console
  	console[ logFunction ]( text );
  	} );
  	}

Short-term, you can probably resolve this by running npm run build (the production build) before you run the end-to-end tests, or simply ignore those specific failures, since they'd not be considered by this Travis build anyways.

[pwkip]

@aduth I've created a new test 'will use Post title as link text if link to existing post is created without any text selected' and also added it to the pull request. Not 100% sure I did it right, would appreciate a quick review of that as well.

[aduth]

This looks and works great! Thanks, and congratulations on your first contribution! 🎉

[github-actions]

Congratulations on your first merged pull request! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

[pwkip]

@aduth Thanks for the follow up and all your helpful feedback! I learned a lot and hope to contribute more in the future. 👍

[getdave]

Congratulations on your PR acceptance @pwkip!