This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Product Gallery: CSS styling tightening up #10867
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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
assets/js/blocks/product-gallery/index.tsx
|
Known conflict/limitationThere's one compromise in the below solution for the Due to the fact that the 
I don't see a simple fix.
|
|
Size Change: -685 B (0%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
|
Hey @danieldudzic, thanks for your great work on this PR! Now, the Large Image and arrows never overflow! However, I noticed two things that I think that we should address:
As we discussed earlier, we agreed to consolidate all styles affecting multiple child blocks into a dedicated The new |
|
@danieldudzic, I bumped this issue to the next release! |
tjcafferkey
reviewed
Sep 11, 2023
assets/js/blocks/product-gallery/inner-blocks/product-gallery-large-image/edit.tsx
Show resolved
Hide resolved
Would you mind elaborating on why we shouldn't be targeting those? It seems that a lot of block styles in our repo target the |
You're right! The wp-block-woocommerce is consumed by WordPress for global styles. I think that every CSS that we write should target a wc-block class: this increases the separation of responsibility and allows us to debug the CSS better. |
Thanks for explaining. This does make sense. |
…ocks stylesheet files
13 tasks
…gle product gallery despite of the Next/Previous mode selected
|
@gigitux We should try to get this merged ASAP, as I can see already that getting this up to date with |
gigitux
suggested changes
Sep 20, 2023
There was a problem hiding this comment.
Gorgeous work! I added some comments, but they should be easy to address! Just to clarify, it is crucial to add the wp-* class in the right place, because this class is used by the Global Styles.
Also, let's change the destination branch to trunk given that this PR includes the fix for #10750 as well.
…o 10750-product-gallery-block-different-icons-for-next-previous-block-css-layout-fix
…-for-next-previous-block-css-layout-fix
…evious-block-css-layout-fix' of https://github.com/woocommerce/woocommerce-blocks into 10750-product-gallery-block-different-icons-for-next-previous-block-css-layout-fix
gigitux
approved these changes
Sep 20, 2023
There was a problem hiding this comment.
Code-wise LGTM. I noticed some regressions that we should address, but since this PR is huge and impacts other PRs, let's merge it and continue the iteration in the next PRs. Please open a dedicated issue for each point:
Fix Zoom Animation
Currently, the zoom animation is broken:
https://github-production-user-asset-6210df.s3.amazonaws.com/4463174/269297195-f7e43dd6-736b-4be4-a0c1-2923a5891f66.mp4
Move classes to the main div and remove the additional wrapper
Currently, there is an unnecessary div that exists because we don't invoke this function anymore, otherwise, the layout is broken. We should check why this happens and fix it:
Thumbnail Position
I'm not sure about the thumbnail position:
E2E test fail
I noticed that the product-gallery-large-image-next-previous.block_theme.side_effects.spec.ts test failed. We should skip it and fix it in a dedicated PR.
cc @thealexandrelara for awareness.
…-for-next-previous-block-css-layout-fix
This was referenced Sep 20, 2023
…-for-next-previous-block-css-layout-fix
15 tasks
13 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
The goal of this PR is to ensure the
Product Galleryblock displays correctly in all setting configurations and is not dependent on the neighboring blocks.Fixes #10838 #10750
Why
This PR attempts to rectify styling shortcomings that arose from various
Product Gallerysettings being added and adding complexity.Testing Instructions
Appearance > Editor.Templates > Single Product.Product Image Galleryblock).Product Galleryblock just aboveProduct Detailstabs, so that it doesn't have any neighboring columns.Product Gallerydisplays correctly (doesn't overflow etc.) in all possible configurations of theThumbnailsPositon setting and theNext/Prev Buttonsappearance setting, both in the editor and frontend.Screenshots or screencast
WooCommerce Visibility
Required:
Checklist
Required:
[type]label or a[skip-changelog]label.Conditional:
[skip-changelog]label is not present).Changelog