This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Fixes CSS spacing and availability issues for breadcrumb, catalog sort, and result count blocks. #8391
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
nerrad
added
type: bug
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: +68 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
nerrad
force-pushed
the
fix/remove-extra-margin-product-results-block
branch
from
8de3f23 to
0b028bc
Compare
kmanijak
reviewed
Feb 13, 2023
There was a problem hiding this comment.
Great catch and thanks @nerrad for working on this!
I spotted the same and also included the fixes within the bigger PR:
Given that I worked on that and that I left two minor comments, please let me know in case you'd prefer me to take it over. I can either:
- address the comments in your PR,
- include the missing Breadcrumbs fix into my PR,
- if you prefer to finish it yourself, please let me know and I'll remove the fixes from my PR so there's no conflicts. 👍
Comment on lines
13
to
17
| .woocommerce .wc-block-catalog-sorting { | ||
| .woocommerce-ordering { | ||
| margin: auto; | ||
| } | ||
| } |
There was a problem hiding this comment.
It would be great to move the Catalog Sorting styles to the respective style file: https://github.com/woocommerce/woocommerce-blocks/blob/trunk/assets/js/blocks/catalog-sorting/style.scss
There was a problem hiding this comment.
Oh yea definitely, my bad 🤦🏻 will do.
Comment on lines
19
to
24
| .woocommerce.woocommerce-shop .wc-block-breadcrumbs { | ||
| .woocommerce-breadcrumb { | ||
| margin: auto; | ||
| display: block; | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar to the above, it could be moved to Breadcrumbs style file:
https://github.com/woocommerce/woocommerce-blocks/blob/trunk/assets/js/blocks/breadcrumbs/style.scss
nerrad
force-pushed
the
fix/remove-extra-margin-product-results-block
branch
from
0b028bc to
78ab031
Compare
Good catch on the reviews about the implementation of the style fixes. I had done the initial iteration quickly to verify fixes and forgot about moving them into the right files. I think it's beneficial to do the style fixes in their own PR to help keep PRs smaller and more precise. |
kmanijak
approved these changes
Feb 13, 2023
nerrad
force-pushed
the
fix/remove-extra-margin-product-results-block
branch
from
78ab031 to
acd3faf
Compare
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.
While testing the implementations of the Store Breadcrumbs, Catalog Sorting and Product Result Counts block, I noticed that on some themes the margins were causing some spacing alignment issues. I also noticed that the Store Breadcrumbs block was not even showing on some pages (Shop page) because of some theme specific rules shipped with WooCommerce core.
Below screenshots are on the Twenty Twenty Two theme.
Store Breadcrumbs
Misaligned Margin
Breadcrumbs not even displaying for shop page.
While it appears okay when editing the Product Catalog template, it is not shown on the frontend for certain themes. It surfaced with Twenty Twenty Two (or derivatives of that theme) for me.
Catalog Sorting Block
Product Results Count
Other Checks
Testing
Automated Tests
User Facing Testing
The issues are primarily visible with the Twenty Twenty Two theme. However, testing should be done against some other themes to make sure this doesn't introduce significant issues with those themes. In theory, it shouldn't because the CSS for the blocks should appropriately inherit what the theme provides for default margins and spacing in the container around the blocks.
Appearance->Editor).sitedomain.com/shopon most WP installs) shows all blocks aligned as in the above screenshots.WooCommerce Visibility
Performance Impact
Changelog