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

Marketplace: Fix the Marketplace header being invisible after viewing other WC Admin pages. #40562

Merged
merged 2 commits into from Oct 3, 2023

Conversation

raicem
Copy link
Contributor

@raicem raicem commented Oct 3, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Change the way we space out the heading and the content area. The previous method caused issues when you go from WooCommerce > Home to WooCommerce > Marketplace. The header was going missing on the Marketplace page.

How to test the changes in this Pull Request:

  1. Checkout this branch and get a fresh build
  2. Visit the WooCommerce > Marketplace page and see the header in place. Confirm the margins and paddings look good.
  3. Visit WooCommerce > Home and then switch back to WooCommerce > Marketplace. Confirm the header is still visible.
  4. Load the page on a smaller viewport. Confirm header continues to work as expected.
  5. Visit other WooCommerce pages (orders, products, Analytics) and WordPress pages (posts list page, block editor) and confirm we didn't break anything there with these changes.
  6. Please test around this issue.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment Fix the Marketplace header being invisible after viewing other WC Admin pages.

@raicem raicem requested a review from a team October 3, 2023 10:13
@raicem raicem self-assigned this Oct 3, 2023
@raicem raicem requested review from bgrgicak and andfinally and removed request for a team October 3, 2023 10:13
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Hi @Dan-Q, @bgrgicak, @andfinally,

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Test Results Summary

Commit SHA: 44f76f6

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 8s
E2E Tests212007021919m 35s

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.

@raicem raicem requested a review from Dan-Q October 3, 2023 11:28
@raicem raicem marked this pull request as ready for review October 3, 2023 11:29
Copy link
Contributor

@kdevnel kdevnel left a comment

Choose a reason for hiding this comment

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

Great change! Love that you're also reducing the CSS footprint with this. Solid improvement, thanks!

@@ -20,7 +20,7 @@
}

@media (width <= $breakpoint-medium) {
margin: $content-spacing-small;
margin: $grid-unit-20 $grid-unit-20 $grid-unit-10 $grid-unit-20;
Copy link
Contributor

Choose a reason for hiding this comment

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

Appreciate that you're switching over to the standard variables instead of our custom ones. That was my hope over time!

Copy link
Contributor

@andfinally andfinally left a comment

Choose a reason for hiding this comment

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

Everything looks great – tested old and new marketplaces, and other admin pages in WordPress and WooCommerce. Big thanks to you and @kdevnel for addressing this! 💯

@raicem raicem modified the milestones: 8.2.0, 8.3.0 Oct 3, 2023
raicem and others added 2 commits October 3, 2023 17:11
Previous method caused issues when you go from WooCommerce > Home to
WooCommerce > Marketplace. The header was going missing on the
Marketplace page.
@andfinally andfinally force-pushed the fix/wccom-18329-unvisible-header branch from 227e45c to 44f76f6 Compare October 3, 2023 16:11
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Oct 3, 2023
@andfinally andfinally merged commit 60c41cd into trunk Oct 3, 2023
26 checks passed
@andfinally andfinally deleted the fix/wccom-18329-unvisible-header branch October 3, 2023 16:48
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Oct 3, 2023
@alopezari alopezari added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Oct 3, 2023
@lanej0 lanej0 modified the milestones: 8.3.0, 8.2.0 Oct 3, 2023
github-actions bot pushed a commit that referenced this pull request Oct 3, 2023
lanej0 pushed a commit that referenced this pull request Oct 3, 2023
* Marketplace: Fix the Marketplace header being invisible after viewing other WC Admin pages. (#40562)

* Prep for cherry pick 40562

---------

Co-authored-by: And Finally <andfinally@users.noreply.github.com>
Co-authored-by: WooCommerce Bot <no-reply@woocommerce.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants