Add URL preview to Link UI

[getdave]

Description

Builds on #18042. When using the LinkControl component to search for a URL, this PR utilises a new REST API endpoint to display the contents of the<title> tag of the remote site in the search results.

This is largely a POC and will need a lot of UX and Design thought (not to mention technical refinement) before it's ready to go.

This relies on #18042 being merged.

How has this been tested?

Manual testing.

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

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've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR. .

[getdave]

Apologies Core team. I submitted as a full PR when I meant to submit as a Draft!

[getdave]

@youknowriad I wonder would you have any time to advise me on the correct approach for data fetching in this PR? Mainly I”m not sure whether the data fetching logic should:

• Remain in the component and use apiFetch as it currently does
• Become part of the Block Editor Settings (similar to how __experimentalFetchLinkSuggestions() works )
• Get moved into the core/data store (similar to how the Embed data is managed).

[youknowriad]

It depends on the level of abstraction you want to give to this component.

so if the LinkControl is something WordPress independent that should stay in block-editor, it should be written in a way where it doesn't have any explicit dependency to the REST API. So yeah something like the editor settings should work. The API of this function should be clearly defined, args, returned value and its format. Third-party editors could implement it with their own APIs.

[getdave]

@youknowriad Thanks. That's really helpful perspective, especially regarding usage outside of WordPress (I need to remember that!).

I've opted to move this into the same place as the calls that go out to get the search suggestions. Is this what you were thinking?

I'm concerned because this isn't within the Block Editor package...

[shaunandrews]

Very small thing: There's a little extra space below the URL option and the bottom of the popover:

Seems like it should be consistent with the spacing on the left and right.

In the gif, the spinner and suggestions kick in a little too early; Its overly aggressive. Can we add a delay to the search before kicking off a request? Maybe wait for the user to stop typing for half a second?

[getdave]

@aduth id like to work on this Friday and Monday. If in light of recent conversations you have any time to review i think it might help.

[aduth]

On an initial impression, regarding the introduction of a new fetchRemoteURLTitle: Isn't this something we could expect to already be handled by the existing fetchLinkSuggestions, where each suggestion would include a title value? Based on how it's implemented here, I see it seems to exist largely to serve the purpose of manually-entered links, but I also wonder if this couldn't be built-in to fetchLinkSuggestions to handle.

I've not yet dug into the history surrounding the existing search endpoint, but the way it's modeled with properties like "type" and "subtype" made me wonder if it was always intended to accommodate external URLs. I could see that being applicable here, in that we wouldn't necessarily need a separate endpoint to retrieve URL details, if it was enough that the existing search endpoint could infer or provide a fallback for URLs which don't correspond to a known post on the site.

[getdave]

I also wonder if this couldn't be built-in to fetchLinkSuggestions to handle.

@aduth I'm not sure this will work. I could be wrong but I think the two concepts are different.

Currently, the search endpoint returns results for Page entities. If - as I think you're suggesting - we amend this to also return results from the entire web then the scope of the endpoint changes.

Let's say I call the endpoint with a term "About". How does the endpoint know whether to return Pages vs Web Search results? And in what order of priority? What about the speed of the search - how is this effected?

We could add an argument to explicitly tell the endpoint to trigger a "Web Search" but this feels like it's complicating the endpoint.

Note that our URL suggestions aren't in fact search results. Rather they are simply presentations of a manually entered link. We're just wanting to grab the title (and ultimately Favicon but not in this PR!) to provide some additional context around the link the user is creating. Therefore, to tie this concept up with a search endpoint feels like it could be mixing concerns.

Unless we're going to greatly expand the scope of the endpoint to include full web searches (not sure which API we'd use to do that because Google doesn't provide a "Search API") I think the current approach might be clearer.

As always, I'm open to being told I'm wrong!

[aduth]

Currently, the search endpoint returns results for Page entities. If - as I think you're suggesting - we amend this to also return results from the entire web then the scope of the endpoint changes.

After spending some more time looking through the history of the endpoint (Trac#39965, #6489), I believe it's entirely within the intended scope of the endpoint (already) to serve this purpose. The endpoint doesn't have any built-in opinions about what types of search results it produces. Even the posts/pages results are implemented as something of an extension intended to handle search requests which explicitly specify a type="post" parameter. So something like a "web search" implementation wouldn't be incompatible with the intended scope of this endpoint.

In more practical terms, I could imagine something where we...

• Implement a "URL" search handler which fetches the title and returns a search result which conforms to the existing search endpoint properties: id, url, title. The first two could be the same, if id is simply meant to serve as some unique identifier.
  • To your point about whether we'd handle any generic (word) terms like "About", I don't think we need to concern ourselves with this for the time being. I think it's enough to just yield no results if it's not something which can be handled (where the handling is currently limited to terms which are valid URLs). That being said, there's nothing to say we couldn't implement this in the future.
• Call the search endpoint with type="any" (instead of type="post") as an expression of the desire for each search handler to have a turn at trying to resolve the search results.
  • per_page might be interesting to consider here, since it's technically specific to the post search handler's implementation, though maybe it's enough that it simply be ignored by other search handlers.

We can also consider that some logic could happen in the client-side implementation of fetchLinkSuggestions, as long as the values it produces mimic the behavior we'd expect from the search endpoint. For example, we could change how we call the search endpoint depending on the search term (does it look like a URL?) or create objects client-side based on assumed results of a server request.

We're just wanting to grab the title (and ultimately Favicon but not in this PR!) to provide some additional context around the link the user is creating. Therefore, to tie this concept up with a search endpoint feels like it could be mixing concerns.

I disagree. I believe the endpoint was intentionally designed exactly to serve these purposes where we want to be able to represent common properties of resources associated with a particular search term, optionally filtered to types or subtypes (which could be "posts", or it could be "web search"). For the specific example of a favicon, it might be something where one of the following could be true instead:

• Any resource (even posts/pages within the current site) can have a favicon associated with it
• Or: We should revise our requirement as less specific to favicon, and more generalized to "a graphical (image) representation of a resource"

cc @felixarntz @danielbachhuber (who had a hand in the original implementation of this endpoint and might have thoughts)

[danielbachhuber]

I quickly skimmed the conversation but don't consider myself 100% up to speed.

From the screenshot in the description, the appears the goal of this pull request is to offer a preview of a single link. If this is the case, it's a bit different of a use case than the search endpoint was intended for. However, if you could expect to see multiple results for a partially completed link, then the search endpoint abstraction is intended to support the use case.

Also, I'd be a little bit concerned about using isURL( prependHTTP( value ) ) to determine whether the value needs a link preview. It looks like isURL() checks for http://. If you're always prepending http://, then ?

gutenberg/packages/url/src/is-url.js

Lines 1 to 17 in 9d4bbef

	const URL_REGEXP = /^(?:https?:)?\/\/\S+$/i;
	/**
	* Determines whether the given string looks like a URL.
	*
	* @param {string} url The string to scrutinise.
	*
	* @example
	* ```js
	* const isURL = isURL( 'https://wordpress.org' ); // true
	* ```
	*
	* @return {boolean} Whether or not it looks like a URL.
	*/
	export function isURL( url ) {
	return URL_REGEXP.test( url );
	}

Hope this helps.

[TimothyBJacobs]

Another thing I'd mentioned is that I wouldn't love the search endpoint having a side effect of performing an external HTTP request when using the search controller. That feels unexpected to me. I think the intention was to search all WordPress content, not the whole web.

Core class to search through all WordPress content via the REST API.

[aduth]

One of my motivations here is that if we have a need to be able to represent link suggestions associated with a search input as both posts on the current site and plain-URL web content, it would be convenient to have a single way to represent this. It appeared to me that the search endpoint was intentionally designed to be broad in the types of results it could report (via the type, subtype hierarchy). While at the time, the use-case might have been limited to only WordPress content, the new requirements seem to align reasonably well under this implementation, so I don't necessarily see it as being at odds with the original intended scope.

Alternatively, there's still the option that we use this object shape as the consistent representation, just client-side. Then the server-side behavior could be implemented however is most appropriate. This already occurs to an extent.

I wouldn't love the search endpoint having a side effect of performing an external HTTP request when using the search controller.

Maybe it's not the best argument, but: Is it conceptually much different that a search handler fetch search result details over HTTP vs. fetch search result details from the local site's database?

[TimothyBJacobs]

we have a need to be able to represent link suggestions associated with a search input as both posts on the current site and plain-URL web content,

Do we? I think they need to be a part of the same UI, but not at the same time if that makes sense? Ie, would we want posts to be returned that have https://wordpress.org In the post body if you are searching for https://wordpress.org? Not just the link preview?

If we would actually want that behavior, then I can see how having them in one endpoint would be helpful.

Is it conceptually much different that a search handler fetch search result details over HTTP vs. fetch search result details from the local site's database?

I think so, because it isn't fetching search result details, you have a URL already that you want to preview. You aren't finding that URL if that makes sense. In other words, if this was searching Google and returning websites that matched the entered search terms, it'd make sense for it to be a search handler to me.

There are also the performance characteristics.

that we use this object shape as the consistent representation, just client-side

Yeah, I think the shape here is essentially identical, and should be reused. But to me the semantics are different.

---

If we were to build this into the search endpoint, we would need to support any for the type argument, right now it is just for sub types. I think I'd also like to see a way to search across all types but excluding the "external" handler/.

[aduth]

Ie, would we want posts to be returned that have https://wordpress.org In the post body if you are searching for https://wordpress.org? Not just the link preview?

It's a good question, and I could see arguments for either. I guess the way I have been approaching this is more in the sense of the former, where even if it's something we could identify as being a single complete URL, we might still want to allow multiple results corresponding to that URL. And in that sense, I think we might both agree that the search endpoint could be appropriate for this?

But I also grant that it could make sense the other way as well: If I've entered a URL, maybe I don't need to consider that a "search", but rather that it's enough to get the details associated with that URL. (This is also what @danielbachhuber was mentioning in distinguishing on "preview of a single URL")

[TimothyBJacobs]

And in that sense, I think we might both agree that the search endpoint could be appropriate for this?

Yes, I think if that is behavior we want it makes much more sense. Though I think the multiple results is still an important distinction.

[getdave]

I'm picking this up again now and working on it.

[getdave]

As so much has changed I'm going to break this down into a set of smaller PRs which can be incrementally merged.

[draganescu]

Can this be closed?

[getdave]

Yes