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

Parent border is drawn over the child content #44690

Closed
rafalmoneta opened this issue May 27, 2024 · 3 comments
Closed

Parent border is drawn over the child content #44690

rafalmoneta opened this issue May 27, 2024 · 3 comments
Labels
Resolution: PR Submitted A pull request with a fix has been provided. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@rafalmoneta
Copy link

rafalmoneta commented May 27, 2024

Description

Starting from version 0.74.0 of React Native, when fabric is enabled, the border of the parent element is rendered over its children elements on iOS. You can refer to the provided screenshot to visualize this issue. This behavior mirrors what's observed in iOS with Swift. However, it lacks compatibility with Android.

Steps to reproduce

  1. Install the application with yarn ios
  2. Enable the new architecture

React Native Version

0.74.1

Affected Platforms

Runtime - iOS

Areas

Fabric - The New Renderer

Output of npx react-native info

System:
  OS: macOS 14.0
  CPU: (10) arm64 Apple M2 Pro
  Memory: 101.33 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.18.0
    path: ~/.nvm/versions/node/v18.18.0/bin/node
  Yarn:
    version: 3.6.4
    path: ~/.nvm/versions/node/v18.18.0/bin/yarn
  npm:
    version: 9.8.1
    path: ~/.nvm/versions/node/v18.18.0/bin/npm
  Watchman:
    version: 2023.09.25.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.14.3
    path: /Users/softwaremansion/.asdf/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2022.3 AI-223.8836.35.2231.10811636
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 11.0.21
    path: /usr/bin/javac
  Ruby:
    version: 3.2.2
    path: /Users/softwaremansion/.asdf/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.1
    wanted: 0.74.1
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

none

Reproducer

https://github.com/rafalmoneta/react-native-border-issue

Screenshots and Videos

Old Architecture New Architecture
0.73 Screenshot 2024-05-27 at 13 26 17 Screenshot 2024-05-27 at 13 29 35
0.74 Screenshot 2024-05-27 at 15 29 43 Screenshot 2024-05-27 at 13 22 47
@rafalmoneta rafalmoneta added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels May 27, 2024
Copy link

⚠️ Newer Version of React Native is Available!
ℹ️ You are on a supported minor version, but it looks like there's a newer patch available - 0.74.1. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

@cortinico
Copy link
Contributor

cc @j-piasecki

@j-piasecki
Copy link
Contributor

I'll look into it

@cortinico cortinico added Resolution: PR Submitted A pull request with a fix has been provided. and removed Needs: Triage 🔍 Newer Patch Available labels Jun 4, 2024
kosmydel pushed a commit to kosmydel/react-native that referenced this issue Jun 11, 2024
Summary:
Fixes facebook#44690

In the code responsible for drawing border on iOS there's a comment saying:
> iOS draws borders in front of the content whereas CSS draws them behind the content. For this reason, only use iOS border drawing when clipping or when the border is hidden.

The condition that follows checks whether the content is clipped and the width and alpha channel of the border: https://github.com/facebook/react-native/blob/e0a2e86d0346bd7e40adf69311daa538ca8c9c5f/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm#L643-L644.

The problem is when the color is not set at all - `colorComponentsFromColor(borderMetrics.borderColors.left).alpha` will be equal to 0 since the relevant `SharedColor` is `null`: https://github.com/facebook/react-native/blob/e0a2e86d0346bd7e40adf69311daa538ca8c9c5f/packages/react-native/ReactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm#L76-L86

Then it uses the path with the default iOS behavior (drawing the border on top of the content) instead of the custom one (with the border below) and it seems like it defaults to drawing black when the passed color is `nil`.

This PR simply adds one more check to make sure the color is actually set before choosing the default platform behavior.

## Changelog:

[IOS] [FIXED] - Fixed border being drawn over children when no color was set

Pull Request resolved: facebook#44777

Test Plan:
Tested on the code from the issue.

|Before|After|
|-|-|
|<img width="546" alt="Screenshot 2024-06-04 at 11 18 14" src="https://github.com/facebook/react-native/assets/21055725/f13250a9-2e99-41c5-a9bc-02d65c00a6c0">|<img width="546" alt="Screenshot 2024-06-04 at 11 17 38" src="https://github.com/facebook/react-native/assets/21055725/f4571a5f-dfc4-4191-854c-fd3faf698b29">|

Reviewed By: cortinico

Differential Revision: D58131337

Pulled By: cipolleschi

fbshipit-source-id: 7da247d81ecec586de6f0023e0cb399f9966213d
Titozzz pushed a commit that referenced this issue Jun 18, 2024
Summary:
Fixes #44690

In the code responsible for drawing border on iOS there's a comment saying:
> iOS draws borders in front of the content whereas CSS draws them behind the content. For this reason, only use iOS border drawing when clipping or when the border is hidden.

The condition that follows checks whether the content is clipped and the width and alpha channel of the border: https://github.com/facebook/react-native/blob/e0a2e86d0346bd7e40adf69311daa538ca8c9c5f/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm#L643-L644.

The problem is when the color is not set at all - `colorComponentsFromColor(borderMetrics.borderColors.left).alpha` will be equal to 0 since the relevant `SharedColor` is `null`: https://github.com/facebook/react-native/blob/e0a2e86d0346bd7e40adf69311daa538ca8c9c5f/packages/react-native/ReactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm#L76-L86

Then it uses the path with the default iOS behavior (drawing the border on top of the content) instead of the custom one (with the border below) and it seems like it defaults to drawing black when the passed color is `nil`.

This PR simply adds one more check to make sure the color is actually set before choosing the default platform behavior.

## Changelog:

[IOS] [FIXED] - Fixed border being drawn over children when no color was set

Pull Request resolved: #44777

Test Plan:
Tested on the code from the issue.

|Before|After|
|-|-|
|<img width="546" alt="Screenshot 2024-06-04 at 11 18 14" src="https://github.com/facebook/react-native/assets/21055725/f13250a9-2e99-41c5-a9bc-02d65c00a6c0">|<img width="546" alt="Screenshot 2024-06-04 at 11 17 38" src="https://github.com/facebook/react-native/assets/21055725/f4571a5f-dfc4-4191-854c-fd3faf698b29">|

Reviewed By: cortinico

Differential Revision: D58131337

Pulled By: cipolleschi

fbshipit-source-id: 7da247d81ecec586de6f0023e0cb399f9966213d
Titozzz pushed a commit that referenced this issue Jun 18, 2024
Summary:
Fixes #44690

In the code responsible for drawing border on iOS there's a comment saying:
> iOS draws borders in front of the content whereas CSS draws them behind the content. For this reason, only use iOS border drawing when clipping or when the border is hidden.

The condition that follows checks whether the content is clipped and the width and alpha channel of the border: https://github.com/facebook/react-native/blob/e0a2e86d0346bd7e40adf69311daa538ca8c9c5f/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm#L643-L644.

The problem is when the color is not set at all - `colorComponentsFromColor(borderMetrics.borderColors.left).alpha` will be equal to 0 since the relevant `SharedColor` is `null`: https://github.com/facebook/react-native/blob/e0a2e86d0346bd7e40adf69311daa538ca8c9c5f/packages/react-native/ReactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics/HostPlatformColor.mm#L76-L86

Then it uses the path with the default iOS behavior (drawing the border on top of the content) instead of the custom one (with the border below) and it seems like it defaults to drawing black when the passed color is `nil`.

This PR simply adds one more check to make sure the color is actually set before choosing the default platform behavior.

## Changelog:

[IOS] [FIXED] - Fixed border being drawn over children when no color was set

Pull Request resolved: #44777

Test Plan:
Tested on the code from the issue.

|Before|After|
|-|-|
|<img width="546" alt="Screenshot 2024-06-04 at 11 18 14" src="https://github.com/facebook/react-native/assets/21055725/f13250a9-2e99-41c5-a9bc-02d65c00a6c0">|<img width="546" alt="Screenshot 2024-06-04 at 11 17 38" src="https://github.com/facebook/react-native/assets/21055725/f4571a5f-dfc4-4191-854c-fd3faf698b29">|

Reviewed By: cortinico

Differential Revision: D58131337

Pulled By: cipolleschi

fbshipit-source-id: 7da247d81ecec586de6f0023e0cb399f9966213d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: PR Submitted A pull request with a fix has been provided. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants