Skip to content

Add shipping dimensions image #34857

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.

Sign up for GitHub

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

Jump to bottom
Merged
mdperez86 merged 6 commits into from
Oct 3, 2022
Merged

Add shipping dimensions image #34857

mdperez86 merged 6 commits into from
Oct 3, 2022

Conversation

@mdperez86
Copy link
Contributor

@mdperez86 mdperez86 commented Sep 27, 2022 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Partialy closes #34329.

How to test the changes in this Pull Request:

  1. Go to new product page.
  2. In Shipping Dimensions section a package image should be shown on the right side of the diemensions fields.
  3. If you focus the fields (A, B, C) a particular side of the package image should be highlighted.

Screen.Recording.2022-09-28.at.10.54.42.AM.mov

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 successfully run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> run 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.

@mdperez86 mdperez86 requested a review from a team September 27, 2022 22:28
@github-actions github-actions bot added focus: react admin package: @woocommerce/components issues related to @woocommerce/components package: @woocommerce/data issues related to @woocommerce/data package: @woocommerce/number issues related to @woocommerce/number plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Sep 27, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 27, 2022 •
edited
Loading

Test Results Summary

Commit SHA: 6205b8a

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests12000201220m 35s
E2E Tests187002018918m 54s
 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.

@mdperez86 mdperez86 force-pushed the add/34329-shipping-dimensions-image branch from 5864af4 to 4af0451 Compare September 28, 2022 18:57
@mdperez86 mdperez86 marked this pull request as ready for review September 28, 2022 19:04
@mdperez86 mdperez86 force-pushed the add/34329-shipping-dimensions-image branch 4 times, most recently from 81a6ebc to e732942 Compare September 29, 2022 18:14
@github-actions github-actions bot removed package: @woocommerce/data issues related to @woocommerce/data package: @woocommerce/number issues related to @woocommerce/number package: @woocommerce/components issues related to @woocommerce/components labels Sep 29, 2022
@mdperez86 mdperez86 force-pushed the add/34329-shipping-dimensions-image branch from e732942 to 4da2d7d Compare October 3, 2022 14:03
@mdperez86 mdperez86 force-pushed the add/34329-shipping-dimensions-image branch from 4da2d7d to db4037d Compare October 3, 2022 14:04
joshuatf
joshuatf previously approved these changes Oct 3, 2022
View reviewed changes
Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

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

This is looking great and testing well! Left a couple minor nitpicky naming comments, but pre-approving.

...ommerce-admin/client/products/fields/shipping-dimensions-image/shipping-dimensions-image.tsx
{ /* Side A */ }
<path
d="M10.4922 134.221V35.2617C10.4922 33.8539 11.9079 32.8867 13.2193 33.3986L98.3109 66.6076C99.0711 66.9043 99.5748 67.633 99.5837 68.449L100.703 171.089C100.719 172.534 99.2449 173.518 97.9167 172.95L11.7054 136.06C10.9695 135.745 10.4922 135.022 10.4922 134.221Z"
fill={ highlight === 'A' ? '#F0F6FC' : '#F6F7F7' }
Copy link
Contributor

@joshuatf joshuatf Oct 3, 2022

Choose a reason for hiding this comment

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

Love this really simple solution; nicely done!

plugins/woocommerce-admin/client/products/sections/product-shipping-section.tsx Outdated
export const ProductShippingSection: React.FC = () => {
const { getInputProps } = useFormContext< Product >();
const { formatNumber, parseNumber } = useProductHelper();
const [ highlightSide, setHighLightSide ] =
Copy link
Contributor

@joshuatf joshuatf Oct 3, 2022

Choose a reason for hiding this comment

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

Nitpicky, but should be setHighlightSide. ("Highlight" being one word)

Copy link
Contributor Author

@mdperez86 mdperez86 Oct 3, 2022

Choose a reason for hiding this comment

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

Absolutly ;)

plugins/woocommerce-admin/client/products/sections/product-shipping-section.tsx Outdated
</p>
<div className="product-shipping-section__container">
<div>
<div className="product-shipping-section__dimensions__body">
Copy link
Contributor

@joshuatf joshuatf Oct 3, 2022

Choose a reason for hiding this comment

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

We typically use a similar class naming style to BEM so this class could either be:

product-shipping-section__dimensions-body

Or if dimensions is its own standalone block:

product-shipping-section-dimensions__body

Copy link
Contributor Author

@mdperez86 mdperez86 Oct 3, 2022

Choose a reason for hiding this comment

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

Done

@mdperez86 mdperez86 requested a review from joshuatf October 3, 2022 17:28
joshuatf
joshuatf approved these changes Oct 3, 2022
View reviewed changes
Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the updates!

@mdperez86 mdperez86 merged commit d3c1964 into trunk Oct 3, 2022
@mdperez86 mdperez86 deleted the add/34329-shipping-dimensions-image branch October 3, 2022 18:06
@github-actions github-actions bot added this to the 7.1.0 milestone Oct 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022 •
edited by mdperez86
Loading

Hi @mdperez86, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

@github-actions github-actions bot mentioned this pull request Nov 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@joshuatf joshuatf joshuatf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

7.1.0

Development

Successfully merging this pull request may close these issues.

[Shipping] Dimensions

3 participants

@mdperez86 @joshuatf