Skip to content
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

Remove accordion from "Other payment providers" in WC Pay Task #37205

Merged
merged 5 commits into from Mar 16, 2023

Conversation

chihsuan
Copy link
Member

@chihsuan chihsuan commented Mar 14, 2023

All Submissions:

Changes proposed in this Pull Request:

Closes #37203.

Remove the accordion from "Other payment providers" in WC Pay Task and add proper space between sections.

How to test the changes in this Pull Request:

  1. Skip OBW
  2. Go to /wp-admin/admin.php?page=wc-settings
  3. Choose a WCPay country, such as the US
  4. Go to WooCommerce > Home > Payment task
  5. Observe that "Other payments providers" section is visible

Screenshot 2023-03-14 at 15 24 27

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?
  • Have you included testing instructions?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@github-actions github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Mar 14, 2023
@chihsuan chihsuan self-assigned this Mar 14, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 2023

Test Results Summary

Commit SHA: 218e45d

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 7s
E2E Tests189006019513m 38s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@chihsuan chihsuan marked this pull request as ready for review March 14, 2023 07:35
@chihsuan chihsuan requested review from a team, moon0326 and adrianduffell March 14, 2023 07:36
rjchow
rjchow previously approved these changes Mar 14, 2023
Copy link
Contributor

@rjchow rjchow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests well and looks good, thanks for getting to it so quickly! 🚢

probably need to rebase on the latest trunk for the pnpm fix

moon0326
moon0326 previously approved these changes Mar 15, 2023
Copy link
Contributor

@moon0326 moon0326 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on it! 👍 🚀

LGTM and tested well.

@chihsuan chihsuan dismissed stale reviews from moon0326 and rjchow via e453c90 March 15, 2023 03:03
@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #37205 (e453c90) into trunk (0cf5677) will decrease coverage by 0.0%.
The diff coverage is 32.0%.

❗ Current head e453c90 differs from pull request most recent head 218e45d. Consider uploading reports for the commit 218e45d to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             trunk   #37205     +/-   ##
==========================================
- Coverage     46.7%    46.7%   -0.0%     
  Complexity   17191    17191             
==========================================
  Files          429      429             
  Lines        64845    64865     +20     
==========================================
+ Hits         30275    30284      +9     
- Misses       34570    34581     +11     
Impacted Files Coverage Δ
...ugins/woocommerce/includes/class-wc-post-types.php 2.7% <0.0%> (-0.1%) ⬇️
...gins/woocommerce/includes/wc-product-functions.php 44.7% <100.0%> (+1.0%) ⬆️

... and 1 file with indirect coverage changes

@github-actions github-actions bot added the package: @woocommerce/admin-e2e-tests issues related to @woocommerce/admin-e2e-tests label Mar 15, 2023
@chihsuan
Copy link
Member Author

Thanks for the review! @rjchow @moon0326 I just rebased it and fixed payment task e2e tests. Could you take a look and approve it again? Thank you. 🙌

Copy link
Contributor

@adrianduffell adrianduffell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great! Thanks @chihsuan 🚀

@chihsuan chihsuan merged commit f50abc7 into trunk Mar 16, 2023
27 checks passed
@chihsuan chihsuan deleted the remove/wcpay-accordion branch March 16, 2023 04:12
@github-actions github-actions bot added this to the 7.6.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: @woocommerce/admin-e2e-tests issues related to @woocommerce/admin-e2e-tests plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove accordion from “Other payment solutions” in WC Pay Task
4 participants