Show message in Inventory tab for variable products #37185

octaedro · 2023-03-12T17:19:16Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR adds the code to show a message in the Inventory tab when the product is type variable.

Closes #37119.

How to test the changes in this Pull Request:

Have you followed the Writing high-quality testing instructions guide?

Checkout this branch.
Go to Products> Add New > Product data > Inventory.
Verify that the message Settings below apply to all variations without manual stock management enabled. [Learn more](https://woocommerce.com/document/variable-product/) is not visible.
Select Variable product and check that now the message is visible.

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?
Have you included testing instructions?

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 · 2023-03-12T17:35:29Z

Test Results Summary

Commit SHA: 4175251

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 55s
E2E Tests	189	0	0	6	0	195	16m 32s

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.

codecov · 2023-03-12T17:50:19Z

Codecov Report

Merging #37185 (4175251) into trunk (c6e6f87) will not change coverage.
The diff coverage is 37.1%.

Additional details and impacted files

@@           Coverage Diff            @@
##             trunk   #37185   +/-   ##
========================================
  Coverage     46.7%    46.7%           
  Complexity   17191    17191           
========================================
  Files          429      429           
  Lines        64845    64845           
========================================
  Hits         30275    30275           
  Misses       34570    34570

Impacted Files	Coverage Δ
...ocommerce/includes/abstracts/abstract-wc-order.php	`86.7% <0.0%> (ø)`
...ce/includes/abstracts/abstract-wc-settings-api.php	`56.3% <0.0%> (ø)`
...ocommerce/includes/admin/class-wc-admin-assets.php	`0.0% <0.0%> (ø)`
...oocommerce/includes/admin/class-wc-admin-menus.php	`14.7% <0.0%> (ø)`
...ommerce/includes/admin/class-wc-admin-settings.php	`16.8% <0.0%> (ø)`
...ocommerce/includes/admin/class-wc-admin-status.php	`12.2% <0.0%> (ø)`
...ludes/admin/notes/class-wc-notes-run-db-update.php	`78.0% <ø> (ø)`
...ocommerce/includes/admin/wc-meta-box-functions.php	`0.0% <0.0%> (ø)`
...ugins/woocommerce/includes/class-wc-post-types.php	`2.8% <0.0%> (ø)`
...oocommerce/includes/react-admin/core-functions.php	`33.3% <ø> (ø)`
... and 6 more

octaedro · 2023-03-12T17:59:30Z

plugins/woocommerce/includes/admin/meta-boxes/views/html-product-data-inventory.php

+	<div class="inline notice woocommerce-message show_if_variable">
+		<p>
+			<?php echo esc_html_e( 'Settings below apply to all variations without manual stock management enabled. ', 'woocommerce' ); ?> <a target="_blank" href="https://woocommerce.com/document/variable-product/"><?php esc_html_e( 'Learn more', 'woocommerce' ); ?></a>
+		</p>
+	</div>


We can abstract this (and every use of this kind of message) in a follow-up PR. We might create a method like woocommerce_wp_notice or woocommerce_wp_message here.

Any reason not to just abstract it in this PR? Do you question whether it should be abstracted?

I don't feel strongly whether it should be at this point, but would lean slightly towards doing so in this PR.

mattsherman · 2023-03-13T12:06:35Z

I'll review this

mattsherman

Tested. Overall, works as expected.

There are a few minor styling tweaks that should be made to ensure that this message looks ideal.

mattsherman · 2023-03-13T13:25:35Z

plugins/woocommerce/client/legacy/css/admin.scss

@@ -959,12 +959,16 @@
 }

 #variable_product_options #message,
+#inventory_product_data .notice,
 #variable_product_options .notice {
 	display: flex;
 	margin: 10px;
 	background-color: #ffffff;
 	> p {
 		width: 85%;


This width: 85% results in the notice looking odd when it goes to two or more lines.

I think it can just be removed.

Before (made the text longer to simulate a longer translation or different user settings that would result in more lines):

After:

Nice catch! I couldn't delete it because we will need the 85% when there is a button, like here:

But I added the code we need for that case in the commit 5108e42.

That explicit 85%/15% sizing is not ideal, and results in display issues. But, that's really outside the scope of this PR I suppose... I'll create a separate PR later today to fix that up...

Before:

After removing explicit widths:

mattsherman · 2023-03-13T13:29:42Z

plugins/woocommerce/client/legacy/css/admin.scss

 #variable_product_options .notice {
 	display: flex;
 	margin: 10px;
 	background-color: #ffffff;
 	> p {
 		width: 85%;
+		a {
+			text-decoration: none;


Ideally there shouldn't be a line break in the middle of this link. We should set white-space: nowrap on it.

Before (again, text made longer to simulate translation, etc.):

After:

mattsherman · 2023-03-13T13:43:42Z

plugins/woocommerce/includes/admin/meta-boxes/views/html-product-data-inventory.php

-
+	<div class="inline notice woocommerce-message show_if_variable">
+		<p>
+			<?php echo esc_html_e( 'Settings below apply to all variations without manual stock management enabled. ', 'woocommerce' ); ?> <a target="_blank" href="https://woocommerce.com/document/variable-product/"><?php esc_html_e( 'Learn more', 'woocommerce' ); ?></a>


There is an errant space at the end of the translated message. enabled. That space is not needed.

mattsherman · 2023-03-13T13:44:26Z

plugins/woocommerce/includes/admin/meta-boxes/views/html-product-data-inventory.php

+	<div class="inline notice woocommerce-message show_if_variable">
+		<p>
+			<?php echo esc_html_e( 'Settings below apply to all variations without manual stock management enabled. ', 'woocommerce' ); ?> <a target="_blank" href="https://woocommerce.com/document/variable-product/"><?php esc_html_e( 'Learn more', 'woocommerce' ); ?></a>
+		</p>
+	</div>


Any reason not to just abstract it in this PR? Do you question whether it should be abstracted?

I don't feel strongly whether it should be at this point, but would lean slightly towards doing so in this PR.

octaedro · 2023-03-13T15:12:59Z

Any reason not to just abstract it in this PR? Do you question whether it should be abstracted?

@mattsherman Yes, I think that it should be refactored but that refactor implies also touching code in 4 different files (wc-admin-functions, html-product-data-attributes, html-product-data-variations, and html-product-data-inventory). I prefer to wrap those changes in a different PR and only add the message in this one, to keep this PR small.

octaedro · 2023-03-13T15:14:09Z

Hey @mattsherman,
Thank you for reviewing this PR. I just addressed the suggestions you made.
Could you take another look at this PR?

mattsherman · 2023-03-13T15:46:16Z

Any reason not to just abstract it in this PR? Do you question whether it should be abstracted?

@mattsherman Yes, I think that it should be refactored but that refactor implies also touching code in 4 different files (wc-admin-functions, html-product-data-attributes, html-product-data-variations, and html-product-data-inventory). I prefer to wrap those changes in a different PR and only add the message in this one, to keep this PR small.

Makes sense.

mattsherman

Thanks for fixing things up, @octaedro ! Nice work. Approved.

mattsherman · 2023-03-13T15:55:22Z

plugins/woocommerce/client/legacy/css/admin.scss

+			white-space: nowrap;
+		}
+	}
+	> p:not( :last-child ) {


As mentioned, this explicit width causes issues but it outside the scope of this PR, so I'm good with this fix for now!

Fernando Marichal added 3 commits March 12, 2023 14:14

Add message to inventory tab

cc1edd1

Add styles

254379a

Add changelog

bade9ca

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 12, 2023

octaedro changed the title ~~Dev/37119 show message for variable products~~ Show message in Inventory tab for variable products Mar 12, 2023

Fix link styles

a6210b1

octaedro commented Mar 12, 2023

View reviewed changes

octaedro marked this pull request as ready for review March 12, 2023 18:00

octaedro self-assigned this Mar 12, 2023

octaedro requested a review from a team March 12, 2023 18:00

mattsherman self-requested a review March 13, 2023 12:06

mattsherman requested changes Mar 13, 2023

View reviewed changes

Fernando Marichal added 3 commits March 13, 2023 11:47

Fix message style without buttons

5108e42

Avoid line break

bbc538f

Remove space

4175251

octaedro requested a review from mattsherman March 13, 2023 15:14

mattsherman approved these changes Mar 13, 2023

View reviewed changes

octaedro merged commit b1a0d31 into trunk Mar 13, 2023
28 of 29 checks passed

octaedro deleted the dev/37119_show_message_for_variable_products branch March 13, 2023 21:26

github-actions bot added this to the 7.6.0 milestone Mar 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show message in Inventory tab for variable products #37185

Show message in Inventory tab for variable products #37185

octaedro commented Mar 12, 2023 •

edited

github-actions bot commented Mar 12, 2023 •

edited

codecov bot commented Mar 12, 2023 •

edited

octaedro Mar 12, 2023 •

edited

mattsherman Mar 13, 2023

mattsherman commented Mar 13, 2023

mattsherman left a comment

mattsherman Mar 13, 2023

octaedro Mar 13, 2023

mattsherman Mar 13, 2023

mattsherman Mar 13, 2023

octaedro Mar 13, 2023

mattsherman Mar 13, 2023

mattsherman Mar 13, 2023

octaedro commented Mar 13, 2023 •

edited

octaedro commented Mar 13, 2023

mattsherman commented Mar 13, 2023

mattsherman left a comment

mattsherman Mar 13, 2023

Show message in Inventory tab for variable products #37185

Show message in Inventory tab for variable products #37185

Conversation

octaedro commented Mar 12, 2023 • edited

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

github-actions bot commented Mar 12, 2023 • edited

Test Results Summary

codecov bot commented Mar 12, 2023 • edited

Codecov Report

octaedro Mar 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsherman commented Mar 13, 2023

mattsherman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

octaedro commented Mar 13, 2023 • edited

octaedro commented Mar 13, 2023

mattsherman commented Mar 13, 2023

mattsherman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

octaedro commented Mar 12, 2023 •

edited

github-actions bot commented Mar 12, 2023 •

edited

codecov bot commented Mar 12, 2023 •

edited

octaedro Mar 12, 2023 •

edited

octaedro commented Mar 13, 2023 •

edited