Remove background color from Express Checkout title #2704
Conversation
Size Change: +147 B (0%) Total Size: 1.57 MB
ℹ️ View Unchanged
|
margin-left: $gap-small; | ||
pointer-events: none; | ||
flex-grow: 1; | ||
} | ||
} | ||
|
||
.wc-block-components-express-checkout__title { | ||
flex-grow: 0; | ||
padding-left: $gap-small; | ||
padding-right: $gap-small; |
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.
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.
We had two selectors with the same specificity competing. The issue happened because depending on the build mode, stylesheets were loaded in a different order, so the styles applied where different.
If this is a style loading issue, should we document this in an issue for further investigation?
Created an issue: #2713.
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.
I tested in Chrome and Safari and this looks good 👏
} | ||
|
||
&::after { | ||
border: $border-width solid transparent; |
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.
Are these pseudo elements used to create the "padding" around the title (i.e. overlay the other express payment border? Might be worth a little comment explaining the intention/approach, or maybe package as a mixin (or class) if that's practical.
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.
Good point. Added a comment: 2b6f524. I don't think we need to create a mixin for it yet considering we are only using it here. We can extract it in the future if we see there are other instances where we use it.
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 the visual explanation below, ingenious fix!
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.
I gave this a quick test & review - looking good.
Note I don't have any express payment methods configured, so I can't really test this.
It's tricky to get control of the whitespace around the heading when there is the express payment box behind! I'm keen to better understand the technique you've used :)
@haszari Yeah, I wasn't even sure it was doable at all, but then I found a way. Basically the border is composed by the border of three elements:
Hope this explanation makes sense. 🙂 The CSS solution was a bit tricky, but I don't think it will make it more difficult for themes, because border color is inherited from the text color, so it will change all at once. |
Fixes #2659.
Screenshots
Before:
After:
How to test the changes in this Pull Request:
Express checkout
title doesn't have a background color different from the rest of the page and has left and right padding (so it doesn't collide with the border).Express checkout
andOrder summary
titles are aligned.Express checkout
box border is 1px wide, like in the new designs.Changelog