-
Couldn't load subscription status.
- Fork 216
Removing frontend code related to PRBs from the checkout page #4687
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
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.
@wjrosa We can not remove the files under client/entrypoints/payment-request-settings/*.
These are the codes for the customization page for the express checkout. As the admin settings were exactly the same for PRB and ECE (except for the buttons preview), we did not create new duplicate code for ECE.
We should transfer these settings related code under client/entrypoints/express-checkout-settings/* and load the correct assets.
|
Thanks for the heads up, Mayisha! I started to rename the files, but I got into a rabbit hole. Then I decided to revert this part to handle it in another PR later. |
|
Okay, I gave it another try, and now it works. I renamed all the frontend files 👍 . I will leave some stuff, like the global variable, for another time. |
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.
Looks good to me.
✅ Settings page is loaded and the changes made to the express checkout settings persist on save.
✅ Express checkout buttons are loading correctly on all the pages,
- Product
- Shortcode cart
- Shortcode checkout
- Block card
- Block checkout
✅ Appearance of the buttons follow the saved settings.
✅ Can pay using Google Pay without any errors.
| public function admin_scripts() { | ||
| // Webpack generates an assets file containing a dependencies array for our built JS file. | ||
| $script_asset_path = WC_STRIPE_PLUGIN_PATH . '/build/payment-requests-settings.asset.php'; | ||
| $script_asset_path = WC_STRIPE_PLUGIN_PATH . '/build/express-checkout-settings.asset.php'; |
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.
Note to remember, we should rename this controller when we remove the backend side of the code.
|
@wjrosa, I will continue testing this later today, but how should we test the case where someone previously had payment request buttons enabled in an earlier version and then upgrade to this version? Do we have a PR that ensures that path is handled correctly? In my mind, that is the main test path for this, as express checkout has been the default for some time. |
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.
I am going to test some more tomorrow, but I think we need to ship the back end changes before we ship this change, as we could get really weird behaviour in cases where the back end still enables the feature and/or tries to queue up the removed JS files.
Co-authored-by: daledupreez <[email protected]>
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.
The changes look good to me and are working well. My only note is that I think we should merge #4710 first or soon after this change, as setting the _wcstripe_feature_ece option to 'no' successfully broke this PR. 😁
Towards STRIPE-724
Depends on #4710
Changes proposed in this Pull Request:
As part of the removal of the legacy checkout, we are also removing code related to Payment Request Buttons (PRBs), which were deprecated along with it.
Another motivation to remove it now is to make our frontend code compatible with the latest version of
@wordpress/scripts. A PR upgrading it can be seen here.Here I am removing only the frontend files. The PR removing the backend part can be seen here.
Testing instructions
update/removing-prbs-frontend-code)Changelog entry
Changelog Entry Comment
Comment
Post merge