You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
I am making a fix / change for the macOS implementation of react-native
I am making a change required for Microsoft usage of react-native
Summary
Pull request #1227 mostly fixed the consistency issue but had a problem with the shadow opacity. There isn't a shadowOpacity property on NSShadow, the shadow opacity from the layer needed to be passed into the alpha channel of the NSShadow shadowColor property.
Note: We tried an additional approach to fix this, since the root issue behind this bug is that modifying certain layer props (including shadow related ones) on a layer owned by an NSView is undefined behavior, and we are still doing this in RCTViewManager with RCT_REMAP_VIEW_PROPERTY
In the second approach, rather than modifying RCTView.m, we changed RCTViewManager.m to use RCT_CUSTOM_VIEW_PROPERTY instead of RCT_REMAP_VIEW_PROPERTY, and set the view.shadow there.
We decided to keep the original approach since the result is the same, it's simpler and there will be less diffs between React Native macOS and React Native Core. If there are further issues with shadow rendering consistency this could be something we revisit.
📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.
DetailsCATEGORY may be:
General
macOS
iOS
Android
JavaScript
Internal (for changes that do not need to be called out in the release notes)
TYPE may be:
Added, for new features.
Changed, for changes in existing functionality.
Deprecated, for soon-to-be removed features.
Removed, for now removed features.
Fixed, for any bug fixes.
Security, in case of vulnerabilities.
MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please select one of the following
Summary
Pull request #1227 mostly fixed the consistency issue but had a problem with the shadow opacity. There isn't a shadowOpacity property on NSShadow, the shadow opacity from the layer needed to be passed into the alpha channel of the NSShadow shadowColor property.
Fixes #824
Test Plan
This fix was tested with the fluentui-react-native tester app.
Before:
before.mov
After:
afteropacityfix.mov