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

Cannot programmatically open detached autocomplete if openOnFocus is false #843

Open
cmfcmf opened this issue Dec 13, 2021 · 12 comments
Open

Comments

@cmfcmf
Copy link

cmfcmf commented Dec 13, 2021

Description

The autocomplete does not open when calling api.setIsOpen(true) if detachedMediaQuery = "" and openOnFocus = false.

Reproduction

https://codesandbox.io/s/magical-tree-499cg?file=/app.tsx

Steps

  1. Go to the codesandbox linked above
  2. Click on the 'open autocomplete' button I added (around line 130 in app.tsx)
  3. The autocomplete will flicker open for a split second, and then close.

Expected behavior

The autocomplete should open when calling api.setIsOpen(true);.

Environment

  • OS: Windows
  • Browser: Chrome
  • Autocomplete version: 1.5.1
@Haroenv
Copy link
Contributor

Haroenv commented Dec 14, 2021

In DocSearch the modal renders only when it's explicitly opened:

https://github.com/algolia/docsearch/blob/10c5b2ea145c633b8bc5d0b3686458b08ea0b80d/packages/docsearch-react/src/useDocSearchKeyboardEvents.ts#L42

https://github.com/algolia/docsearch/blob/80677fd7d36a85b6a205383a7ca3d68969fbe3bf/packages/docsearch-react/src/DocSearch.tsx#L70-L98

I'm not saying that's per se the best solution, and what you're describing here does seem like a bug, I'd just like to offer a different workflow that avoids this issue

@sarahdayan
Copy link
Member

It does look like a bug. The problem persists in 1.5.0 so it doesn't look like a regression introduced in 1.5.1.

I will investigate.

@sarahdayan
Copy link
Member

sarahdayan commented Jan 31, 2022

I've looked into it, this is indeed a bug. Here's what happens:

  • When calling setIsOpen(true), the next state is computed with isOpen=true, which opens the search modal.
  • When the modal opens, the search input inside it gets automatically focused (internal behavior set here).
  • This focus event triggers a call to onFocus, which triggers a state update. Because of this condition, the next isOpen state is false when openOnFocus=false.

This is why you might briefly see the modal opening then closing—the modal is opening as requested, but the side-effects of focusing the input closes it. You can observe a similar behavior on mobile when openOnFocus=false and detached mode isn't disabled.

The behavior of the state reducer is likely correct, this issue should be fixed in at the detached mode level, in autocomplete-js.

Note that "just" removing this internal focus behavior, or making it depend on autoFocus isn't the answer: even without focusing on open, clicking the input after the modal has opened would trigger the same behavior. We might want to override the flow when we're in detached mode only.

@sarahdayan
Copy link
Member

@Haroenv The DocSearch example uses autocomplete-core and fully controls the UI. In this case, autocomplete-js is at fault and isn't user-controlled, so I'm afraid this can't be circumvented that way.

@cmfcmf What's your use case for setting openOnFocus to false? Are you trying to avoid an empty query? There might be a way to work around the problem until we get this fixed.

For example, if the goal is to avoid an empty query, you can add a check before returning sources:

https://codesandbox.io/s/charming-jasper-4o1ix?file=/app.tsx:993-1049

@domstubbs
Copy link

I’ve just run into this. In my case I’m aiming not to have an inline search field, just a button that opens up the detached search view with the field selected (on desktop and mobile). It’s not a huge problem to enable openOnFocus but it does mean that on init the search UI includes some messy white space where the empty panel sits.

openOnFocus: true

open-focus-true

openOnFocus: false

open-focus-false

@sarahdayan
Copy link
Member

@domstubbs Yes, this is a problem because in this scenario, setIsOpen opens both the modal and the panel. You can fix this in the meantime with some CSS:

.aa-PanelLayout:empty {
  display: none;
}

@domstubbs
Copy link

@sarahdayan I did implement a CSS workaround but yours is neater. Thanks!

@cmfcmf
Copy link
Author

cmfcmf commented Feb 19, 2022

@cmfcmf What's your use case for setting openOnFocus to false? Are you trying to avoid an empty query? There might be a way to work around the problem until we get this fixed.

I didn't remember initially, but I think it was mainly for the reason @domstubbs brought up.

@atanasStoyanov
Copy link

I believe we have a similar issue and @sarahdayan we would appreciate your advice on it!
We have a button which opens the detached modal by using setIsOpen(true) and openOnFocus is set to true, but when the user clicks on the input to open the keyboard the detached modal is being closed. So I assume that the state is being refreshed and isOpen is evaluated to false again. The issue happens only on iOS.
Could you please advise is there any workaround about this problem? Thanks!

@sarahdayan
Copy link
Member

Hey @atanasStoyanov, could you share a minimal reproduction so we can debug?

You can use this starter template to help you get started.

@atanasStoyanov
Copy link

Hey @sarahdayan thanks for your willingness to help. We found that the issue comes from our implementation. Long story short the component has been re-rendered too many times because of scroll event listener and that was causing the detached modal being re-rendered as well and respectively closed. Once we prevented those re-renders it got fixed and now works properly. Thank you!

@vrilcode
Copy link

vrilcode commented Oct 6, 2022

@domstubbs Yes, this is a problem because in this scenario, setIsOpen opens both the modal and the panel. You can fix this in the meantime with some CSS:

.aa-PanelLayout:empty {
  display: none;
}

What's a CSS solution, if I have and need noResults template? It's showing up initially and I can't use :empty in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants