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

Show Settings navigation tabs on Product Sets taxonomy screens #2814

Merged
merged 4 commits into from
Oct 31, 2024

Conversation

layoutd
Copy link
Contributor

@layoutd layoutd commented Oct 26, 2024

Changes proposed in this Pull Request:

This PR renders the basic navigation tabs used on "Connection", "Product Sync", and "Advertise" pages, on the "Product Sets" pages as well.

They're rendered (hidden) at the top of the corresponding list + edit pages, and then moved in the DOM and displayed. Depending on page load speed, they might not be available for short period, but I've seen the tabs flicker on all the other settings pages of the extension as well.

  • Do the changed files pass phpcs checks? Please remove phpcs:ignore comments in changed files and fix any issues, or delete if not practical.

Screenshots:

Before
image

After
image
image

Other Settings Pages (for comparison)
image

Detailed test instructions:

  1. Connect a store.
  2. Navigate around the different sections of the Facebook for WooCommerce extension.
  3. Confirm the navigation tabs appear correctly in the different screens related to Product Sets.
  4. Try with several browser resolutions (mobile, desktop, etc).
  5. Also confirm that the correct menu item is highlighted always (Marketing > Facebook).

Changelog entry

Add - Extension navigation tabs on Product Sets screens.

@layoutd layoutd self-assigned this Oct 26, 2024
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Oct 26, 2024
@layoutd layoutd requested a review from a team October 29, 2024 15:55
Copy link
Contributor

@brezocordero brezocordero left a comment

Choose a reason for hiding this comment

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

@layoutd
Tested and it works.
I am pre-approving but please remove the commented code.
I have a question about the approach but it is not blocking as this works. It could also be added as a future improvement.

includes/Admin/Settings.php Outdated Show resolved Hide resolved
includes/Admin/Settings.php Outdated Show resolved Hide resolved
@brezocordero
Copy link
Contributor

@layoutd Thanks for making the changes.
Tested and looks good. 🚢

@layoutd
Copy link
Contributor Author

layoutd commented Oct 31, 2024

Thanks @brezocordero!

@layoutd layoutd merged commit 0cb2a05 into develop Oct 31, 2024
4 checks passed
@layoutd layoutd deleted the tweak/tab-nav-product-sets branch October 31, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants