ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
30 stars 8 forks source link

Combobox with autocomplete="both" filters out too many options #2347

Open jattasNI opened 1 month ago

jattasNI commented 1 month ago

πŸ› Bug Report

When the combobox is set to autocomplete="both", it is changing its value to equal the item that autocompletes. This causes other items that should match the entered text to be filtered out.

πŸ’» Repro or Code Sample

  1. In the Storybook combobox page, set autocomplete to both and choose "Many options"
  2. Type "S"

https://github.com/user-attachments/assets/9556d96e-ffd3-4f48-a41f-3e29d1a6ff1e

πŸ€” Expected Behavior

All of the "Sue..." items should be visible

😯 Current Behavior

Only the first matching item, "Sue (1)", is visible.

πŸ’ Possible Solution

@atmgrifter00 says

So I think I have an understanding of why the combobox behaves the way it does, and it basically involves us putting in some workarounds to make the combobox behave more like a text field, which FAST wasn't ready (or willing) to do. Unfortunately, we just had an oversight resulting in this bug, which the fix for is, annoyingly enough, likely going to involve some more state management for the filter text.

πŸ”¦ Context

This was originally reported as an SLE bug: Bug 2670859: Nimble combo box | Drop down does not filter the matched text correctly

That will likely be fixed by migrating to a filterable select. Another possible workaround is to use autocomplete="list".

🌍 Your Environment

Not environment-dependent

rajsite commented 1 month ago

My understanding of our intended direction is to align the filter-mode / autocomplete modes with the select. We shouldn't have an autocomplete mode both anymore, effectively just list with improved filtering that aligns with select.

Is there a specific use-case for autocomplete mode both or mode inline? Can we pre-emptively mark it deprecated and say only list should be used?

jattasNI commented 3 weeks ago

Agreement from the team that we can deprecate both. To fix this we should

  1. update the enum and docs
  2. update clients to move away from both.

1 is low priority backlog work, 2 is worth doing sooner

jattasNI commented 1 week ago

I started working on item 1 in #2376 and item 2 in this SLE PR. But I ran into a bug with the remaining autocomplete mode "list": #2378. Temporarily marking this blocked because I don't think we want to tell clients to migrate to "list" with that known bug still open.