-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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: get all bytes from the text in native event key in fabric #45274
fix: get all bytes from the text in native event key in fabric #45274
Conversation
Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Base commit: 261f82e |
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.
Yes the culprit is the front()
operation however this code change doesn't fixes the complete problem. Even the attached screen-recording tries for changing language to Hindi but the text input's onKeyPress
doesn't show the entered non-ASCII character.
Examples of non-ASCII characters include: Accented letters: é, à, etc.
The problem is the way std::string handles unicode is different. It can store UTF-8 encoded data however it does not provide any built-in support for handling the encoding/decoding of Unicode characters. Using NSString
might help since it uses UTF-16 encoding internally, which simplifies handling of a wide range of characters or might wanna fallback to this kinda logic:
std::string getFirstChar(const std::string& str) {
if (str.empty()) {
return "";
}
unsigned char firstByte = str[0];
size_t charLength = 1;
if ((firstByte & 0x80) == 0) {
charLength = 1; // 1-byte character (ASCII)
} else if ((firstByte & 0xE0) == 0xC0) {
charLength = 2; // 2-byte character
} else if ((firstByte & 0xF0) == 0xE0) {
charLength = 3; // 3-byte character
} else if ((firstByte & 0xF8) == 0xF0) {
charLength = 4; // 4-byte character
}
return str.substr(0, charLength);
}
The problem is also entering non ASCII character (é) is being registered as onChange event instead of onKeyPress
Thanks @arushikesarwani94 .I will look in to it . |
What I mean by that is upon entering accented letters é (Long press e to select é) that doesn't trigger as a However your fix does address the non-ASCII characters when they are typed in directly from keyboard like (keyboard combo of Opt + Shift + Y produces Á, Opt + Shift + K produces ) |
@arushikesarwani94 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@arushikesarwani94 merged this pull request in 820a884. |
This pull request was successfully merged by @deepanshushuklad11 in 820a884. When will my fix make it into a release? | How to file a pick request? |
Summary:
Fixes #45199.
Problem was whenever you type in TextInput onKeypress callback called and event key was returned as
keyPressMetrics.text.front();
which basically fetch the first byte from text .
But in case of non-ascii character text is coming like ' \804'(in octal) which means first byte was empty string .
So solution was the return whole text as it is i.e.
keyPressMetrics.text
Changelog:
[IOS][FIXED] - nativeEvent key returning empty in case any non-ascii character entered , in IOS fabric
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
Test Plan:
Below are the screenrecording for solution in new arch
Solution_With_new_arch.1.mov