Trim input value in navigation search input field

[sainthkh]

• Closes LinkControl - for search term and suggested URL, input value should be passed trimmed of whitespace. #19631

Description

Trim input values in navigation search input field.

How has this been tested?

Added related e2e code.

Screenshots

Before:

After:

NOTE: It doesn't show url item at the bottom because page is not a valid url.

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.
• [NA] I've included developer documentation if appropriate.
• [NA] I've updated all React Native files affected by any refactorings/renamings in this PR. .

I'm sorry @WunderBart. I realized that I intercepted your issue after finished implementing it.

[getdave]

Thank you for the PR. A nice fix!

It should be ok to remove whitespace from start/end of string. Would you consider adding a unit test in LinkControl to cover this?

[sainthkh]

@getdave

1. Removed e2e test because it doesn't work on CI.
2. Added unit test.

[getdave]

Thank you so much for submitting this. I'd love to see this get merged.

Currently, however, the implementation doesn't correctly ensure the handler which fetches search results over XHR has its search term trimmed. Moreover, the tests give you a false positive. You probably know this already but I felt it was worth pointing out.

I've left a comment with a suggestion on amending both tests and implementation detail to get this working for you.

I'd recommend:

• Rebasing against upstream master to bring in the latest changes.
• Amending the tests as I suggest (assuming you agree).
• Checking tests correctly confirm the implementation fails (ie: tests fail).
• Amending the implementation.
• Checking tests now pass.

I hope this helps.

[sainthkh]

@getdave thanks for the amazing review.

The goal of submitting PR is to help you save time and do other more valuable things, but I failed to do that. It's a bit embarrassing.

[sainthkh]

Fixed merge conflict.

[getdave]

Fixed merge conflict.

Looking better.

I'm afraid I merged a rather large commit yesterday on <LinkControl> so it looks like you have another small merge issue. The component should be stable for a while so now is a good time to do it.

[sainthkh]

Conflicts are resolved in my local branch.

I'm waiting for your opinion about adding trim() to TextHighlight.

[sainthkh]

• Fixed conflicts.
• Moved trim to TextHighlight

[getdave]

Thanks for your work on this @sainthkh 👍

[WunderBart]

If the updateSuggestions() func is expected to always be called with trimmed value, it's probably better to do the trimming inside that very function. Not a blocker though, just a suggestion - feel free to 🚢 this one. Nice work!

[sainthkh]

IMHO, it's only duplicated once now. When we need trimming once more, it's strikeout and let's refactor.

I cannot think of a case that doesn't need trimming. But we don't know the future.

[getdave]

@sainthkh 👋 Are you still intending to merge this?

[sainthkh]

@getdave Yes. Or is there any problem?