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

Product creation experience: add more context about product data #33755

Merged
merged 7 commits into from
Jul 20, 2022

Conversation

octaedro
Copy link
Contributor

@octaedro octaedro commented Jul 6, 2022

All Submissions:

Changes proposed in this Pull Request:

This PR adds a tooltip to provide more context to the Product data area, during product creation.

Screen Capture on 2022-07-15 at 13-04-52

Closes #33539.

// cc @jarekmorawski @pmcpinto

How to test the changes in this Pull Request:

  1. Checkout this branch.
  2. Go to Products > Add New.
  3. Verify an interrogation sign (?) icon is visible next to the product type selector, in the Product data area.
  4. Verify that the text in the tooltip looks correct.
Product tye Text
simple Simple – covers the vast majority of any products you may sell. Simple products are shipped and have no options. For example, a book.
grouped Grouped – a collection of related products that can be purchased individually and only consist of simple products. For example, a set of six drinking glasses.
external External or Affiliate – one that you list and describe on your website but is sold elsewhere.
variable Variable – a product with variations, each of which may have a different SKU, price, stock option, etc. For example, a t-shirt available in different colors and/or sizes.
  1. Install this plugin (it will include a product type to the selector).
  2. Verify that the product type My product has been added.
  3. Select it and verify the tooltip shows the text Product types define available product details and attributes, such as downloadable files and variations. They're also used for analytics and inventory management.

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 changelog add --filter=<project>?

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 the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jul 6, 2022
@octaedro octaedro marked this pull request as ready for review July 6, 2022 17:50
@octaedro octaedro requested a review from a team July 6, 2022 17:51
@louwie17 louwie17 self-requested a review July 8, 2022 12:42
Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

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

This tested well @octaedro, I just noticed one spelling mistake in the class name.

But as @jarekmorawski mentioned in the last comment in the original issue, the tooltip text does feel a little odd, pointing users to click on it instead, and not something we have done before.
I like the idea that Jarek proposed, or maybe we want to add a link within the tooltip that users can click? (although a recent change I made doesn't allow users to hover over the tooltip content, so that would make clicking a link within a bit difficult, I could update that though).

@@ -4770,6 +4770,13 @@ img.help_tip {
line-height: 24px;
}

.tips_prodcut_types {
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling mistake, have to switch the c and u in prodcut

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I fixed it in commit 0188d18

@@ -21,6 +21,7 @@
<?php endforeach; ?>
</optgroup>
</select>
<a href="https://woocommerce.com/document/managing-products/#product-types" class="tips_prodcut_types" target="_blank"><?php echo wc_help_tip( __( 'Click in the icon to learn more about each product type', 'woocommerce' ) ); ?></a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same spelling issue as here as above in the class name.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 13, 2022

Test Results Summary

Commit SHA: d4266a3

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

@octaedro
Copy link
Contributor Author

Hey @louwie17,

Could you take another look at this PR?

I added the fix to the typo you mentioned, the changes that Jarek suggested, and updated the testing instructions as well.

@octaedro octaedro requested a review from louwie17 July 18, 2022 16:54
Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @octaedro, nice work on getting the tooltip content to update when the dropdown changes 👍
I just left one minor suggestion for a styling change to keep in sync with how the other tooltips look. Suggestion in in-line comment.

display: inline-block;
}
.woocommerce-product-type-tip {
margin-left: 1em;
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to better reflect how the tip icon looks compared to the other icons:
Screen Shot 2022-07-18 at 4 14 16 PM

They have this css:

font-size: 1.4em;
margin-left: 9px;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eyes @louwie17! I fixed the icon's style in the commit d4266a3

@octaedro octaedro requested a review from louwie17 July 19, 2022 20:43
Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

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

Thanks for sticking with the several changes and iterations of this issue @octaedro, this tested well and code looks good as well, this is ready to ship :shipit: 💯

@octaedro octaedro merged commit 8b9ec81 into trunk Jul 20, 2022
@octaedro octaedro deleted the add/33539_more_context_about_product_data branch July 20, 2022 16:36
@github-actions github-actions bot added this to the 6.9.0 milestone Jul 20, 2022
@github-actions
Copy link
Contributor

Hi @octaedro, 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

jacob-sewell pushed a commit that referenced this pull request Jul 29, 2022
)

* Add product types tooltip

* Add target `_blank`

* Add changelog

* Fix typo

* Add onChange method

* Fixed product type tooltip

* Fix icon style

Co-authored-by: Fernando Marichal <contacto@fernandomarichal.com>
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.

Product creation experience: add more context about product data
2 participants