fix(searchbox): handle Enter key in the input field to search + blur

[lorumic]

Done

• Make SearchBox trigger search on Enter keydown, and blur the input field.

QA

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

• Check SearchBox examples in Storybook
• Press "Enter" on the enabled examples and see that the input field blurs

Percy steps

This should not produce visual changes in Percy.

[webteam-app]

Demo

Jenkins

demos.haus

[bartaz]

When is this needed?

Search box includes a submit button to perform search. And it should be placed inside a form element that performs given search. In such case ENTER key is handled natively by the browser (it performs the submit of the form).

I guess in apps it may be more natural to use SearchBox component outside of the context of the form that does the search. Then maybe the form itself could be part of the component?

Because the proposed solution looks a bit like a workaround to something that could work natively.

[bartaz]

Now I noticed the difference. In Vanilla examples the p-search-box is on <form> element.
In React the root element of the SearchBox component is a <div>.

So a better solution would be to change the React component to use <form> instead and handle submit on it (that would cover both clicking on submit icon and enter key).
The downside (and I think that's why it was not done this way) is that if you want to use such component INSIDE another form it would be an issue to have it's own form built-in.

[lorumic]

This is needed every time we have a search box to filter the results in any table / list of results. No form in that case as the action (filtering) is performed onChange. So it's just a way to align the behaviour of the click on the magnifier icon and the Enter key press. I'll leave any further clarification to the requester of this change, @acarege

[bartaz]

I don't think this component was meant to do filtering at all.

Search & Filter component was meant to more advanced search and filter functionality.

[lorumic]

Maybe I didn't express myself properly. By "filtering" I mean setting a search query and showing only the results matching that query, as in the example below:

I don't think the SearchAndFilter is the right component to do this. Also, it doesn't have the magnifier icon, so it would not have the correct appearance.

[bartaz]

I guess we get to the point where it's more of a design UX question than implementation question.

So far the SearchBox component was meant for triggering search (when Enter/Search is hit). Not for automatic searching/filtering.
The SearchAndFilter component was meant for automatic searching and filtering (why it doesn't have a search icon is a question I don't have answer to).

If neither component does what you want to do, it's something that should be brought up in design (possibly Vanilla Working Group).

[lorumic]

SearchAndFilter seems an overkill to do what shown in the screen capture above. To be honest, I don't see which problem the changes in this PR create. It should just work fine in every case - both inside and outside a form.

[bartaz]

SearchAndFilter seems an overkill to do what shown in the screen capture above. To be honest, I don't see which problem the changes in this PR create. It should just work fine in every case - both inside and outside a form.

I agree that SearchAndFilter seems overkill, I just wanted to make sure it's clarified from design point of view, because there was some effort at some point to define how search/filter should work and which component does what, and by allowing some exceptions we may move away from consistency.

As for the functionality itself, it seems that the fact that there actually is onSearch handler on SearchBox component already is allowing it not only to "submit" search relying on external form. So I guess making sure ENTER triggers it as well is an improvement.
Would be good to verify how does that behave with form now. Would enter trigger the search AND submit the form? Should it cancel propagation (or default handler) so that form is not submitted if handler is triggered?

[lorumic]

Would be good to verify how does that behave with form now. Would enter trigger the search AND submit the form? Should it cancel propagation (or default handler) so that form is not submitted if handler is triggered?

Good point. I will test this specifically and get back to you with the result.

[lorumic]

@bartaz I tested the behaviour in Form component's Storybook. I added an onSubmit argument with a simple console.log to the args of the default example, then replaced the Input children with a SearchBox, and tried hitting Enter there to see if the onSubmit code got executed, but it didn't.

I'm not sure this is a good approach to test it, what do you think? Should I add an e.stopPropagation() to the keydown event handler for the Enter key just in case?

[bartaz]

Should I add an e.stopPropagation() to the keydown event handler for the Enter key just in case?

I'm afraid that adding that in by default will actually prevent ENTER from triggering submit of a form when it's needed.
So it's probably better as is.

The only problematic scenario may be if someone has both onSearch and a surrounding form to submit. But that would be a weird case already.

[bartaz]

LGTM from code point of view.

[bartaz]

I'm not 100% sure if blur by default is a desired behaviour (what if there is some validation and you should stay in the search to fix the value?). But I guess we can verify in practice if that creates any issues.

[lorumic]

I'm not 100% sure if blur by default is a desired behaviour (what if there is some validation and you should stay in the search to fix the value?). But I guess we can verify in practice if that creates any issues.

What about changing the condition to:

      if (e.key === "Enter" && internalRef.current.checkValidity()) {

Would that be a good solution?

[edlerd]

LGTM

[lorumic]

This should stop blur when the input contains an invalid value (#1065 (comment))

[github-actions]

🎉 This PR is included in version 0.51.5 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀