Fix navigation editor link search suggestions #29707
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
talldan
added
[Type] Regression
talldan
added
[Feature] Navigation Screen
and removed
[Block] Navigation
|
Size Change: +5 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
talldan
requested review from
adamziel,
ajitbohra,
draganescu,
kevin940726,
nerrad,
noisysocks,
ntwb and
tellthemachines
as code owners
draganescu
approved these changes
Mar 10, 2021
gwwar
approved these changes
Mar 10, 2021
There was a problem hiding this comment.
Thanks @talldan this tests well for me! This is good to land once the E2Es are fixed (or put into a follow up PR).
navigation-editor.mp4
Running npm run test-e2e -- --puppeteer-interactive packages/e2e-tests/specs/experiments/navigation-editor.test.js locally I see
● Navigation editor › displays suggestions when adding a link
TimeoutError: waiting for selector `button[aria-label="Add Link"]` failed: timeout 30000ms exceeded
349 |
350 | // Add an inner link block.
> 351 | const appender = await page.waitForSelector(
| ^
352 | 'button[aria-label="Add Link"]'
353 | );
354 | await appender.click();
| @@ -282,4 +328,60 @@ describe( 'Navigation editor', () => { | |||
| } ); | |||
| expect( submenuLinkHidden ).toBeDefined(); | |||
| } ); | |||
|
|
|||
| it( 'displays suggestions when adding a link', async () => { | |||
There was a problem hiding this comment.
👏 Thanks for adding an additional e2e test here
|
@gwwar Thanks for reviewing. Yep, I'm having trouble getting the new end to end test to pass. When running on CI, the Navigation Link block seems to have no variations. Clicking the appender inserts the block directly rather than opening the quick inserter menu. If you revert the changes in 88068a7 it should run well locally, but fails in CI. Slightly baffled by this 🤔 |
talldan
force-pushed
the
fix/navigation-editor-link-suggestions
branch
from
88068a7 to
e76f117
Compare
talldan
force-pushed
the
fix/navigation-editor-link-suggestions
branch
from
e76f117 to
b17a39e
Compare
|
I've updated the test to work for no variations, but this means it won't pass locally. Would be good to understand what's happening here. |
Curious 🤔 @talldan one thing I changed recently was moving the fallback hardcoded variations to the gutenberg/packages/block-library/src/navigation-link/hooks.js Lines 71 to 75 in 647a41c |
|
The bit that's unusual is that it works fine locally, the setup should be exactly the same. I thought it might be that one of the test plugins was somehow staying active, but I've tested those locally and couldn't see any issues. But it could still be another test leaving behind some stray configuration. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
#29012 caused a regression in the link suggestions for the navigation editor, resulting in some very long failing HTTP requests:
The line of code that cause the issue is:
https://github.com/WordPress/gutenberg/pull/29012/files#diff-8b871841ffe5cb86e63dd3e6df4e2230028ca02ff8ecebad37fac39cf5aa2b99R29-R30
This provided no way for the search text or search options to be passed into the
fetchLinkSuggestionsfunction. It's remedied in this PR.How has this been tested?
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: