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

Focus after select not focusing #11

Closed
Amerlander opened this issue Feb 21, 2021 · 7 comments · Fixed by #12
Closed

Focus after select not focusing #11

Amerlander opened this issue Feb 21, 2021 · 7 comments · Fixed by #12
Assignees
Labels
bug Something isn't working

Comments

@Amerlander
Copy link
Contributor

When I do focusAfterSelect='true' the search results stay open after an selection, but the input doesn't get focus. I have to select it with the mouse to change the text in it.
The console error says TypeError: searchRef.focus is not a function.

I wasnt able to get the input focused, but the error message can be removed by changing

    if (focusAfterSelect) searchRef.focus();

    hideDropdown = true;

to

        if (!focusAfterSelect)   hideDropdown = true;
@metonym
Copy link
Owner

metonym commented Feb 21, 2021

This one's on me. A breaking change in svelte-search was to remove the exported focus function in favor of binding to ref.

I think the fix would be to bind to the ref prop and perform a null check before focusing the input element.

<Search
    {...$$restProps}
-  bind:this={searchRef}
+  bind:ref={searchRef}

@metonym metonym self-assigned this Feb 21, 2021
@metonym metonym added the bug Something isn't working label Feb 21, 2021
metonym added a commit that referenced this issue Feb 21, 2021
@metonym metonym mentioned this issue Feb 21, 2021
metonym added a commit that referenced this issue Feb 21, 2021
* fix: bind input element reference correctly #11

Fixes #11

* fix: don't pass id to Search #10

Fixes #10
@Amerlander
Copy link
Contributor Author

Amerlander commented Feb 21, 2021

When focus after select enabled the hide dropdown should not be called:

if (focusAfterSelect) {
  searchRef.focus();
} else {
  hideDropdown = true;
}

hideDropdown = true;

Before the fix it wasn't called because the searchRef.focus() failed and stopped further execution.
Or was this a bug and the intention is to close the result list even when focus is keept?

@metonym
Copy link
Owner

metonym commented Feb 21, 2021

Yes, it's intentional to hide the dropdown after selecting or a result or blurring the input.

@Amerlander
Copy link
Contributor Author

But with getting the input focused again it should open again (or just not got closed).

Now when having focusAfterSelect='true' and selecting an item the input focuses, but the result list does not show up.
Clicking in the input again will not open the results what I would expect as a user, since the input is already focused.

@Amerlander
Copy link
Contributor Author

Maybe just changing the order would be the solution. Then all events will get called and the resultset will stay open as expected:

 hideDropdown = true;
if (focusAfterSelect)  searchRef.focus();

@metonym
Copy link
Owner

metonym commented Feb 21, 2021

from a UX perspective, Id prefer the dropdown to be closed after searching and selecting a result.

@Amerlander
Copy link
Contributor Author

Amerlander commented Feb 21, 2021

I totally agree when inputAfterSelect is set to clear or update - in the first case the result set will close anyways, in the other case the result set will be equal to the selected item and it doesn't make much sense to show it.

I understood Typeahead more as a search, than an input field, what was the reason for introducing keep for me.
In that combination it feels pretty much like a bug, when the results are closing and cant be opened by focusing the input.

Here is a sample with inputAfterSelect set to keepand focusAfterSelect to true:

select_closing.mp4

Compared to:

select_staying.mp4

The video is made on the code of this pr without the user styling applied: #13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants