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

underlineColor transparent not working on API 21 #30897

Closed
wants to merge 14 commits into from

Conversation

fabOnReact
Copy link
Contributor

@fabOnReact fabOnReact commented Feb 5, 2021

Summary

This issue fixes #29379 underlineColorAndroid'='transparent' does not work on Android API 21.
Setting style: { bottomBorderColor: 'transparent'} fixes the issue.

The following steps fix underlineColorAndroid on Android API 21:

  • Store the bottomBorderColor in a local variable
  • Then set the bottomBorderColor to the same color of underlineColorAndroid
  • Set underlineColorAndroid
  • Then Set the bottomBorderColor to the previous color previously stored in the local variable

This change requires ReactViewBackgroundDrawable method getBorderColor to be public and accessible from ReactTextInputManager to retrieve the border color.

private int getBorderColor(int position) {
float rgb = mBorderRGB != null ? mBorderRGB.get(position) : DEFAULT_BORDER_RGB;
float alpha = mBorderAlpha != null ? mBorderAlpha.get(position) : DEFAULT_BORDER_ALPHA;
return ReactViewBackgroundDrawable.colorFromAlphaAndRGBComponents(alpha, rgb);
}

Related Discussions #18938 #18988 #29412 (comment)

More Information at fabOnReact/react-native-notes#4 (comment)

Changelog

[Android] [Fixed] - Fix underlineColorAndroid transparent not working on API 21

Test Plan

This changes fix the Java API which can not be tested as explained in commit 709a441
The java TextInputTest was excluded from the test suite in commit 709a441 as they need the Yoga libraries to run

CLICK TO OPEN TESTS RESULTS - API 21

Does not show underline by default (transparent)

<TextInput />
<TextInput underlineColorAndroid="red" />
<TextInput
  underlineColorAndroid="green" 
  style={
     {
        borderBottomColor: 'red', 
        borderBottomWidth: 2, 
        borderTopColor: 'black', 
        borderTopWidth: 4
     }
   }
/>
<TextInput
  style={
     {
        borderBottomColor: 'red', 
        borderBottomWidth: 2, 
        borderTopColor: 'black', 
        borderTopWidth: 4
     }
   }
/>
<TextInput
  underlineColorAndroid="blue"
  style={
    {
      borderBottomColor: 'red', 
      borderBottomWidth: 2,
      borderTopColor: 'black',
      borderTopWidth: 4,
      borderLeftColor: 'pink', 
      borderLeftWidth: 7,
      borderRightColor: 'yellow', 
      borderRightWidth: 7,
    }
  }
/>
<TextInput
  underlineColorAndroid="transparent"
  style={
    {
      borderBottomColor: 'red', 
      borderBottomWidth: 2,
      borderTopColor: 'black',
      borderTopWidth: 4,
      borderLeftColor: 'pink', 
      borderLeftWidth: 7,
      borderRightColor: 'yellow', 
      borderRightWidth: 7,
    }
  }
/>

@facebook-github-bot facebook-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 Feb 5, 2021
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

@analysis-bot
Copy link

analysis-bot commented Feb 5, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 189c2c8
Branch: main

on API 21 we change the logic as follows to fix underlineColorAndroid:
- we store the bottomBorderColor in a local variable
- we set the bottomBorderColor to the same color of
  underlineColorAndroid
- we set the underlineColorAndroid
- we re-set the bottomBorderColor to the previous color

As by Facebook design it defaults to
transparent

facebook#18938
facebook#18988

but it is important to notice how this default is now not anymore used
as JAVA sets a new default of transparent for all colors (null is not
passed to this setter)

facebook#29412 (comment)
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

Fixes the following Scenario.
```javascript
<TextInput
  underlineColorAndroid="green"
  style={{borderBottomColor: 'red', borderBottomWidth: 2, borderTopColor: 'black', borderTopWidth: 4}}
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107062411-9167df80-67d9-11eb-810c-749992d8d2d3.png" width="150" />
drawableToMutate.setColorFilter(underlineColor, PorterDuff.Mode.SRC_IN);
if (Build.VERSION.SDK_INT == Build.VERSION_CODES.LOLLIPOP) {
int bottomBorderColor = view.getBorderColor(Spacing.BOTTOM);
setBorderColor(view, Spacing.START, underlineColor);

This comment was marked as off-topic.

@fabOnReact fabOnReact marked this pull request as ready for review February 5, 2021 17:06
@analysis-bot
Copy link

analysis-bot commented Feb 11, 2021

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,203,284 -917
android hermes armeabi-v7a 7,810,344 -724
android hermes x86 8,575,453 -291
android hermes x86_64 8,526,060 -482
android jsc arm64-v8a 9,872,024 -920
android jsc armeabi-v7a 8,863,616 -744
android jsc x86 9,840,280 -290
android jsc x86_64 10,435,005 -492

Base commit: 189c2c8
Branch: main

@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Oct 1, 2021
@facebook-github-bot
Copy link
Contributor

@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @fabriziobertoglio1987

The following steps fix underlineColorAndroid on Android API 21:

  • Store the bottomBorderColor in a local variable
  • Then set the bottomBorderColor to the same color of underlineColorAndroid
  • Set underlineColorAndroid
  • Then Set the bottomBorderColor to the previous color previously stored in the local variable

Can I ask you why this is needed? Like what's the underlying issue that is causing this?

@@ -545,7 +545,14 @@ public void setUnderlineColor(ReactEditText view, @Nullable Integer underlineCol
if (underlineColor == null) {
drawableToMutate.clearColorFilter();
} else {
drawableToMutate.setColorFilter(underlineColor, PorterDuff.Mode.SRC_IN);
if (Build.VERSION.SDK_INT == Build.VERSION_CODES.LOLLIPOP) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment here on why this is needed otherwise would be really hard to understand afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cortinico thanks a lot. I added the comment with commit 6ebac50 and updated the summary of the pull request #30897 (comment) to include a link to fabOnReact/react-native-notes#4 (comment). I remain available.

@facebook-github-bot
Copy link
Contributor

@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @fabriziobertoglio1987 in 52aee50.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Mar 10, 2022
alphatrl pushed a commit to taskade/react-native that referenced this pull request Dec 6, 2022
Summary:
This issue fixes facebook#29379 `underlineColorAndroid'='transparent'` does not work on Android API 21.
Setting `style: { bottomBorderColor: 'transparent'}` fixes the issue.

The following steps fix underlineColorAndroid on Android API 21:
- Store the bottomBorderColor in a local variable
- Then set the bottomBorderColor to the same color of underlineColorAndroid
- Set underlineColorAndroid
- Then Set the bottomBorderColor to the previous color previously stored in the local variable

This change requires `ReactViewBackgroundDrawable` method `getBorderColor` to be public and accessible from `ReactTextInputManager` to retrieve the border color.

https://github.com/facebook/react-native/blob/6061b7928320c64a94716ce3d6646667ebf3f6b5/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java#L1231-L1236

Related Discussions facebook#18938 facebook#18988 facebook#29412 (comment)

More Information at fabOnReact/react-native-notes#4 (comment)

## Changelog

[Android] [Fixed] - Fix underlineColorAndroid transparent not working on API 21

Pull Request resolved: facebook#30897

Test Plan:
This changes fix the Java API which can not be tested as explained in commit facebook@709a441
The java TextInputTest was excluded from the test suite in commit facebook@709a441 as they need the Yoga libraries to run

**<details><summary>CLICK TO OPEN TESTS RESULTS - API 21</summary>**
<p>

Does not show underline by default (`transparent`)

```javascript
<TextInput />
```

<image src="https://user-images.githubusercontent.com/24992535/107060953-dee34d00-67d7-11eb-8f01-360dbb1420b8.png" width="150" />

```javascript
<TextInput underlineColorAndroid="red" />
```

<image src="https://user-images.githubusercontent.com/24992535/107061134-194cea00-67d8-11eb-9da1-9780b1aa8294.png" width="150" />

```javascript
<TextInput
  underlineColorAndroid="green"
  style={
     {
        borderBottomColor: 'red',
        borderBottomWidth: 2,
        borderTopColor: 'black',
        borderTopWidth: 4
     }
   }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107062411-9167df80-67d9-11eb-810c-749992d8d2d3.png" width="150" />

```javascript
<TextInput
  style={
     {
        borderBottomColor: 'red',
        borderBottomWidth: 2,
        borderTopColor: 'black',
        borderTopWidth: 4
     }
   }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107062735-e6a3f100-67d9-11eb-93c3-02cd768f8421.png" width="150" />

```javascript
<TextInput
  underlineColorAndroid="blue"
  style={
    {
      borderBottomColor: 'red',
      borderBottomWidth: 2,
      borderTopColor: 'black',
      borderTopWidth: 4,
      borderLeftColor: 'pink',
      borderLeftWidth: 7,
      borderRightColor: 'yellow',
      borderRightWidth: 7,
    }
  }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107063263-75b10900-67da-11eb-97ab-316736d525a2.png" width="150" />

```javascript
<TextInput
  underlineColorAndroid="transparent"
  style={
    {
      borderBottomColor: 'red',
      borderBottomWidth: 2,
      borderTopColor: 'black',
      borderTopWidth: 4,
      borderLeftColor: 'pink',
      borderLeftWidth: 7,
      borderRightColor: 'yellow',
      borderRightWidth: 7,
    }
  }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107063332-8792ac00-67da-11eb-82fc-99793bf4d49d.png" width="150" />

</p>
</details>

Reviewed By: cortinico

Differential Revision: D33906415

Pulled By: lunaleaps

fbshipit-source-id: bd7efe4aac40ad701aec83f051ac40dcd4a99cda
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 15, 2023
Summary:
This issue fixes facebook#29379 `underlineColorAndroid'='transparent'` does not work on Android API 21.
Setting `style: { bottomBorderColor: 'transparent'}` fixes the issue.

The following steps fix underlineColorAndroid on Android API 21:
- Store the bottomBorderColor in a local variable
- Then set the bottomBorderColor to the same color of underlineColorAndroid
- Set underlineColorAndroid
- Then Set the bottomBorderColor to the previous color previously stored in the local variable

This change requires `ReactViewBackgroundDrawable` method `getBorderColor` to be public and accessible from `ReactTextInputManager` to retrieve the border color.

https://github.com/facebook/react-native/blob/6061b7928320c64a94716ce3d6646667ebf3f6b5/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundDrawable.java#L1231-L1236

Related Discussions facebook#18938 facebook#18988 facebook#29412 (comment)

More Information at fabOnReact/react-native-notes#4 (comment)

## Changelog

[Android] [Fixed] - Fix underlineColorAndroid transparent not working on API 21

Pull Request resolved: facebook#30897

Test Plan:
This changes fix the Java API which can not be tested as explained in commit facebook@709a441
The java TextInputTest was excluded from the test suite in commit facebook@709a441 as they need the Yoga libraries to run

**<details><summary>CLICK TO OPEN TESTS RESULTS - API 21</summary>**
<p>

Does not show underline by default (`transparent`)

```javascript
<TextInput />
```

<image src="https://user-images.githubusercontent.com/24992535/107060953-dee34d00-67d7-11eb-8f01-360dbb1420b8.png" width="150" />

```javascript
<TextInput underlineColorAndroid="red" />
```

<image src="https://user-images.githubusercontent.com/24992535/107061134-194cea00-67d8-11eb-9da1-9780b1aa8294.png" width="150" />

```javascript
<TextInput
  underlineColorAndroid="green"
  style={
     {
        borderBottomColor: 'red',
        borderBottomWidth: 2,
        borderTopColor: 'black',
        borderTopWidth: 4
     }
   }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107062411-9167df80-67d9-11eb-810c-749992d8d2d3.png" width="150" />

```javascript
<TextInput
  style={
     {
        borderBottomColor: 'red',
        borderBottomWidth: 2,
        borderTopColor: 'black',
        borderTopWidth: 4
     }
   }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107062735-e6a3f100-67d9-11eb-93c3-02cd768f8421.png" width="150" />

```javascript
<TextInput
  underlineColorAndroid="blue"
  style={
    {
      borderBottomColor: 'red',
      borderBottomWidth: 2,
      borderTopColor: 'black',
      borderTopWidth: 4,
      borderLeftColor: 'pink',
      borderLeftWidth: 7,
      borderRightColor: 'yellow',
      borderRightWidth: 7,
    }
  }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107063263-75b10900-67da-11eb-97ab-316736d525a2.png" width="150" />

```javascript
<TextInput
  underlineColorAndroid="transparent"
  style={
    {
      borderBottomColor: 'red',
      borderBottomWidth: 2,
      borderTopColor: 'black',
      borderTopWidth: 4,
      borderLeftColor: 'pink',
      borderLeftWidth: 7,
      borderRightColor: 'yellow',
      borderRightWidth: 7,
    }
  }
/>
```

<image src="https://user-images.githubusercontent.com/24992535/107063332-8792ac00-67da-11eb-82fc-99793bf4d49d.png" width="150" />

</p>
</details>

Reviewed By: cortinico

Differential Revision: D33906415

Pulled By: lunaleaps

fbshipit-source-id: bd7efe4aac40ad701aec83f051ac40dcd4a99cda
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[0.63.1][Android 5] underlineColorAndroid not transparent
5 participants