Update the border colors in the Cart and Checkout blocks #11474
Conversation
tarunvijwani
changed the title
tarunvijwani
force-pushed
the
update/11354-cart-and-checkout-blocks-colors
branch
from
3fe469b to
4dbd5df
Compare
|
This PR is on hold. There were concerns about user experience with light border colors. Internal discussion: p1698400336133709/1698387117.246639-slack-C8X6Q7XQU |
|
We decided to continue with $universal-border-light color for borders. |
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: -214 B (0%) Total Size: 1.61 MB
ℹ️ View Unchanged
|
tarunvijwani
force-pushed
the
update/11354-cart-and-checkout-blocks-colors
branch
from
908ea3e to
8d4e0cd
Compare
tarunvijwani
added
the
type: enhancement
tarunvijwani
changed the title
woocommercebot
requested review from
a team and
mikejolley
and removed request for
a team
|
Hi @elizaan36, could you take a moment to review the PR? I've included a screen recording for reference. Just wanted to let you know that I added |
There was a problem hiding this comment.
Hi @tarunvijwani I'm trying to figure out why, but the border color in the implementation is too light and almost disappears on the screen.
It looks like the correct value from the code, but when I compare it visually to the Figma it's much lighter.
Why do you think this is?
Also for the $universal-border-dark variable, could we still use the RGB value for consistency (17, 17, 17, .80) ? Rather than 0/0/0.
@elizaan36 I think that's because the rgba value is not correct. We're using rgba(17, 17, 17, 0.115); which is #e7e7e7 on white. However, on Figma we're using #E0E0E0; Would you be able to help me with the correct rgba value for it?
|
|
No problem, I'll screenshot what I have now in Figma. For the 
Sorry for the confusion there as those Gutenberg values slipped into the mocks by accident. Hopefully this nails it, and I'll spend some time cleaning up the Figma. One last thing. I noticed that the borders between the section numbers aren't following the same color system. Can we update those borders to the 
|
|
Thank you, @elizaan36, for the color code.
I noticed form borders had opacity that's why they were appearing more lighter than the actual color. I have now removed the opacity from them. Here is the updated output: 1080.mov |
|
I think we overcorrected a bit considering the additional transparency you mentioned, so tweaking the color one more time to the lighter rgba (17, 17, 17, .12) or even rgba (17, 17, 17, .11) would work. |
|
@elizaan36 as we discussed over slack, I have rolled back the $universal-border-light value to rgba(17, 17, 17, 0.115). Here is the updated video: Storefront themeStorefront.movTove themeOther.theme.mov |
|
The border color looks perfect now. Thanks, @tarunvijwani 🙏 |
| content: ""; | ||
| display: block; | ||
| height: $universal-border-radius - $border-width; | ||
| margin-right: $gap-small; | ||
| opacity: 0.3; | ||
| opacity: 1; |
There was a problem hiding this comment.
Are these values needed? 1 should be default.
There was a problem hiding this comment.
@mikejolley While testing the PR, there were some instances where opacity was overridden to 0.3 again. I added this manually to make sure it remains the same and also doesn't inherit the theme opacity styles.
There was a problem hiding this comment.
I think you should investigate where its coming from and fix there. It could be a mixin like with-translucent-border.
There was a problem hiding this comment.
@mikejolley Thank you for the suggestion! I have passed opacity argument in the with-translucent-border as it accepts opacity and assign it to the psuedo element:
@mixin with-translucent-border($border-width: 1px, $opacity: 0.3) {
position: relative;
&::after {
border-style: solid;
border-width: $border-width;
bottom: 0;
content: "";
display: block;
left: 0;
opacity: $opacity;
pointer-events: none;
position: absolute;
right: 0;
top: 0;
}
}
|
Moving to 11.6.0 |
tarunvijwani
added
team: Rubik
and removed
needs: design
|
@elizaan36 Thank you for the confirmation. |
tarunvijwani
added
focus: global styles
There was a problem hiding this comment.
Thanks for working on this, @tarunvijwani. I've been reviewing the contrast aspects in line with our discussions and the WCAG guidelines. While the Text Contrast guidelines focus on text elements, the Non-Text Contrast guidelines also emphasize the importance of sufficient contrast for UI components, including checkboxes and radio buttons.
I noticed that in the default styles of the Twenty Twenty-Four theme, checkboxes and unselected radio buttons have a contrast ratio of 1.86:1, which is below the recommended 3:1 ratio. This could impact users with visual impairments. These are the UI elements I'm referring to:
Would it be possible to adjust these elements to align with the 3:1 contrast ratio as suggested by the guidelines? This enhancement could significantly improve accessibility for all users.
cc: @elizaan36
|
Increasing the contrast of the border color for the checkbox and radio buttons could lead to an inconsistent look. However, we should consider the WCAG guidelines. I will wait for @elizaan36 to provide her suggestions. |
|
We can bump the $universal-border-medium to The increase in opacity passes in my contrast checker, let me know if that passes on your end. |
Thanks for considering this change, @elizaan36. I want to clarify that my intention isn't to block this PR. I'm simply very passionate about a11y and strive to ensure that all UI elements are equally accessible. |
- Change checkbox and radio button colors from rgba(25, 23, 17, 0.3) to rgba(25, 23, 17, 0.48).
|
Thank you, @nielslange and @elizaan36! I have updated the colors, here is how it looks like on the Twenty Twenty Four theme:
|
There was a problem hiding this comment.
Thanks for working on this, @tarunvijwani. Overall, the PR looks good. I just noticed one definition that doesn't seem to do anything.
assets/js/blocks/cart-checkout-shared/payment-methods/express-payment/style.scss
Outdated
Show resolved
Hide resolved
What
Fixes #11354
Why
In this PR, we're updating the border colors in the Cart and Checkout blocks to match the order confirmation block template defaults and create a cohesive experience.
Testing Instructions
Screenshots or screencast
Screen.Recording.2023-11-16.at.3.26.38.PM.mov
WooCommerce Visibility
Required:
Checklist
Required:
[type]label or a[skip-changelog]label.Conditional:
[skip-changelog]label is not present).Changelog