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

Fix when enter a URL in button block #6915

Closed
wants to merge 3 commits into from
Closed

Conversation

Rahmon
Copy link
Contributor

@Rahmon Rahmon commented May 23, 2018

Description

UrlInput component was selecting the first search result in Button Block (and probably in other blocks).

This happened because onKeyDown event at input just check if some suggestion was selected when Enter key is press.

screenshot from 2018-05-23 11-15-34

Consequently, how suggested posts are buttons inside a form, the onClick event of first button get fired and set the link.

screenshot from 2018-05-23 11-20-34

I also moved prependHTTP function from format-toolbar.js to url-input.js. Otherwise I need to repeat the same function in Button Block.

Closes: #6841

How has this been tested?

This has been tested with "npm test" and manually on Chrome

Types of changes

Bug fix

Checklist:

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

@brandonpayton
Copy link
Member

Hi @Rahmon, this seems like a reasonable fix but needs rebased. If you're up for that, I can review after.

@brandonpayton brandonpayton self-requested a review September 15, 2018 04:33
@Rahmon
Copy link
Contributor Author

Rahmon commented Sep 16, 2018

Hi @brandonpayton. I'll update this PR to check if this issue still happening.

@brandonpayton
Copy link
Member

Some additional info:
On further discussion, it would be good to get input from design for a full fix to #6841.

@gziolo
Copy link
Member

gziolo commented Feb 1, 2019

@designsimply is that still an issue? I see that all linked references are already closed.

@gziolo gziolo added the [Block] Buttons Affects the Buttons Block label Feb 1, 2019
@designsimply designsimply added the Needs Testing Needs further testing to be confirmed. label Feb 1, 2019
@designsimply
Copy link
Member

I re-tested and found that the problem raised in #6841 where you cannot hit enter to accept a domain added to the URL input in the link inserter for the button block is still a problem.

#6841 was closed to consolidate with #1838 which was a different issue requesting that validation and feedback be added for the URL input field and which was fixed by #11835 and two additional PRs were split out to address separate concerns about visual formatting for invalid links in #11793 and #11796.

I re-opened #6841 because it has not yet been addressed and added a note to say updating the button block link inserter to use the URLPopover as requested in #10128 should solve that problem automatically wince hitting enter to accept a link is already working in URLPopover from what I can see in testing.

In summary, I think we can close this PR in favor of updating to URLPopover in #10128 and also say thank you to @Rahmon for submitting a fix in the mean time! @Rahmon if you have any additional input or spot something I've missed, please let me know!

@Rahmon Rahmon deleted the fix/6841 branch February 1, 2019 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block Needs Testing Needs further testing to be confirmed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Button Block does not prefix a URL with http://
4 participants