[Mobile] - Support background colors for Group block #25994
Conversation
geriux
added
[Status] In Progress
|
Size Change: +742 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
|
Hey @iamthomasbishop @kyleaparker 👋 I've made a build with the ongoing work to support background colors for Group block, just in case you want to test some of the templates. It also has the latest full-width changes (except Columns). |
|
Nice, this is working well for me! |
|
@geriux This is looking solid, nice work! I found one issue, but I'm not sure if it's related to the changes you've made — I'm guessing it's specific to recent work done on the BottomSheet component. The issue is on block settings sheets; when you open them, they get stuck at ~40-60px height. Here's a screenshot:
And fwiw, I'm only seeing it happen on these blocks (and not only in the context of Group block):
|
|
I just noticed after making some edits on a page with this build, it starts to give me these errors on the desktop editor: The page was working fine on the web immediately before I made changes in the mobile editor. |
Oops, forgot to disable that 😅. The issue you're seeing is that it's trying to show the Thanks for testing @iamthomasbishop! |
There's an ongoing issue with the Columns block and it's included in the build I created so sorry for that, hope you were able to recover your changes 🤞 Once this issue is solved I'll do more testing to double check is not giving any issues between the app and the web editor. Thanks for testing @kyleaparker! |
|
@iamthomasbishop The issue is now solved, I added the Color picker to the blocks that support text/background colors:
The build now includes this just in case you wanted to try it out =) |
|
This looks great! Not sure why, but I didn't realize we were also adding color support for non-Group blocks — so exciting! Are we planning on adding some padding to any block with
As a side note, playing around with this makes me think we should add an explicit default color button on the color sheets so the user can "reset" back to the default. Perhaps at the bottom right of the sheet like so? // cc @lukewalczak
|
|
Hey @iamthomasbishop 👋
Well, when I activated it for Groups I saw those were working too so I didn't limit this feature for Groups just yet. I just added paddings for those as well, can you check and let me know if it works as expected?
Yeah I saw we might need that when I was testing and I couldn't remove any colors =D what do you think?:
Thanks for the feedback and for testing! I updated the test build with the latest changes. |
|
@geriux Thanks for the updated build and the quick turn-around!
Positioning looks good, but I think "Reset" or "Use default" for the label might be more appropriate. I also think we might want to keep the user on that same sheet after they've pressed the clear button, although that might just be a personal preference.
Paddings work as I would expect. Nice work! One thing I wanted to raise — I am concerned about the side-effects WRT light vs. dark modes. For example, let's say I have dark mode enabled. If I change the background of a Group to dark gray, it'll look fine in the editor in dark mode because the base text color is white. However, if I preview it, I will see that the output is black text because the theme's base text color is black. Here's what I mean:
I think if we had a mechanism to calculate color contrast and adjust the color of an element based on the ratio between it and another element's color (i.e. text color vs background color), we would be in great shape. But as it stands, we'll be stuck in the middle — and I'm not sure if it's a good idea to ship in this state. Another thing that might help is if we can read and apply the theme's base color values (likely with global styles?), but until then we'll need to figure out a way to support both light and dark modes dynamically. I believe the web has a mechanism for auto-converting the text color based on the color behind it, but they also have a color contrast mechanism that alerts the user when the color contrast ratio between two colors isn't sufficient — perhaps we can utilize these in some way? |
|
I've been trying to edit some of the homepage layout designs on this build to see how they look with the new colour settings and the page seems to freeze each time, I have been going back and forth between the mobile and web editor so it may be related to that issue? The app is also crashing when I try to add the spacer block to any post/page. I can send more details about the content on the homepages if you need :) |
|
Hey @iamthomasbishop thanks for giving it another test 👋
Sounds good! I'll change it to "Reset" and keep the user on the same sheet.
Yay!
The thing is we are still missing information from the theme like the default text color, or background. What we currently have is all of the colors the theme uses but even with that, we don't really know what color to pick. Of course, we can compare the background color with the colors from the theme and pick the light color if the background is dark but it might have different results from web. What web does is just using the CSS classes from the theme and applying the text colors for each background. As of today, we don't have a way to get that. Regarding the color contrast mechanism, we could use that but we need both background and text color to compare. If the user doesn't select a text color we wouldn't be able to know if the background color is right for the theme text color. If we don't want to ship this like this, we could disable the color picker unless a block already has colors set. So for layouts, they could tweak them. But if they create a new block there won't be an option to customize colors. To be honest (Light mode / Dark mode) will always be an issue with blocks with custom colors =( |
|
Hey @kyleaparker 👋
So sorry for that, we had recent issues that broke some blocks. I'll update the build with the fixes. Thanks for testing! |
|
@geriux thanks for the context, really great info.
That's a good point. I hope in the future (via global styles, perhaps) we will be able to receive some registered color properties straight from the theme, but until then it seems like we're stuck.
I think that's an option (albeit a "nuclear" one), but let's try to think about some alternatives before going there.
We can move this discussion to another ticket if necessary, especially if we decide on a more robust approach, but I wanted to note a couple of ideas that might be worth considering in case it sparks ideas for a quick solution. Some of these are straight forward, and others conceptual departures from the current approach, but we need to come up with something until we have global styles or similar access to the theme's various properties.
// cc @maxme any ideas re: ⬆️ ? |
|
Hey @iamthomasbishop 👋 I created a new build with the fix for the extra bottom padding in Group blocks with a background color. Let me know if you find any other issues. Thanks for testing! |
|
Thanks @geriux! That padding issue looks to be fixed! 👍 |
|
Hey @dratwas 👋 I've updated the code to disable the Color settings for these blocks, so this PR will only focus only on rendering the colors. Would you mind giving it another review when you can? I also created new builds to test. Thank you! |
Gutenberg Mobile PR-> wordpress-mobile/gutenberg-mobile#2703WordPress iOS PR-> wordpress-mobile/WordPress-iOS#15079WordPress Android PR-> wordpress-mobile/WordPress-Android#13279Part of wordpress-mobile/gutenberg-mobile#2195
Description
This PR adds rendering colors support for:
How has this been tested?
Steps to test: Group/Paragraph
Steps:
From the web editor
Or use this code:
Steps to test: Quote
Steps:
From the web editor
Note: You won't see the colors applied to the Quote block in the web editor
Or use this code:
Screenshots
Types of changes
New feature
Checklist: