[Android] Password color #6064
[Android] Password color #6064
Conversation
|
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 - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
|
Thanks for the PR @sooth-sayer. What's your test plan? Have you tried setting this from JS and running the code? Can you add a screenshot showing how this looks? Does the feature work on iOS exactly the same way? Asking because consistency between iOS and Android APIs where possible is great. |
sooth-sayer
force-pushed
the
input-password-color
branch
from
ce2ef6f
to
8c11e43
Compare
|
@sooth-sayer updated the pull request. |
|
|
| @@ -165,6 +165,11 @@ public void updateExtraData(ReactEditText view, Object extraData) { | |||
| } | |||
| } | |||
|
|
|||
| @ReactProp(name = ViewProps.COLOR, defaultInt = 0) | |||
| public void setColor(ReactEditText view, @Nullable Integer color) { | |||
There was a problem hiding this comment.
You should probably say that this has customType="color" as well.
I'm not sure if defaultInt will keep null from actually showing up. If null does show up or defaultInt you probably need to set it back to whatever it was before.
There was a problem hiding this comment.
@dmmiller Could you explain in more detail about defaultInt?
|
any plans for this fix to be merged in soon? we tested on a forked version of 0.20 and it worked well. |
|
@sooth-sayer updated the pull request. |
| @@ -165,6 +165,11 @@ public void updateExtraData(ReactEditText view, Object extraData) { | |||
| } | |||
| } | |||
|
|
|||
| @ReactProp(name = ViewProps.COLOR, defaultInt = 0, customType="color") | |||
There was a problem hiding this comment.
Can you remove the defaultInt and check one thing for me? If you set the color to something and then you rerender with no color set, does it revert back to the proper color?
There was a problem hiding this comment.
@dmmiller If i set the color and then rerender without color, it fails with "Error while updating property 'color' of a view managed by: AndroidTextInput"
There was a problem hiding this comment.
If you leave the defaultInt bit in, does it then work?
There was a problem hiding this comment.
No. it fails regardless of defaultInt presence
There was a problem hiding this comment.
We need to figure that out before we can take it. It's possible for people to want to change their color and then back. You could also look at why the color that works via spans doesn't for password colors.
|
One thing to check which I put inline and then I will accept. Thanks |
|
@sooth-sayer updated the pull request. |
|
@sooth-sayer I'd like to take this, but I want to know the answer to whether changing colors and removing colors correctly updates. Can you please indicate that? |
|
@dmmiller No, it fails when I remove color. I'll try to fix it as soon as I will have enough time. |
Summary:Color property is ignored for TextInput with secureTextEntry={true} on Android.
This is coming from #6540 and being split into 2 PRs by suggestion of dmmiller.
As he mentioned does almost the same except it #6064 doesn't handle null case.
Working example:
Closes #6563
Differential Revision: D3077583
Pulled By: mkonicek
fb-gh-sync-id: f0761346b77060abf17f1d4573b375adebc5c607
shipit-source-id: f0761346b77060abf17f1d4573b375adebc5c607
Summary:Color property is ignored for TextInput with secureTextEntry={true} on Android.
This is coming from facebook#6540 and being split into 2 PRs by suggestion of dmmiller.
As he mentioned does almost the same except it facebook#6064 doesn't handle null case.
Working example:
Closes facebook#6563
Differential Revision: D3077583
Pulled By: mkonicek
fb-gh-sync-id: f0761346b77060abf17f1d4573b375adebc5c607
shipit-source-id: f0761346b77060abf17f1d4573b375adebc5c607
|
You can probably remove the defaultInt, check for null and use the default text color from the resources with something like |
|
Just noticed there's another PR that does this that landed already #6563. I guess we can close this one. |
|
Thanks @janicduplessis! |
Summary:Color property is ignored for TextInput with secureTextEntry={true} on Android.
This is coming from facebook#6540 and being split into 2 PRs by suggestion of dmmiller.
As he mentioned does almost the same except it facebook#6064 doesn't handle null case.
Working example:
Closes facebook#6563
Differential Revision: D3077583
Pulled By: mkonicek
fb-gh-sync-id: f0761346b77060abf17f1d4573b375adebc5c607
shipit-source-id: f0761346b77060abf17f1d4573b375adebc5c607
Summary:Color property is ignored for TextInput with secureTextEntry={true} on Android.
This is coming from #6540 and being split into 2 PRs by suggestion of dmmiller.
As he mentioned does almost the same except it #6064 doesn't handle null case.
Working example:
Closes facebook/react-native#6563
Differential Revision: D3077583
Pulled By: mkonicek
fb-gh-sync-id: f0761346b77060abf17f1d4573b375adebc5c607
shipit-source-id: f0761346b77060abf17f1d4573b375adebc5c607
Closes #4435