feat(Dropdown): remove diacritics on filter #2021
Conversation
src/modules/Dropdown/Dropdown.js
Outdated
| @@ -795,7 +795,14 @@ export default class Dropdown extends Component { | |||
| filteredOptions = search(filteredOptions, searchQuery) | |||
| } else { | |||
| const re = new RegExp(_.escapeRegExp(searchQuery), 'i') | |||
| filteredOptions = _.filter(filteredOptions, opt => re.test(opt.text)) | |||
| filteredOptions = _.filter(filteredOptions, function (opt) { | |||
There was a problem hiding this comment.
This should just be:
-filteredOptions = _.filter(filteredOptions, opt => re.test(opt.text))
+filteredOptions = _.filter(filteredOptions, opt => re.test(_.deburr(opt.text)))
|
See inline comment on |
|
for example when i have this options:
|
|
Yep, I'm noting we should use lodash's
|
|
ty for your quick response, i will try to add a test, first time I am doing a pull request so please have patience :) |
Codecov Report
@@ Coverage Diff @@
## master #2021 +/- ##
=======================================
Coverage 99.76% 99.76%
=======================================
Files 148 148
Lines 2556 2556
=======================================
Hits 2550 2550
Misses 6 6
Continue to review full report at Codecov.
|
src/modules/Dropdown/Dropdown.js
Outdated
| @@ -795,7 +795,8 @@ export default class Dropdown extends Component { | |||
| filteredOptions = search(filteredOptions, searchQuery) | |||
| } else { | |||
| const re = new RegExp(_.escapeRegExp(searchQuery), 'i') | |||
| filteredOptions = _.filter(filteredOptions, opt => re.test(opt.text)) | |||
| // remove diacritics on search | |||
| filteredOptions = _.filter(filteredOptions, opt => re.test(opt.text ? _.deburr(opt.text): opt.text)) | |||
There was a problem hiding this comment.
Lodash gracefully handles null and undefined values, so you can safely remove the ternary check here. The method will always return a string:
|
|
||
| wrapper | ||
| .find('.selected') | ||
| .should.contain.text('FLOREŞTI') |
There was a problem hiding this comment.
This looks great. We can end the test here as one assertion is enough to prove the feature.
Skipping the additional arrow down behaviors will make this test stronger. This way, the filter after diacritics test doesn't fail if the arrow down behavior changes, but only if the search filter behavior changes. 👍
|
Awesome, congrats on your first PR :) I left a couple more comments, then I think we're ready to go here. |
|
Be sure to run |
|
hey i've made the changes, i hope they are good, thank you soooo much for your help, i am going to brag all day to my work colleagues :))) good job on this library 👍 |
layershifter
changed the title
|
Thanks! Nice work, brag away my friend :) |
* remove diacritics on filter * change to arrow function * use lodash _.deburr instead * added a test for filter after diacritics * remove unnecessary ternary check * remove unnecessary tests check on filter diacritics
|
Released in |
* remove diacritics on filter * change to arrow function * use lodash _.deburr instead * added a test for filter after diacritics * remove unnecessary ternary check * remove unnecessary tests check on filter diacritics
I really need this feature, can you guys help me?