Remove the override css for the express payment grid. #9248
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
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +162 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
alexflorisca
requested review from
a team and
cesarcosta99
and removed request for
a team
cesarcosta99
requested review from
asumaran
and removed request for
cesarcosta99
|
@alexflorisca, could you please fix the conflicts here? I modified one of the files you’re updating. I wouldn't delete the I suggest double-checking your changes, as I worked on the spacing and alignment of the express checkout buttons on all pages. Let me know if you still need this PR reviewed. |
|
Thanks @asumaran, fixed the conflict, left the sass file there. Is there a reason why you override the |
@alexflorisca |
|
@alexflorisca do you drill want me to review this PR? |
|
Closing this PR as @pierorocca has confirmed that we will use 12px spacing for the Express Checkout buttons |
|
Sorry for the delay in responding to this. I think it would be helpful not to have the override here. I've edited the code in Core to have a grid gap of 12px in Small updates to the express checkout area. It would be good if we can coordinate these changes so they just live in one place rather than overriding styles. Other extensions may do the same which we don't want. |
asumaran
reviewed
Aug 28, 2024
There was a problem hiding this comment.
@alexflorisca this PR breaks the spacing and alignment of Express Checkout buttons.
Removing the import makes the space to be 16px on Express Checkout buttons on the Cart Block page. I've tested with the latest WooCommerce 9.2.3.
It should be 12px as it is with the import:
And removing the CSS declaration from the client/express-checkout/blocks/express-checkout-element.scss file breaks the button alignment on the Checkout Block page.
Screen.Recording.2024-08-28.at.11.10.07.AM.mov
|
I don't think this has been released yet, I've only merged it a couple of days ago. Can you test with latest trunk? |
asumaran
requested changes
Aug 30, 2024
There was a problem hiding this comment.
@alexflorisca I've tested with WooCommerce's trunk branch this time and still found an issue.
| @@ -9,7 +9,6 @@ import { Elements } from '@stripe/react-stripe-js'; | |||
| */ | |||
| import ExpressCheckoutComponent from './express-checkout-component'; | |||
| import { getExpressCheckoutButtonAppearance } from 'wcpay/express-checkout/utils'; | |||
| import '../express-checkout-element.scss'; | |||
There was a problem hiding this comment.
| Before | After |
|---|---|
Removing the CSS import also removes the CSS rules for the Cart block page, which reverts the spacing back to 16px. So, we either fix the spacing in WooCommerce Core or leave the import for now.
There was a problem hiding this comment.
Good catch. I've left the import in. I initially remove it as the code I removed was the only thing in it!
| @@ -14,14 +14,3 @@ | |||
| margin: 24px 0 !important; | |||
| height: 20px; | |||
| } | |||
|
|
|||
| // Checkout Block | |||
| .wc-block-components-express-payment--checkout { | |||
There was a problem hiding this comment.
Removing this looks fine with trunk now that the mininum with of the grid is set to 240px by WC core. 👍
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes proposed in this Pull Request
As part of the ECE work, a temporary override was added to change the express payment grid to support larger buttons. This has now been fixed in Woo Core in woocommerce/woocommerce#49304. This PR removes the unnecessary CSS.
Testing instructions
240pxwidth.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge