-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Product Cards] Add image masks #2302
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.
Feature and approach look good to me! A few misc comments and questions.
I hope we could bring this to these other sections next:
|
We've had a number of followup discussions about this PR following the thread here. The primary sticking point now just seems to be the way that the shapes interact with existing global settings. I pushed a new approach to getting shadows working as expected, and here's an overview of the way the PR stands today:
In general, I think this behavior is alright. The only setting that's thrown out entirely is the border, but we're adding a warning about that. Otherwise, things look pretty good in most reasonable use cases. Please give this PR another test, and let me know if you agree (or if you don't!). I appreciate everyone sticking with this. 🙌 |
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.
Sweet, glad we don't need to ignore as many global settings now. But I guess we reversed our decision to move the shape setting into the card settings? Otherwise gtg 👍
Yeah, the main benefit of making this global would've been to allow merchants to "clean up" the appearance if it looked broken. But with the last couple commits, I think it's less likely to look broken. 😄 |
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.
LGTM 🚀
I'm ok with the compromise with product card global settings if y'all are agreed on this approach. It feels right to me - as a merchant if I was setting an image shape I would not want a border or background if card style is standard.
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.
LGTM!
Below are the test scenarios I went through. Everything seems to be working as expected and the inclusion of the label to highlight that the border gets overridden by the Image Mask sounds clear enough to me.
- Mask types
- Arch
- Blob
- Chevron left
- Chevron right
- Diamond
- Parallelogram
- Round
- Global Settings
- Corner Radius
- Border (Card or No Mask only)
- Shadows
- Card Type: Standard
- Card Type: Card
- Sections
- Featured Collection
- Collection -> Product Grid
- Search Results
- Related Products
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.
Still lgtm
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.
Actually.. sorry @kjellr, one more thing 😬 Were section onboarding states discussed at all? Noticed that these don't receive any shapes currently. It's probably fine if you wanted to handle this in a fast follow PR, but I do think we should address it.
☝️ This is also how any product card for products with no images looks like. I assumed it was intentional, as there is no image in these cards for a shape to be applied to. We can revisit whether or not the onboarding states should include a demo image, but just like |
Thanks for the heads up! That's right, the lack of shape in text cards was intentional. The feature is framed as an "image mask", so if there's no image, no shape is applied. It's a little unfortunate that the control won't have an effect when the start state for the section is applied — I can open a followup issue after merge to look at it. |
It's definitely unfortunate if a theme using shapes intentionally as part of its art direction might not show the shapes in its initial state, butwould accurately portray other card styles. I know they're technically text cards in this state, but we certainly intend to inform how the cards will actually display with the onboarding states. I feel like (and hope) products without images are super rare irl. I'm good to proceed for now 👍 but we'll probably want to circle back with Meli and get her thoughts on it if we're inclined to keep things this way. |
Yeah — even setting this specific issue aside, I feel like having an image preview in there by default would be a nicer experience. Most merchants have product images, so it'd feel more appropriate in general. We'll sort out something that works. |
* shopify/main: Snippetize header menu components (Shopify#2401) Release bump shadow fix (Shopify#2469) Update 1 translation file (Shopify#2459) [Product Cards] Add image masks (Shopify#2302) [Feature] reveal on scroll animations (Shopify#2367)
* Initial commit * Add more transforms, improve blob shape. * Add lots of blob shapes. * Fix filtering. * Remove image filters. * Remove filter code. * Fix circle shape. * Animate circle hovers. * Add additional hovers. * Animate all the blobs. * Fix standalone image with text blob. * Stop applying scale hover for cards when blobby. * Cleanup, add arch style. * Animate the arch. * Move svg renders out of theme.liquid. * Use all masks in all places. Tidy up. * Ensure svg is hidden from view. * Fix name of circle shape. * Add warning about shadow and border. * Fine-tune the blob shapes. * Tidy up the shapes used. * Respect prefers-reduced-motion * Fix typo. * Match circle animation to the image zoom. * Alphabetize options. * Remove border clipping for image banner shapes. * Round blob coordinates down to three decimal places.. * Remove Image with Text hovers. * Rewrite the helper text to be more positive. Co-authored-by: melissaperreault <melissa.perreault@shopify.com> * Add settings for Search & Product Collection * Add settings for related products. * Remove label about border and shadow. * Remove unused settings schema entry. * Blobs > Blob * Remove borders from shapes. * Remove unnecessary specificity from card shape rules. * Use consistent card hover selectors. * Remove unnecessary rules. * Use a shared class approach for applying shapes. * Remove unnecessary rule. These styles aren't currently applied to the collage section at all. * Remove extra space. * Use a generic image_shape schema to avoid duplication. * Reinstate the grow-on-hover for blob product cards. * Fix background color for text-only cards. * Use "Round" instead of "Oval" and "Circle" * Fix Image with Text labels. * Try fixing the circle clipping. * Try fixing interactions with global container settings. * Refector last commit, fix tiny bugs. * Cleanup * Remove image with text effects from this PR. * Move mask-blobs into a CSS file. * Fix card--shape class conditional. * Fix the image_shape description. Co-authored-by: Lucas Lacerda <37168033+LucasLacerdaUX@users.noreply.github.com> * Use a unique id naming convention for the Arch shape svg. * Update 20 translation files * Get shape shadows working. * Add warning about borders not working. * Update 17 translation files * Update 3 translation files --------- Co-authored-by: melissaperreault <melissa.perreault@shopify.com> Co-authored-by: Lucas Lacerda <37168033+LucasLacerdaUX@users.noreply.github.com> Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
PR Summary:
Adds seven image masks to product cards (Arch, Blobs, Chevron Left, Chevron Right, Diamond Parallelogram, Diamond, Round).
Examples:
product-cards.mp4
Why are these changes introduced?
These shapes introduce more design possibilities to our merchants.
What approach did you take?
All of these are implemented using CSS
clip-path()
. The Arch style is created using an SVG mask, while the others are powered by simplepolygon()
andcircle()
CSS functions. Many shapes include simple, pure-css hover animations.The most complicated of these implementations is the Blobs shape. There are six total blob shapes, and the PR uses
nth-child()
to approximate a random-ish pattern when they're arranged in a product grid. The blobs have been implemented usingpolygon()
instead of using simpler SVG paths, because SVG paths cannot be animated natively with CSS. This approach works well — the main downfall is thatpolygon()
supports points but not curves, and blobs are made of curves. To avoid the shapes looking pointy at large sizes, each blob has 120 points. This gets a little verbose. I've assigned each blob shape to a CSS variable to compensate and maintain code readability.Other considerations
prefers-reduced-motion
preferences.Decision log
Visual impact on existing themes
These shapes are off by default, and are optional. I don't expect any issues using them on other Dawn-based themes.
Testing steps/scenarios
Checklist