Skip to content
This repository has been archived by the owner on Dec 23, 2022. It is now read-only.

default filter as caseInsensitive ? #17

Closed
Sharlaan opened this issue Sep 30, 2016 · 17 comments
Closed

default filter as caseInsensitive ? #17

Sharlaan opened this issue Sep 30, 2016 · 17 comments
Assignees

Comments

@Sharlaan
Copy link

default filter is set as caseSensistive in v0.4.1

Would not it be better as caseInsensitive by default ?
(would just have to add .toLowerCase() to the filter function)

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

Indeed, that would be cool. You'd have to use .toLowerCase() on the value the user typed and the auto complete values for comparison, though.
Do you want to create a PR for this?

@Sharlaan
Copy link
Author

me and PR = 2 ...

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

@Sharlaan What? 😄

@Sharlaan
Copy link
Author

Maybe something like this ?

static defaultProps = {
    ...
    filter: AutoComplete.caseInsensitiveFilter,
    ...
}

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

The filter needs to match every value if the input is empty so that the dropdown menu opens automatically on focus. The default filters of AutoComplete don't do this. That's why we have to provide our own default filter.

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

@Sharlaan Re-using AutoCompletes filters sounds good to me. I'll change the way the auto-open feature works (and resolve this issue while I'm on it). 👍

@Sharlaan
Copy link
Author

Thanks but my current tests seem to show the filter property doesnot take any provided function:

<ChipInput
  filter={(search, key) => search === '' || key.toLowerCase().includes(search.toLowerCase())}
/>

... still gives caseSensistive behavior while should be insensitive.

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

@Sharlaan I'm currently fixing that. 👍

@Sharlaan
Copy link
Author

Impressive answer speed 👍

@leMaik leMaik self-assigned this Sep 30, 2016
@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

This works now:

<ChipInput
  openOnFocus
  filter={AutoComplete.fuzzyFilter}
  dataSource={['alpha', 'beta']}
  hintText="Try typing apha..." //sic!
/>

@leMaik leMaik closed this as completed in b4bf851 Sep 30, 2016
@Sharlaan
Copy link
Author

Great job, default caseInsensitive works perfect !

but... filter={AutoComplete.fuzzyFilter} means user has to import { AutoComplete } from 'material-ui' on top of importing material-chip-input....

That's why i suggested the enum in Issue #18 .

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

@Sharlaan I don't wand to add any filter to this component. Importing AutoComplete adds no overhead at all, as this component also uses AutoComplete.

@Sharlaan
Copy link
Author

import React from 'react'
import { AutoComplete } from 'material-ui'
import ChipInput from 'material-chip-input'

const Container = (props) => {
  return (
    <ChipInput filter={AutoComplete.fuzzyFilter} />
  )
}

compared to:

import React from 'react'
import ChipInput from 'material-chip-input'

const Container = (props) => {
  return (
    <ChipInput filter={'fuzzyFilter'} />
  )
}

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

Wow, that's one line in your code vs. more code I have to maintain. 👎 I'm still not convinced. What's so bad about importing stuff? Using libraries and re-using code is great! 😄

@Sharlaan
Copy link
Author

true it's more typing for you, i just meant AutoComplete reference should be purely internal.

@leMaik
Copy link
Member

leMaik commented Sep 30, 2016

Oh, I see what you mean. You don't want to depend on AutoComplete because it's just an implementation detail? Just think of the filter function as of any function, where AutoComplete.fuzzyFilter (and the other functions) fortunately have the same interface and thus can be used as such any function. :)

It's much more flexible this way, as it allows to use any function and not just some fixed set of functions.

@Sharlaan
Copy link
Author

Ok got it, sorry i didnot realize the "single interface for any filter function" pattern.
Fine with that.

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

No branches or pull requests

2 participants