WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.54k stars 4.21k forks source link

Navigation Link URL setting in Inspector Controls does not allow you to delete last character #60758

Open fabiankaegy opened 7 months ago

fabiankaegy commented 7 months ago

Description

The URL setting in the naviation link blocks settings sidebar does not allow you to remove the final character. So if you for example have an anchor link in there you are not able to delete the # from the URL.

Step-by-step reproduction instructions

  1. Go to the site editor
  2. Select a navigation menu
  3. Add a new custom link menu item
  4. Open the settings sidebar
  5. Try to remove the URL via the text field in the sidebar

You should be able to remove everything including the last character

Screenshots, screen recording, code snippet

https://github.com/WordPress/gutenberg/assets/20684594/629453a1-7cee-4e72-bdb6-1123dfde88b8

Environment info

Also:

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

fabiankaegy commented 7 months ago

I believe the issue may be this check here:

https://github.com/WordPress/gutenberg/blob/0469173a4876818ce5a47e7bf8d05d9d61a5a34e/packages/block-library/src/navigation-link/update-attributes.js#L91

It checks that the URL is truthy via the && check. Which would fail for empty strings.

Mamaduka commented 7 months ago

When I update the logic and allow setting an empty URL value, this is what happens: the URL popover opens and steals the focus.

Screenshot

https://github.com/WordPress/gutenberg/assets/240569/378cd0cb-78fb-4293-961e-8c35cb36d839

Mamaduka commented 7 months ago

It looks like the issue might have been introduced via #49417.

@scruffian, I know it's super old PR, but maybe you can remember why this change was needed.

P.S. The issue affects older versions of WP if we don't meet the timeline for the next minor release. It should be fine to bump this until the next major.

Mamaduka commented 6 months ago

I've yet to test this, but a proposed fix in #61374 might help us fix this issue.

scruffian commented 5 months ago

@scruffian, I know it's super old PR, but maybe you can remember why this change was needed.

Sorry I can't remember!

getdave commented 4 days ago

This is still a problem. When you correct the logic the rich text of the label in the block canvas steals the focus.