Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the default for underlineColorAndroid transparent #18938

Closed
janicduplessis opened this issue Apr 19, 2018 · 7 comments
Closed

Make the default for underlineColorAndroid transparent #18938

janicduplessis opened this issue Apr 19, 2018 · 7 comments
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot. Type: Enhancement A new feature or enhancement of an existing feature.

Comments

@janicduplessis
Copy link
Contributor

https://twitter.com/notbrent/status/986843528004292608

This will help reduce differences between iOS and Android, the default style is actually kind of broken right now because we no longer add extra padding on Android.

Before:
image

After:
image

The easiest implement this might be just to add underlineColorAndroid: 'transparent' to TextInput default props.

@janicduplessis janicduplessis added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. 🌟Feature Request and removed 📋No Template labels Apr 19, 2018
@facebook facebook deleted a comment from react-native-bot Apr 19, 2018
@joefazz
Copy link

joefazz commented Apr 19, 2018

How about just making it underlineColor (with transparent default) and make it so it just adds a border bottom on iOS? I tend to underline most of my textinputs but always just make the android underline transparent so I can share the style for both platforms. I realise this does increase complexity of the issue but it feels like if you set this to transparent then it's very rarely going to be used and people are only ever going to set a borderBottom.

@janicduplessis
Copy link
Contributor Author

@joefazz I think the underline is more of a material design thing, and poly-filling it on iOS is out of this scope. It's actually a bit different than border, both in the way it is positioned and is affected by focus. Also it could lead to some weird things if someone uses both border and underlineColor.

@AndrewIngram
Copy link
Contributor

It’s worth pointing out this that API doesn’t produce an underline compatible with the Material design spec for form inputs. So right now, any dev who is trying to implement material is also setting this prop to transparent and doing all the work on top of a blank slate.

@mikaoelitiana
Copy link
Contributor

I am willing to work on this one as a first issue.

@janicduplessis
Copy link
Contributor Author

@mikaoelitiana Cool! You can mention me in the PR and I'll have a look.

macdoum1 pushed a commit to macdoum1/react-native that referenced this issue Jun 28, 2018
Summary:
Set default `underlineColorAndroid` to `transparent`.
<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

Fixes facebook#18938

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

Use a TextInput in a component without defining `underlineColorAndroid`, the underline color should be transparent.

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[ANDROID] [BREAKING] [TextInput] - set default underlineColorAndroid to transparent

<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18988

Reviewed By: mdvacca

Differential Revision: D7765569

Pulled By: yungsters

fbshipit-source-id: f7ad57a46fc0d18b47271ca39faae8c635995fbb
@shubhnik
Copy link

@janicduplessis @mikaoelitiana with this commit, the underline color doesn't change to colorAccent defined in the theme in styles.xml. Is there anything else I need to do to get the desired behavior which was possible before this commit. This change and how to get the desired effect also isn't mentioned in the docs.

My RN version is 0.58.3.

@martnst
Copy link

martnst commented Feb 12, 2019

@shubhnik you can get back the old behaviour by setting.

<TextInput underlineColorAndroid={null} … />

See more this comment for more finding of mine on this.


Or in case you want to actually patch this globally:

function patchTextInputUnderlineColorAndroid() {
  const initialDefaultProps = TextInput.defaultProps
  TextInput.defaultProps = {
    ...initialDefaultProps,
    underlineColorAndroid: null // bring back underline color changed to transparent since RN 0.58
  }
}  

I am applying this in my index.js

@hramos hramos added the Type: Enhancement A new feature or enhancement of an existing feature. label Mar 14, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Apr 26, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants