-
Notifications
You must be signed in to change notification settings - Fork 2k
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
UI: fuzzy and tokenized job search #4201
Conversation
This time around fuzzy, exact, and regex can all be toggled. Additionally, each search type gets its own set of keys to search for. This means fuzzy search can only look at name while regex and exact match will still look at ID.
Trailing whitespace messes with tokenization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a neat implementation! I only have one trivial code formatting suggestion, everything looks good to me. Great test coverage 👍
ui/app/mixins/searchable.js
Outdated
} | ||
return results.uniq(); | ||
} | ||
return this.get('listToSearch'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inverting the condition and returning the full list earlier would reduce indentation for the search code and might be a little clearer.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
The better to search through large lists with.