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

Async Combobox not rendering initial result #5234

Open
binaryartifex opened this issue Oct 10, 2023 · 18 comments
Open

Async Combobox not rendering initial result #5234

binaryartifex opened this issue Oct 10, 2023 · 18 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed rsp:ComboBox

Comments

@binaryartifex
Copy link

Provide a general summary of the issue here

combobox does not render a list of results from an initially empty or undefined list given a search query. however each subsequent query change will render a list.

🤔 Expected Behavior?

Combobox popover should render a list of results from an initially empty or undefined async response in the first instance.

😯 Current Behavior

At present, the current example provided by the docs for an async combobox does an initial fetch on page render. great for small lists, not so great for searching for an amazon product or returning an address from google places api. I added a small predicate in the useAsyncList list callback to only search when the filterText length is greater than 0. this takes care of the initial fetch however it appears the popover is now no longer synced with the state of the combobox because when there IS a list of results in the first instance, popover does not render a list. however with each new request after the first one that has results, it will flash the initial list before quickly showing the 'current' list.

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

Example can be found here
Its taken straight from the docs example under the RAC combobox async docs (an actual async RAC combobox code example would be nice though). all ive added is a console log to track the items passed to combobox and a predicate to prevent initial render.

  1. open link provided above, observe console log to see full initial list returned on page load.
  2. uncomment first line in useAsyncList 'load' callback to prevent initial fetch
  3. type in letter 'L' (lowercase) and watch for console output of returned list with no initial popover being rendered
  4. type in letter 'u' and observe the 'initial' results list immediately render until its replaced by the new list

Version

3.29.0 (react-aria)

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Windows 11

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

@oreqizer
Copy link

this exact same thing also happens when manually specifying list of items, both via the items prop on ComboBox, or listing out Item children manually down in ItemBox

@ldvilchez
Copy link

I was dealing with this issue last night, with a static list of items the popover opens up inmediality after you type, with an async list the behaviour is not consistent.

@binaryartifex
Copy link
Author

@snowystinger sorry for the poke mate but any chance of someone having a look at this?

@snowystinger
Copy link
Member

snowystinger commented Nov 14, 2023

No worries, sorry it slipped between the cracks.

As a workaround, you could pass allowsEmptyCollection: true to your combobox and render an empty state.

It's a bit weird that it's behind. I think we could fix it if we added a condition on which to open the combobox here

The comment at the beginning of the effect seems pretty clear what cases this covers. We'll need to make sure this doesn't cause it to open at some unexpected times.

if (
      isFocused &&
      (filteredCollection.size > 0 || allowsEmptyCollection) &&
      !triggerState.isOpen &&
      (inputValue !== lastValue &&
      menuTrigger !== 'manual') ||
      (filteredCollection !== lastCollection) // new one
    )

It'd be great to get an async RAC example in. We're open to contributions and it'd help cover this scenario. Would you be willing to contribute a PR?

I was dealing with this issue last night, with a static list of items the popover opens up inmediality after you type, with an async list the behaviour is not consistent.

can you clarify "not consistent"?

@snowystinger snowystinger added the bug Something isn't working label Nov 14, 2023
@ldvilchez
Copy link

ldvilchez commented Nov 14, 2023

I was dealing with this issue last night, with a static list of items the popover opens up inmediality after you type, with an async list the behaviour is not consistent.

can you clarify "not consistent"?

I'm using https://www.npmjs.com/package/use-places-Autocomplete the hook has a debounce prop and as soon as I removed the debounce it worked like a charm, at the cost of firing off a request for every keystroke which is obviously not ideal.

So I'm just setting the debounce to a number around 150ms which seems to result in a better experience but obviously not ideal either.

@LFDanLu
Copy link
Member

LFDanLu commented Nov 17, 2023

We can experiment with always updating the internal isOpen state in useComboBoxState and add a condition in the returned isOpen that checks if collection is empty or not as well.

@binaryartifex
Copy link
Author

@LFDanLu appreciate you guys are winding down after this last release, ive tried to get the RAC combobox straight from the asyncList example to behave more inline with the autocomplete pattern reminiscent of say a google search but again, it doesn't handle initial empty lists well at all, id go as far to say the fetch url used in the async example is a bit of a false positive as well because without any input it initially returns a full known list of items - not so good if your hitting an endpoint that returns 1000 items instead of 10 in the first instance before any characters are typed. Are you guys gonna be looking into a standalone Autocomplete RAC? in any case a solid example of using combobox in this manner or a searchbox/listbox example in the docs with all the combobox bells and whistles would be bloody awesome...

@LFDanLu
Copy link
Member

LFDanLu commented Dec 19, 2023

Our upcoming near term priority will be implementing a treeview but I will double check if autocomplete will be picked up soon. Theoretically changing the useComboBoxState hook to update an internal isOpen always should help with the behavior you were describing in the original issue and then we could update the example in the docs so that it doesn't fetch any items upfront.

EDIT: We will see if we can pick this up in our next sprint after the break.

@LFDanLu
Copy link
Member

LFDanLu commented Jan 9, 2024

Just chiming in to let you know that we've tried the fix I described above and it resulted in some additional complexities that were difficult to resolve. The team will hopefully continue digging into this in the coming sprint.

@LFDanLu
Copy link
Member

LFDanLu commented Jan 16, 2024

We'll be picking up ComboBox work as a part of a larger project effort soon, stay tuned. The team still needs to decide the form of the fix for this specific issue though, it may be something with the internal ComboBox hook logic or it may be exposing allowsEmptyCollection as suggested here.

@Camilo318
Copy link

The ComboBox menu opens when the user edits the input text. In my case, I'm fetching some data using the user input, since there's no initial data, the menu doesn't appear. Once the data is fetched and available, it is too late, the Combobox won't show anything up until the user types something again. In the next keystroke, the menu will be shown (old data from the previous fetch) but it will be inconsistent if the variable passed as items is undefined while it's fetching new data. So that's why it feels inconsistent. ComboBox needs to be aware of changes to the items props so it can show the menu when the fetching is done and we have new data ready to be shown. Hope that made sense, how I see it that's the cause of the issue

@LFDanLu
Copy link
Member

LFDanLu commented Jan 31, 2024

@Camilo318 That's correct, we currently only update the open state based on user action + the items available at that moment. We could perhaps make the isOpen state true when the collection goes from empty to having items but that will have then mesh with scenarios where allowsEmptyCollection is true and the user manually closes the dropdown before the items are loaded. I'm thinking that perhaps we refactor some of the allowsEmptyCollection logic in the hook so we ignore if there are items available or not when calculating isOpen internally and then return isOpen && (collection.items.length >0 || allowsEmptyCollection) as isOpen from the hook

@ashleyryan
Copy link

ashleyryan commented Apr 18, 2024

Is there any suggestions/recommendations for managing the open/close state as a workaround without allowsEmptyCollection (without going fully controlled)? Could we enable allowsEmptyCollection, but then override the open behavior ourselves when there are no results?

So far I've been telling consumers to put a loading spinner in the dropdown or a no results found message, depending on the state of useAsyncList, but neither of those apply if you're backspacing to clear out all of the results:

https://codesandbox.io/p/sandbox/vigilant-frost-8fz2ns?file=%2Fsrc%2FApp.tsx

@LFDanLu
Copy link
Member

LFDanLu commented Apr 18, 2024

Unfortunately, combobox doesn't have support for controlled open state at the moment (historically because we ran into many edge cases implementing it) so I don't think there is a easy way to handle this if you want to prevent the dropdown from opening only for specific cases when allowsEmptyCollection is enabled. Perhaps you could wrap/override the open state from useComboboxState if you drop down to the hook level for you implementation but not sure what edge cases may arise. For this case, we've either rendered a base list of results when the input is empty or a loading spinner/no results message. Ideally we'd implement something like #5234 (comment) internally in the hook, but haven't picked it up again after some initial investigation due to other priorities. Happy for others to take a stab at it though

@LFDanLu LFDanLu added the help wanted Extra attention is needed label Apr 18, 2024
@binaryartifex
Copy link
Author

honestly at this point i think the combo-box is not fit for purpose for this particular use case. if you've got a static list from a database like a list of countries or something to load in to prevent having a huge select field then great. for truly dynamic use cases that, well, everyone here are expecting, id be looking at a whole new component at the risk of adding ever more complexity to whats probably already a complex implementation behind the scenes...im a kid in a candy store waiting for the tree view stuff so i won't pressure the lads anymore than i have, but damn if this shouldn't be the very next thing to look at. theres a search field in the header of damn near every web app you come across these days.

@silviu-at
Copy link

silviu-at commented Apr 22, 2024

@binaryartifex I see your point, but it feels fairly reasonable for the users of this component to be able to specify that they want the ComboBox to open when new items are received. The items prop is part of ComboBox's core API and saying that you want the ComboBox to "react" based on changes to that prop is well within the scope of this component. However, I'm ignorant at the moment when it comes to the implementation details, so my answer is purely theoretical.

@ashleyryan
Copy link

ashleyryan commented May 13, 2024

I tried working around the issue by adding another check to the logic to render the popover or not. Instead of overlayState.isOpen, I have overlayState.isOpen && props.items.length > 0. But that throws an exception from a mutation observer that seems to be expecting the popover to be there, so now I'm officially stumped on how to work around this. I'm half tempted to put display: none on the popover when there are no results, but would almost certainly cause Accessibility issues

https://codesandbox.io/p/sandbox/hidden-dream-3mjnxm?file=%2Fsrc%2FAutocomplete.tsx%3A56%2C17

I can try and take a stab at the implementation mentioned above and see if I can contribute something

@ashleyryan
Copy link

I spent an hour or two looking at the hook, and slightly tweaked the logic when to open it, but it made about 6 other comboboxes fail, so this doesn't seem like it's going to work. I looked at the isOpen logic, but wasn't quite sure how to tweak it to make isOpen false without breaking other logic:

main...ashleyryan:react-spectrum:main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed rsp:ComboBox
Projects
Status: 🗑 Deferred
Development

No branches or pull requests

8 participants