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
Try wrapping entire coming soon page under cover block to fix layout issue #46914
Conversation
flex-direction: column; | ||
justify-content: space-between; | ||
} | ||
.woocommerce-coming-soon-banner-container > .wp-block-group__inner-container { |
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.
This is required for classic themes, wp-block-group__inner-container
appears out of nowhere 🤷
.woocommerce-coming-soon-header, .coming-soon-cover .wp-block-cover__background { | ||
background-color: #bea0f2 !important; | ||
.woocommerce-coming-soon-header { | ||
height: 40px; |
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.
Small hack: Fixed header height in order to ensure small modifications will keep the center part relatively stable
Hi @psealock, @rjchow, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
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.
This is working nicely @ilyasfoo.
I have a bit of a refactor currently under review as well which will cause some conflicts. Do you want to give it a shot here or after merge?
} | ||
.woocommerce-coming-soon-header, .coming-soon-cover .wp-block-cover__background { | ||
background-color: ${ color } !important; |
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.
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 catch!
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.
Fixed in c471e52
Merge remote-tracking branch 'origin/trunk' into fix/lys-attempt-using-cover-block-fix
@psealock Thanks! I've resolved conflicts without issues
Edit 2: Hmm, false alarm. Tested again in a fresh JN and no issues found. This PR is ready for review! |
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.
This is looking great @ilyasfoo, testing well and code changes look good 🚢
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.
Thanks @ilyasfoo. Nice fix. 🚢
Submission Review Guidelines:
Changes proposed in this Pull Request:
Another attempt to partially fix layout issues from https://github.com/woocommerce/team-ghidorah/issues/311 by wrapping everything under the cover block and flexbox positioning instead of using absolute positioning.
Note that this does not yet fix the styling issues (such as changing background color, default margin/padding mismatches, etc.)
How to test the changes in this Pull Request:
launch-your-store
feature flag.WooCommerce > Settings > Site Visibility
and enableComing soon
checkbox and save.Appearance > Editor > Templates > Page: Coming soon
and edit it.kCrgRV.mp4
Changelog entry
Significance
Type
Message
Comment