-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[RNMobile][Embed block] Fix bottom sheet link color in dark mode #34424
Conversation
Size Change: 0 B Total Size: 1.04 MB ℹ️ View Unchanged
|
Will go ahead and contribute a review from my side so Carlos can focus on other PRs for the embeds blocks 👍 |
👋 @ceyhun! Using the installable build from this WPAndroid test PR, I see that the color is more bright than the "Before" case, but not the same as the "Add link" button (of the bottom sheet) you mention in the testing instructions. The new color is indeed better for dark mode, and light mode works perfect as well so, the PR looks great to me. Can you verify whether the "same as the Add link button" test should hold? Thanks! |
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.
Changes look and work fine for me, so it's a LGTM for me! I've only left one comment asking about the test against the "Add link" button color.
Thanks @hypest! I added that case based on the example image in a comment shared by @iamthomasbishop (Internal reference p5T066-2Bq-p2#comment-9741). I thought they were the same, but now I checked and the "ADD LINK" button seems to be using the
I've tried using
But this change might mean changing it in other place as well, like Audio Block's |
+1 for merging since this PR does fix the basic issue of the color being too dark for dark mode. |
Description
Fixes Embed block bottom sheet link color in dark mode
How has this been tested?
Learn more about embeds
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.js
files for terms that need renaming or removal).