-
Notifications
You must be signed in to change notification settings - Fork 24.6k
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
[iOS] Fix semi-transparent backgrounds on Text components #23872
Conversation
the test failing seems unrelated. e2e test for DatePickerIOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
The problem does exist, but that's not a proper place to fix it. We should not apply background color for outer VirtualText.
To be sure, create a test with nested background like this:
<Text style={{backgroundColor: '#00000020', padding: 10}}>
Text in a gray box!
<Text style={{backgroundColor: 'red'}}>another text in red box</Text>
</Text>
I pushed changes to reflect (I think) what you said, but it breaks other places in the RNTester app. If I understand you correctly, it should look like this:But this has undesired effects when text doesn't have children:So should the solution be to allow the background to be drawn so the after image works & so the nested backgrounds work? |
@shergin fixing the snapshot now, changing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This pull request was successfully merged by @ericlewis in bbf7156. When will my fix make it into a release? | Upcoming Releases |
Summary
Fix #23849. When setting a semi-transparent background on text, it becomes obvious that we are drawing the background color twice. Since background color is handled by the view, we should not need to draw the glyph background color too.
Changelog
[iOS] [Fixed] - Semi-transparent backgrounds on text
Test Plan
Added a new section to iOS RNTester Text Examples, CI passes. Also confirmed the TextExample snapshot tests passed with this change before I added the new test & snapshot.