-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cart/Checkout: Adjust styling of product attributes so that they are visually distinct. #52607
Conversation
Hi @senadir, Apart from reviewing the code changes, please make sure to review the testing instructions and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. You can follow this guide to find out what good testing instructions should look like: |
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
Size Change: +17.2 kB (+0.34%) Total Size: 5.07 MB
|
@wavvves bump on this if you're around today 🙇🏻 |
@@ -18,7 +19,7 @@ | |||
display: inline-block; | |||
} | |||
|
|||
.is-large:not(.wc-block-checkout) { |
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.
Seems this was on purpose, any idea why it was set that way?
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 idea to test subscriptions. The checkout and cart styles have departed a lot since then so I didn't think too much of it not being needed anymore.
Subscriptions is an extension though right, shouldn't any CSS fix for that specific case belong in that plugin?
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.
Original PR (woocommerce/woocommerce-blocks#3665) does mention WCSubs, I will test against that tomorrow
@elizaan36 When you have a chance would you mind having another look? I've matched the design spacing and font sizes as best I can (our We don't have the new badge design so still the spacing between product image and description are different. I'll see if I can pick that up in a cooldown sometime, because the badge really cleans up the look of the area too. Anyway here's how a product with attributes looks: |
Really nice @samueljseay thanks! I still think we need to capitalize the first letter of the attribute category if possible. |
@pmcpinto for visibility. The order summary section ends up being a long list of bolded, important elements drawing attention from the main CTA, placing the order. There's already a lot of divergence between WooPay and Blocks pdpOw2-4Df-p2 that Nikki was attempting to converge and to address mobile optimization. Can we regroup before progressing any further? Shopify Benchmark - note the lack of even needing attribute labels. ShopPay Checkout (Yes it's identical to the store guest checkout order summary AND the order summary in the TY page). |
@pierorocca I agree it would be good to overall optimize the design, especially I like the shopify concept of removing the attrtibute labels |
It makes sense right? If the word shown is "Green", the label "Color" is redundant. I appreciate how mobile optimized and to the point their summary is. Before Nikki left we were discussing a mobile first approach which led to the size comparisons in the linked P2. |
Woo’s case differs slightly from Shopify, as our users have more flexibility to create attributes and variations, unlocking more bespoke and niche situations. Still, I think having the attribute label can be redundant in most situations. Cc @elizaan36 in case you have some suggestions on displaying the attribute value without the label. PS: it will be interesting to see if Shopify’s expansion of their product model with more complex attributes and higher number of variations will impact their order summary. |
Different ways to approach this. Certainly needs a full exploration, which is what I'm advocating for here before we make changes to a very important component. |
@pmcpinto seems the advocation here is to put a hold on these changes, so I'd just be keen to know what you think are best steps forward here, I don't have strong feelings either way. 🙇 |
@samueljseay TT5 design is in flight which I see already has some changes to the order summary sections, there's a new checkout designer that started yesterday, and there are changes coming to how we approach the checkout design in general so I'd recommend holding off. |
I agree that putting this work on hold is the best option. Thanks. |
Thanks folks! Closing this for now 🙇 |
Thanks y'all. I'm going to put this on the list of first projects for Yoann to take a look at. |
Changes proposed in this Pull Request:
Closes #42022
name
element.How to test the changes in this Pull Request:
This is a visual change and also impacts cart, so check that it hasn't introduced any visual regressions or issues in either cart or checkout.
Before:
Mobile:
Desktop:
After:
Mobile:
Desktop:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Cart/Checkout: Adjust styling of product attributes so that they are visually distinct.
Changelog Entry Comment
Comment