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.

Sign up for GitHub

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

Jump to bottom

LinkControl - hitting enter triggers submit error in developer console #19634

Closed
getdave opened this issue Jan 14, 2020 · 1 comment
Closed

LinkControl - hitting enter triggers submit error in developer console #19634

getdave opened this issue Jan 14, 2020 · 1 comment
Assignees
@getdave
Labels
[Block] Navigation Affects the Navigation Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended

Comments

@getdave
Copy link
Contributor

getdave commented Jan 14, 2020 •
edited
Loading

Describe the bug
When adding a link using LinkControl (eg: buttons block or navigation block) then adding a link using the suggestion list the form that the submit button is in is unmounted when submission
occurs, resulting in a warning

'Form submission canceled because the form is not connected'

...in Chrome.

This causes an error in both real world usage and also causing e2e tests to fail (eg: the ones in #19458).

To reproduce
Steps to reproduce the behavior:

  1. Add buttons block
  2. Add a link for https://www.wordpress.org
  3. Hit [ENTER] without selecting an item
  4. See error in console (as above).

Expected behavior
The console should not warn. Ideally, the suggestions wouldn't be implemented using submit buttons.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context

  • Please add the version of Gutenberg you are using in the description.
  • To report a security issue, please visit the WordPress HackerOne program: https://hackerone.com/wordpress.
@getdave getdave added [Type] Bug An existing feature does not function as intended Good First Issue An issue that's suitable for someone looking to contribute for the first time [Block] Navigation Affects the Navigation Block labels Jan 14, 2020
@getdave getdave self-assigned this Jan 14, 2020
@getdave
Copy link
Contributor Author

getdave commented Jan 14, 2020

Whoops! Closing in favour of #19056

@getdave getdave closed this as completed Jan 14, 2020
getdave added a commit that referenced this issue Jan 15, 2020
@getdave
Fix e2e test failure due to console warning in LinkControl
b6d6c26 
This is a temp fix and I’ve raised an Issue to solve the core issue which will then make this fix redundant.

#19634
getdave added a commit that referenced this issue Jan 15, 2020
@getdave
Nav Block - show recent pages as default suggestions when creating Na…
b448074 
…v Links (#19458)

* Adds ability to manual trigger search data fetch

* Update to prop to experimental and rename to initialSuggestions

* Query for and display 5 most recently modified Pages

* Update initial suggestions to be display whenever the input is empty and there are no current suggestions

This is needed to ensure that the async fulfillment of the initialSuggestions prop causes the component to re-render and display the initial suggestions. Without this the initial suggestions would only be available if the prop is fufilled on the initial component mount.

* Fix to ensure initial suggestions are shown when pages request fulfills

Addresses issues noted in #19458 (comment)

* Updates to display 3 most recently modified pages instead of 5

Addresses #19458 (comment)

* Fix to allow keyboard arrow to move focus into suggestions when present

Previously it was not possible to use the down arrow key to move the focus into the search suggestions when initialSuggestions were displayed. This was due to requirement for there to be a value in the `input` which is now no longer valid now that initial suggestions can be displayed.

Note that removing the derived setting of selectedSuggestion state is valid as selectedSuggestions are always reset on each new data fetch anyway.

Resolves issue noted in #19458 (comment)

* Fix bug whereby not providing initialSuggestions to LinkControl disabled the ability to search at all

Test failures led to uncovering of a bug whereby the URLInput component’s `updateSuggestions` method would exit early it initialSuggestions wasn’t provided. As initialSuggestions is an optional prop this means that all fetching of search suggestions was disabled if the initialSuggestions wasn’t provided.

Fixed this bug and all tests by improving the conditionals.

* Adds tests to cover displaying initial suggestions

* Removes redundant const comment

* Test fix to e2e tests

* Try looser mock request matching to fix e2e test failures

* Try fix Travis e2e test failure by clearing request mocks

* Fix not awaiting mock setup

* Fix inadvertant revert of e2e test fix

* Refactor to remove seperate initialSuggestions query

As per the GH thread below, this refactors the code to avoid the need to introduce a new fetch API around `__experimentalInitialSuggestions`. Now instead we simply reuse the existing fetchSuggestions handler to get the initial results. The only different being we introduce an arguments object to queries to restrict the number of results displayed for initial Suggestions.

See #19458 (comment)

* Refactor to use empty value as condition for initialSuggestions request

Addresses #19458 (comment)

* Fix “suggestions” typo

* Update to reuse existing edit constant

* Fix initial suggestions showing when input has value

* Fix potential infinite render using flag to conditionalise updating suggestions on update or mount

* Add test to cover inifinite re-render loop.

* Fixes typo in WordPress ORG link in e2e tests

* Fix e2e test failure due to console warning in LinkControl

This is a temp fix and I’ve raised an Issue to solve the core issue which will then make this fix redundant.

#19634

* Revise snapshots

* Restore missing `fetchSearchSuggestions` prop following rebase

* Fix lint indentation error

* Rename prop

* Fix test broken due to prop rename
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@getdave getdave

Labels
[Block] Navigation Affects the Navigation Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@getdave