Skip to content

Conversation

@joshuatf
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

Fixes up the product form header on smaller screens and move some secondary buttons into the menu group.

Screen Shot 2022-11-17 at 5 40 55 AM

Screen Shot 2022-11-17 at 5 40 45 AM

Screen Shot 2022-11-17 at 5 40 42 AM

Closes #35168 .

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

  1. Navigate to the new product page Products -> Add new (MVP)
  2. Check that the header is unchanged on larger screens
  3. Narrow your viewport to < 782px.
  4. Note the styling changes
  5. Under the "Publish" menu button group, check that Save draft / Switch to draft and Preview options are shown

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?

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.

@joshuatf joshuatf requested a review from a team November 17, 2022 16:29
@joshuatf joshuatf self-assigned this Nov 17, 2022
@github-actions github-actions bot added focus: react admin package: @woocommerce/components issues related to @woocommerce/components plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Nov 17, 2022
Base automatically changed from add/35170 to trunk November 18, 2022 17:51
@github-actions github-actions bot removed package: @woocommerce/components issues related to @woocommerce/components plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Nov 18, 2022
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore No types for this exist yet.
// eslint-disable-next-line @woocommerce/dependency-group
import { store } from '@wordpress/viewport';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to add types to this, but the dependency did not work as expected. Worth sanity checking to make sure it's not just my environment.

const SecondaryActionsComponent = isSmallViewport ? MenuItem : Button;
const secondaryActions = (
<>
<SecondaryActionsComponent
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No changes in this code other than the SecondaryActionsComponent swapping that occurs.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2022

Test Results Summary

Commit SHA: 88c14e2

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

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.

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Nov 20, 2022
Copy link
Contributor

@mdperez86 mdperez86 left a comment

Choose a reason for hiding this comment

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

You have some conflicts here. But anithing else works well. Great job!

@mdperez86 mdperez86 merged commit db737f2 into trunk Nov 24, 2022
@mdperez86 mdperez86 deleted the update/35168 branch November 24, 2022 14:00
@github-actions github-actions bot added this to the 7.3.0 milestone Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Header] Header default state

3 participants