Skip to content
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

Fix case sensitive fluent icon search service #814

Merged
merged 1 commit into from
Mar 6, 2021

Conversation

mbice
Copy link
Contributor

@mbice mbice commented Mar 4, 2021

Q A
Bug fix? [X]
New feature? [ ]
New sample? [ ]
Related issues? Nothing logged I could find

What's in this Pull Request?

There is an issue in the Icon Picker control where searching for Fluent icon names is case sensitive. For example, searching for "aadlogo" will return an icon, whereas searching for "AADLogo", which is the friendly name of the icon, returns nothing.

Searching for "aadlogo"

image

Searching for "AADLogo"

image

This fix ensures that the casing of the search term is ignored so that icons are returned regardless of lower/uppercase characters.

Thanks!

@AJIXuMuK AJIXuMuK merged commit 1028894 into pnp:dev Mar 6, 2021
@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Mar 6, 2021

Thank you @mbice for the fix!

I've merged it with a small change: I reverted back !startsWith to startsWith === false because this change reverses the default behavior.

@AJIXuMuK AJIXuMuK added this to the 2.6.0 milestone Mar 6, 2021
@mbice
Copy link
Contributor Author

mbice commented Mar 6, 2021

Thanks for the merge, @AJIXuMuK.

Sorry, I should have documented the change to "startsWith" as well. I noticed that this method is not always being called with the startsWith parameter, as it's optional. In the case of the iconPicker, nothing gets passed in, so startsWith === false will never evaluate as true. In the case of the icon picker this means it will always string match from the starting of the string. I changed it to !startsWith so that either explicitly passed in as "false" or not provided would be true.

Thoughts?

Here's the call from icon picker:

image

@AJIXuMuK
Copy link
Collaborator

Got it @mbice!

I added it back and also added a new prop to the controls to be able to switch the behavior.

@mbice mbice deleted the hotfix/icon-search-case-sensitive branch March 10, 2021 01:37
@estruyf estruyf mentioned this pull request Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants