-
Notifications
You must be signed in to change notification settings - Fork 219
Add color and typography styles on the mini cart title block #9382
Add color and typography styles on the mini cart title block #9382
Conversation
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.
This is looking great, @daniloparrajr, thank you so much for contributing with this PR.
It's testing as expected and code changes look good. My only comment would be that I think we should disable support for background color (sorry, I know that wasn't specified in the original issue 🙏 ). The reason is that I feel it looks a bit off because it has some spacing around it (padding and margin from other elements):
For now, I think being able to change the text color and font size would be enough.
@Aljullu How did you manage to see the background color? On my end, both when using TT2 and TT3, I do not see the background color, as the following SCSS definition overwrites that setting on my end:
|
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.
Thanks for working on this, @daniloparrajr.
Besides @Aljullu feedback, could you add testing steps to the PR? In #8905, you can find a great example of testing steps. Adding them, does not only help for testing the PR now, but also later when it gets merged into core. 😀
Changing the status from "Request changes" to "Comment"
Thank you @Aljullu and @nielslange for checking my PR, I added the change to disable the background color support for the mini cart title block. I also added some testing steps, I hope this is helpful. |
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.
Thank you @Aljullu and @nielslange for checking my PR, I added the change to disable the background color support for the mini cart title block. I also added some testing steps, I hope this is helpful.
Thanks for updating the PR and adding testing steps, @daniloparrajr. This is looking great. I will wait until tests pass and after that I will proceed merging. It there aren't any followup issues, your change will be included in WC Blocks 10.3 and WC core WC 7.9. 🎉
How did you manage to see the background color? On my end, both when using TT2 and TT3, I do not see the background color, as the following SCSS definition overwrites that setting on my end:
@nielslange Which color did you choose? When choosing one from the palette, it comes with an !important
tag in the CSS, so it would override the default background. But maybe custom colors don't? Sidenote: this shouldn't block this PR as we have removed support for background colors.
The goal of this PR is to make the Mini Cart Title customizable through the settings sidebar (colors, typography).
In which the two inner blocks
Mini Cart Title Label
andMini Cart Title Items Counter
will inherit by default.Fixes #9346
Testing
Mini Cart Title
block and add some customizations on the settings sidebar (colors, typography).Screenshots
WooCommerce Visibility
Performance Impact
Changelog