-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[Autocomplete] Filter based on visible option labels by default #19798
Comments
@gforrest-bw Thanks for opening an issue. You seemed to have identified a great improvement opportunity. We have had a couple of iteration on the stringify method, and it's still a partial solution. However, I think that using What do you think of this diff? Do you want to work on a pull request? :) It seems that we will save a bit of bundle size too 👌 diff --git a/docs/src/pages/components/autocomplete/autocomplete.md b/docs/src/pages/components/autocomplete/autocomplete.md
index 6ec7d561c..19c9350b6 100644
--- a/docs/src/pages/components/autocomplete/autocomplete.md
+++ b/docs/src/pages/components/autocomplete/autocomplete.md
@@ -137,16 +137,24 @@ You can use it to change the default option filter behavior.
import { createFilterOptions } from '@material-ui/lab/Autocomplete';
```
-It supports the following options:
+### `createFilterOptions(config) => filterOptions`
+
+#### Arguments
1. `config` (*Object* [optional]):
- `config.ignoreAccents` (*Boolean* [optional]): Defaults to `true`. Remove diacritics.
- `config.ignoreCase` (*Boolean* [optional]): Defaults to `true`. Lowercase everything.
- `config.matchFrom` (*'any' | 'start'* [optional]): Defaults to `'any'`.
- - `config.stringify` (*Func* [optional]): Defaults to `JSON.stringify`.
+ - `config.stringify` (*Func* [optional]).
- `config.trim` (*Boolean* [optional]): Defaults to `false`. Remove trailing spaces.
- `config.limit` (*Number* [optional]): Default to null. Limit the number of suggested options to be shown. For example, if `config.limit` is `100`, only the first `100` matching options are shown. It can be useful if a lot of options match and virtualization wasn't set up.
+#### Returns
+
+`filterOptions`: the returned filter method can be provided directly to the component/hook directly.
+
+#### Example
+
In the following demo, the options need to start with the query prefix:
```js
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
index e616b2b1a..03dc35ca8 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
@@ -9,13 +9,14 @@ export interface CreateFilterOptionsConfig<T> {
limit?: number;
}
-export interface FilterOptionsState {
+export interface FilterOptionsState<T> {
inputValue: string;
+ getOptionLabel: (option: T) => string;
}
export function createFilterOptions<T>(
config?: CreateFilterOptionsConfig<T>,
-): (options: T[], state: FilterOptionsState) => T[];
+): (options: T[], state: FilterOptionsState<T>) => T[];
export interface UseAutocompleteCommonProps<T> {
/**
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 9957ed76d..30f196254 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -11,35 +11,17 @@ function stripDiacritics(string) {
: string;
}
-function defaultStringify(value) {
- if (value == null) {
- return '';
- }
-
- if (typeof value === 'string') {
- return value;
- }
-
- if (typeof value === 'object') {
- return Object.keys(value)
- .map(key => value[key])
- .join(' ');
- }
-
- return JSON.stringify(value);
-}
-
export function createFilterOptions(config = {}) {
const {
ignoreAccents = true,
ignoreCase = true,
matchFrom = 'any',
- stringify = defaultStringify,
+ stringify,
trim = false,
limit,
} = config;
- return (options, { inputValue }) => {
+ return (options, { inputValue, getOptionLabel }) => {
let input = trim ? inputValue.trim() : inputValue;
if (ignoreCase) {
input = input.toLowerCase();
@@ -48,7 +30,7 @@ export function createFilterOptions(config = {}) {
input = stripDiacritics(input);
}
const filteredOptions = options.filter(option => {
- let candidate = stringify(option);
+ let candidate = (stringify || getOptionLabel)(option);
if (ignoreCase) {
candidate = candidate.toLowerCase();
}
@@ -269,7 +251,7 @@ export default function useAutocomplete(props) {
}),
// we use the empty string to manipulate `filterOptions` to not filter any options
// i.e. the filter predicate always returns true
- { inputValue: inputValueIsSelectedValue ? '' : inputValue },
+ { inputValue: inputValueIsSelectedValue ? '' : inputValue, getOptionLabel },
)
: []; |
I'd be happy to work on a PR. I'll probably open it under my personal account (@a-type) off work hours. |
@gforrest-bw Awesome :D |
of options against input value (mui#19798)
by default (mui#19798)
by default (mui#19798)
@oliviertassinari is there any possibility to change the filtering criteria from id to label have implemented highlighting, highlithing doesnt work and "[email protected]" is again selectable my ids are generated from front end( based on time milliseconds) so every option has different id, so when it comes to tags i can select already selected items too thats a problem ? any work around for this? this is the code i use for filtering
|
It would be absolutely fantastic if the documentation mentioned that This issue made me realize that I had to do |
Summary 💡
Rather than filtering based on the actual value of the Autocomplete options, by default the Autocomplete component could apply filtering to the output of
getOptionLabel
. This more closely aligns with what the user expects, since their only reference point of the options is what they see visibly on-screen.Examples 🌈
Supposing we had an Autocomplete like so:
The filtering of this Autocomplete would match the
STATE_NAMES
values of each state, rather than theSTATE_CODES
values, which would be shorter and less obvious, even if that's the value which the developer desires to be represented by the underlying form logic.Motivation 🔦
This behavior is intuitive for both users and developers. Users expect that if they type based on what they see (like "Calif"), it will match the option they expect ("California"). They do not expect to have to type some other hidden value ("CA").
In my experience as a developer, I also expect that if I supply a function to stingify my options, then my Autocomplete will search against those stringified versions, not the raw values. That's probably a less broad generalized assumption, but I still think it makes sense as a default.
The existing default experience appears to be stringifying the raw value naively - i.e. an object value would be simply have all its values concatenated (https://github.com/mui-org/material-ui/blob/master/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js#L23). This seems like it has a high chance of leading to unexpected behavior. For instance, if my Autocomplete is selecting from objects with randomized IDs, the user's input might happen to match one of them without clear reasoning.
Caveats
Obviously if the user supplies
renderOption
, the above ideas won't be true anymore. But I think that's a reasonable tradeoff. By the time you customize withrenderOption
, you'll probably have to supply afilterOptions
which aligns with your rendered options as well.The text was updated successfully, but these errors were encountered: