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

Support non-ASCII characters in EuiSearchBar & Query #1415

Merged

Conversation

chandlerprall
Copy link
Contributor

@chandlerprall chandlerprall commented Jan 8, 2019

Summary

Fixes #1409 by adding support for non-western/latin/ASCII characters & punctuation within term & field values. This casts a wide net, allowing any and all UTF-16 characters in the range U+00C0-U+FFFF (begins in Latin-1 Supplement). It's highly unlikely we would want to use any character in that range for another purpose in queries.

Implementation

I added the U+00C0-U+FFFF range as a supported value within the grammar's wordChar entry. PEG.js doesn't understand regex character classes so this abuses the fact that character ranges only care about byte code values. The exact characters for U+00C0 and U+FFFF are injected into the grammar's source instead of their unicode escape sequences.

Checklist

- [ ] This was checked in mobile
- [ ] This was checked in IE11
- [ ] This was checked in dark mode
- [ ] Any props added have proper autodocs
- [ ] Documentation examples were added

  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
    - [ ] This was checked against keyboard-only and screenreader scenarios
    - [ ] This required updates to Framer X components

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not address the use case from the original report. I linked in your changes and tried it out and got the same error. 汽车 is my test.

@chandlerprall
Copy link
Contributor Author

@bmcconaghy did you yarn build in EUI before linking? We also haven't worked on the EUI linking experience since webpack dlls were introduced to kibana's build process, I've heard you must completely stop & restart the kibana dev server to pick up changes in EUI's lib directory (which yarn build targets).

Your example query works fine in the EUI docs' Search Bar page with these changes
unicode search

@jasonrhodes
Copy link
Member

jasonrhodes commented Jan 9, 2019

So looks like before this change, these characters would cause an error in the search bar:

screen shot 2019-01-09 at 1 27 04 pm

or

screen shot 2019-01-09 at 1 28 30 pm

And now they are properly passed onto the search callback etc, is that right?

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unicode changes look sane to me.

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now that I have these changes actually running in my Kibana :-)

@chandlerprall
Copy link
Contributor Author

So looks like before this change, these characters would cause an error in the search bar:
...
And now they are properly passed onto the search callback etc, is that right?

Yeah, that is the goal :)

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.

3 participants