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

Block Editor: LinkControl: Avoid lingering value properties from previous value #20020

Closed
wants to merge 2 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 4, 2020

Related: #19827

This pull request seeks to resolve an issue introduced by #19651, where if a link is assigned a value with an associated title (or any other supported value properties), they will be inherited into the next value when the user manually enters a URL. This ties in to related work at #19827, where ideally the suggestion value shapes are made to be consistent. As of #19651, the form submit handler will only assign the url property of the next value.

Even with separate improvements to make the suggestion value consistent, the changes here may still be valuable as an expression of intent of the next value to only derive settings properties from the previous values, and no other properties.

Testing Instructions:

Verify that a title does not transfer to a newly manually-entered value:

  1. Navigate to Posts > Add New
  2. Insert a Navigation block
  3. Click "Create empty"
  4. When prompted with the link input, search for a post within the current site, and select its suggestion entry
  5. Click "Link" in the block toolbar to edit the link
  6. Click "Edit" to edit the URL
  7. Now manually enter a URL, e.g. http://example.com
  8. Press Enter
  9. Note the "Title Attribute" field in the block inspector's SEO Settings is empty, and not the title from the link selected at step 4

@aduth aduth added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block labels Feb 4, 2020
@getdave
Copy link
Contributor

getdave commented Feb 17, 2020

I tried to test this following the testing instructions but the title attribute wasn't in the Block Inspector.

Also, I was struggling to follow the purpose here. If we create a link, and then decide to change it to something else I thought we'd want to keep any existing settings (eg: "new tab") in place.

For example, say I enter a link and then set it to open in a new tab, and then I realise I made a typo and meant https instead of http it would be pretty annoying to find my settings had been reset.

I'm not 100% sure it this is what you are intending or whether I've got completely the wrong end of the stick. I feel like I could be missing something here...

@getdave
Copy link
Contributor

getdave commented Feb 17, 2020

With #19775 merged we will still need to deal with this. Sorry it perpetuated the issue a little. I'm happy to help review if I can help.

Base automatically changed from master to trunk March 1, 2021 15:43
@jasmussen
Copy link
Contributor

Looking at this and trying to reproduce steps, if I understand it correctly this issue appears to have been resolved?

currently

@annezazu annezazu closed this Jul 27, 2022
@sirreal sirreal deleted the fix/link-control-pick-next-value-properties branch September 25, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants