[RNMobile] Social Icons: Ensure inactive icons are visible with block-based themes #55398
Conversation
This file contains 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
SiobhyB
added
the
[Block] Social
|
Size Change: 0 B Total Size: 1.69 MB ℹ️ View Unchanged
|
SiobhyB
added
Mobile App - i.e. Android or iOS
|
Flaky tests detected in a725f24. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6612177137
|
|
|
Thank you for working on addressing this issue @SiobhyB ! I see you mentioned:
But what if we just use the active background but with opacity for inactive states? Similar to what the web editor does: 
Have we considered that? It's just an idea, having this approach works too! |
|
Something like:
|
@geriux, this is a great idea, I much prefer the look from your screenshots. cc-ing @osullivanchris, @paulforgione for a confidence check on the best approach, too. |
|
@SiobhyB @geriux I like using the background color with opacity. It looks good in the first example. I am a bit confused by the 4 screenshots shown in the second example. To me, only the light mode screenshot looks like it has inactive buttons? The others all look fully active, unless I've missed something. |
I think it's because I didn't add opacity to the icons and just their backgrounds 😅 , but ideally the opacity should be applied to both the icon and the background. Something like:
|
Yeah that looks like what I would expect! |
added 2 commits
October 23, 2023 11:46
As per the feedback here, we've decided to reduce the opacity of inactive icons, rather than setting custom colors: #55398 (comment) This change aligns with the web's behaviour.
|
@geriux, @osullivanchris, thank you both! I've gone ahead to make those changes and updated the screenshots in the PR's description. I set the opacity to |
|
@SiobhyB looks good to me! |
geriux
approved these changes
Oct 23, 2023
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Fixes wordpress-mobile/gutenberg-mobile#4780, by ensuring inactive icons within the Social Icons block are always visible within the editor.
Why?
There are currently certain scenarios where inactive icons in the block are invisible. This happens if a block-based theme with a light background is active, as the native editor displays the light background in contrast with the block's dark mode styles.
How?
The logic associated with inactive icon styles has been updated so that, instead of using custom colours, we now always use a reduced opacity. This ensures the inactive icons are visible across all types of themes, and also matches the web's behaviour. Further discussion on this approach can be found in the comments here: #55398 (comment)
Testing Instructions
Testing Instructions for Keyboard
N/A, solely a visual change.
Screenshots or screencast
Dark mode
Light mode