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

Inline Link: Set link text to post title when post selected from dropdown #12726

Closed

Conversation

ebinnion
Copy link

@ebinnion ebinnion commented Dec 9, 2018

Fixes #11930

Description

In #11930, it was requested that the link selector insert the post title as the link text instead of the link href as the link text.

This PR will ensure that when creating a new link, after searching for and selecting a post, that new link does use the post title. Updating a link does not change the link's text.

Design consideration: When updating an existing link by searching for a post, should the link text become the post title of the post? For example, let's say I search for "Same Page" and insert a link to that page. The link text would be "Sample Page". Now, let's say I edit that link, search for "Hello World!" and select the "Hello World!" post. Should the link text now be "Hello World!"?

How has this been tested?

I am on Mac and Chrome 70.0.3538.110 (Official Build) (64-bit). I tested by:

  • Inserting https://facebook.com into the new link UI
  • Searching for "Sample Page" and inserting a link via the new link UI
  • Updating an existing link and searching for the "Hello World!" post
  • Updating an existing link and setting it to https://facebook.com

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@earnjam earnjam added Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. labels Dec 9, 2018
@jameslesliemiller
Copy link

Is the link tool aware of whether the existing link is a link to a post?

If it is — and can compare the existing link text to the post title — then it makes sense to update the link text to the new post title where existing link text equals the post title.

If it is not aware, or is but cannot perform a comparison of the existing link text to the post title of the linked post, then it should not update the link text upon updating the href value. Otherwise, it may lead to overwriting descriptive link text or other undesirable changes.

@mapk
Copy link
Contributor

mapk commented Dec 31, 2018

I agree with @jameslesliemiller here. If it's a clear swap of one post & post title link with another, great. Otherwise refrain from it so we don't overwrite custom text the user may have intended to keep.

@mapk mapk removed Needs Design Needs design efforts. Needs Design Feedback Needs general design feedback. labels Dec 31, 2018
@aduth aduth added the [Package] Format library /packages/format-library label Jan 4, 2019
@gziolo gziolo requested a review from ellatrix February 6, 2019 11:34
@gziolo gziolo added this to the 5.1 (Gutenberg) milestone Feb 6, 2019
@gziolo gziolo requested a review from talldan February 6, 2019 11:36
@gziolo gziolo added the [Type] Enhancement A suggestion for improvement. label Feb 6, 2019
@youknowriad youknowriad removed this from the 5.3 (Gutenberg) milestone Mar 18, 2019
@adnan-cefalo
Copy link

@talldan, @ellatrix, Can you review it and remove the block?

@youknowriad
Copy link
Contributor

Hi There! I've triaging PRs today and it seems this one is stale and has some potential blockers to implement it. I'm going to close now but please consider reopening if these issues are solved. Thanks for your efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Format library /packages/format-library [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the link anchor also retrieve the title of the post/page
9 participants