Allow filtering characters in onChangeText for TextInput - #19087

Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom
rplankenhorn:filterCharactersFix
Closed

Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom
rplankenhorn:filterCharactersFix

Conversation

@rplankenhorn

@rplankenhornrplankenhorn commented May 1, 2018

Copy link
Copy Markdown

Fixes#18874

Test Plan

Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).

Release Notes

[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.

@facebook-github-bot

facebook-github-bot commented May 1, 2018

Copy link
Copy Markdown
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@react-native-botreact-native-bot added 📋Release Notes Component: TextInput Related to the TextInput component. labels May 1, 2018
@rplankenhorn
rplankenhorn changed the base branch from 0.55-stable to masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 1, 2018
@facebook-github-bot

facebook-github-bot commented May 1, 2018

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@tangkunyintangkunyin mentioned this pull request May 9, 2018
Closed
3 tasks
@tangkunyin

tangkunyin commented May 9, 2018

Copy link
Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456

@rplankenhorn

rplankenhorn commented May 9, 2018

Copy link
Copy Markdown
Author

Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.

@nol13

nol13 commented May 29, 2018•
edited
Loading

Copy link
Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.

With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.

Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.

@nol13nol13 mentioned this pull request May 30, 2018
Closed
@facebook-github-bot

facebook-github-bot commented Jun 1, 2018

Copy link
Copy Markdown
Contributor

@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@Ashoat

Ashoat commented Jun 8, 2018

Copy link
Copy Markdown
Contributor

Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
Closed
Closed
@rplankenhornrplankenhorn mentioned this pull request Jun 19, 2018
Closed
3 tasks
@lucasbento

lucasbento commented Jun 28, 2018

Copy link
Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!

@pvagare

pvagare commented Jul 9, 2018

Copy link
Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .

Please merge this pull request. So will use the same.

Thanks !

@lzxb

lzxb commented Jul 13, 2018

Copy link
Copy Markdown

I also have the same problem, the version is 0.55.4

@kelsetkelset added the Missing Test Plan This PR appears to be missing a test plan. label Jul 24, 2018
@kelset
kelset removed the request for review from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot added ✅Test Plan and removed Missing Test Plan This PR appears to be missing a test plan. labels Jul 24, 2018
@Ashoat

Ashoat commented Jul 30, 2018

Copy link
Copy Markdown
Contributor

Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?

@wsun

wsun commented Aug 2, 2018

Copy link
Copy Markdown
Contributor

This is a nice patch, thanks @rplankenhorn!

After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.

@hramos

hramos commented Aug 2, 2018

Copy link
Copy Markdown
Contributor

What Ashoat said. Do we still need this patch now that 892212b has landed?

@nol13

nol13 commented Aug 2, 2018

Copy link
Copy Markdown

Don't think 892212b fixes issue.

Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.

sidnair added a commit to taptapsend/react-native that referenced this pull request Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki

misheki commented Aug 14, 2018

Copy link
Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.

@lucasbento

lucasbento commented Aug 14, 2018

Copy link
Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?

@HelmerBarcos

HelmerBarcos commented Aug 17, 2018

Copy link
Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.

@RWOverdijk

RWOverdijk commented Aug 20, 2018

Copy link
Copy Markdown

+1. Using a patch is literally a patch :p

sidnair added a commit to taptapsend/react-native that referenced this pull request Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos

hramos commented Sep 12, 2018

Copy link
Copy Markdown
Contributor

In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.

hramos
hramos suggested changes Sep 12, 2018
View reviewed changes

@hramoshramos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.

@nol13

nol13 commented Sep 12, 2018

Copy link
Copy Markdown

For my use case, issue appears to be resolved in 0.57!

@vovkasm

vovkasm commented Sep 12, 2018

Copy link
Copy Markdown
Contributor

@hramos I can confirm that filtering issue already resolved in RN 0.57.0
Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57

@hramoshramos closed this Sep 13, 2018
dan-f pushed a commit to taptapsend/react-native that referenced this pull request Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit to taptapsend/react-native that referenced this pull request Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin
1 more reviewer
@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Allow filtering characters in onChangeText for TextInput - #19087
Closed
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Closed
Allow filtering characters in onChangeText for TextInput#19087
rplankenhorn wants to merge 1 commit into
react:masterfrom 
rplankenhorn:filterCharactersFix
Conversation
@rplankenhorn
@rplankenhornrplankenhorn
commented
May 1, 2018

Copy link


Copy Markdown

Fixes#18874
Test Plan
Create a sample app that performs validation in the onChangeText callback and modifies the characters. (e.g. filters out emojis).
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed issue with not being able to filter characters in onChangeText.
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.
If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!
@react-native-botreact-native-bot
added

📋Release Notes

Component: TextInput
Related to the TextInput component.
labels
May 1, 2018
@rplankenhorn
rplankenhorn
changed the base branch from
0.55-stable
to
masterMay 1, 2018 19:14
@facebook-github-botfacebook-github-bot
added
the 
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
May 1, 2018
@facebook-github-bot
facebook-github-bot
commented
May 1, 2018

Copy link


Copy Markdown

Contributor
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
@tangkunyintangkunyin
mentioned this pull request
May 9, 2018

Closed

3 tasks
@tangkunyin
tangkunyin
commented
May 9, 2018

Copy link


Copy Markdown

@rplankenhorn It doesn't work for me. But this can help: #18456
@rplankenhorn
rplankenhorn
commented
May 9, 2018

Copy link


Copy Markdown

Author
Hi @tangkunyin. I already commented on that change and it actually breaks a few things with text input so I don’t think it should be merged. I’ll work on creating a sample app so you can view the issue and my fix.
@nol13
nol13
commented
May 29, 2018•

edited

Loading

Copy link


Copy Markdown

This patch and #18456 both seem to work for me to get react-native-tag-input working reliably again.
With this patch I need to also apply #18627 along with it to fix the faulty backspace emitting issue, where #18456 seems to take care of that issue as well.
Edit: I dunno maybe something was cached, need #18627 either way now, current fork using this + hamaron's backspace fix.
@nol13nol13
mentioned this pull request
May 30, 2018

Closed

@facebook-github-bot
facebook-github-bot
commented
Jun 1, 2018

Copy link


Copy Markdown

Contributor
@rplankenhorn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.
@Ashoat
Ashoat
commented
Jun 8, 2018

Copy link


Copy Markdown

Contributor
Does this PR achieve the same goals as #18456? Can somebody explain the differences?

This was referenced Jun 8, 2018
 Closed


Closed

@rplankenhornrplankenhorn
mentioned this pull request
Jun 19, 2018

Closed

3 tasks
@lucasbento
lucasbento
commented
Jun 28, 2018

Copy link


Copy Markdown

Applied this patch to react-native 0.54.2 and works really well, thank you @rplankenhorn!
@pvagare
pvagare
commented
Jul 9, 2018

Copy link


Copy Markdown

Applied patch to react-native 0.54.2 and works really well, @rplankenhorn .
Please merge this pull request. So will use the same.
Thanks !
@lzxb
lzxb
commented
Jul 13, 2018

Copy link


Copy Markdown

I also have the same problem, the version is 0.55.4
@kelsetkelset
added
the 
Missing Test Plan
This PR appears to be missing a test plan.
label
Jul 24, 2018
@kelset
kelset
removed the request for review
from mhorowitzJuly 24, 2018 11:26
@react-native-botreact-native-bot
added

✅Test Plan

and removed

Missing Test Plan
This PR appears to be missing a test plan.
labels
Jul 24, 2018
@Ashoat
Ashoat
commented
Jul 30, 2018

Copy link


Copy Markdown

Contributor
Does 892212b solve this problem? The initial issue was constrained to Unicode emoticons yes?
@wsun
wsun
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
This is a nice patch, thanks @rplankenhorn!
After applying, I observed that I could no longer enter strings into uncontrolled password TextInput fields, so I modified the patch to ignore those situations.
@hramos
hramos
commented
Aug 2, 2018

Copy link


Copy Markdown

Contributor
What Ashoat said. Do we still need this patch now that 892212b has landed?
@nol13
nol13
commented
Aug 2, 2018

Copy link


Copy Markdown

Don't think 892212b fixes issue.
Or at least, after replacing the patched RCTBaseTextInputShadowView.m with RCTBaseTextInputShadowView.m from master in my 55.3 project, the issue that this patch solves reappears. Haven't tested on 56.
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 3, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
9661665
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@misheki
misheki
commented
Aug 14, 2018

Copy link


Copy Markdown

I applied this patch to 0.55.4 and is working perfectly so far! Thanks @rplankenhorn! I owe you coffee.
@lucasbento
lucasbento
commented
Aug 14, 2018

Copy link


Copy Markdown

@shergin, @hramos: would any of you have time to take a look at this PR?
@HelmerBarcos
HelmerBarcos
commented
Aug 17, 2018

Copy link


Copy Markdown

I have tested this patch on 0.56 and it works nice but the filtered characters are showed for a second. This should be handled as on Android, where undesirable characters are not showed.
@RWOverdijk
RWOverdijk
commented
Aug 20, 2018

Copy link


Copy Markdown

+1. Using a patch is literally a patch :p
sidnair added a commit
to taptapsend/react-native
that referenced
this pull request
Aug 21, 2018
@sidnair
fix ios bug where <TextInput> ignored value prop
7238193
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
@hramos
hramos
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
In order to move this PR forward, it would be useful to have someone review if it fixes the issue when this patch is applied on top of the changes we have on master right now. We've landed some fixes to related issues since this PR was opened, and I want to make sure the PR is still valid before proceeding.
hramos
hramos
suggested changes
Sep 12, 2018
View reviewed changes
@hramoshramos
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Rebase on top of master and make sure tests do not regress. Let us know if the fix is still valid after rebasing on top of master.
@nol13
nol13
commented
Sep 12, 2018

Copy link


Copy Markdown

For my use case, issue appears to be resolved in 0.57!
@vovkasm
vovkasm
commented
Sep 12, 2018

Copy link


Copy Markdown

Contributor
@hramos I can confirm that filtering issue already resolved in RN 0.57.0

Tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57
@hramoshramos
closed this
Sep 13, 2018
dan-f pushed a commit
to taptapsend/react-native
that referenced
this pull request
Jul 10, 2019
@sidnair@dan-f
fix ios bug where <TextInput> ignored value prop
19b32e4
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
benanderman pushed a commit
to taptapsend/react-native
that referenced
this pull request
Apr 10, 2020
@sidnair@benanderman
fix ios bug where <TextInput> ignored value prop
05c0382
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.
There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.
This implementation was inspired by PR react#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@sherginsherginAwaiting requested review from shergin

1 more reviewer

@hramoshramoshramos requested changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Component: TextInputRelated to the TextInput component.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

16 participants

@rplankenhorn@facebook-github-bot@tangkunyin@nol13@Ashoat@lucasbento@pvagare@lzxb@wsun@hramos@misheki@HelmerBarcos@RWOverdijk@vovkasm@kelset@react-native-bot