Product creation experience: shortcut to add variation price #34948

octaedro · 2022-10-04T18:37:09Z

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 aims to introduce an easy way for users to add the variation price. It also modifies a few styles to show the action buttons for the variations without hovering over the rows.

Closes #33554.

How to test the changes in this Pull Request:

Go to Products > Add New.
Add a product name and a description.
In the Product data section select Variable product.
Go to Attributes and create (or select) a product attribute to use for variations.
Go to Variations and add a few variations (don't modify them).
Press Save changes.
Verify that a warning is shown with the number of variations without price.

Press Add price. A modal like the one below will be visible.
Verify the currency matches the currency used on the site.
Add a text (not numbers) and verify that the validation fails. Add a number with multiple decimal points and verify that the error text says: Please enter a value with one decimal point (%s) without thousand separators.
After adding a valid number the button OK should be enabled.
Add a valid number and press Add prices.
Verify that the Regular price for every variation is set with the number added in the modal.
Verify that the accordion expands after pressing Edit.
Press Save changes. The warning should disappear.
Publish the product.
Add another variation. Press Save changes. The warning should be shown again.
Now add another value and verify that the new value has been added to the new variation without modifying the other variations' values.

Now create a new product but select Variable subscription instead of Variable product in step 3, and continue with the other steps.

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.

github-actions · 2022-10-04T18:59:18Z

Test Results Summary

Commit SHA: fc2777a

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	3	0	262	1m 15s
E2E Tests	186	0	0	6	0	192	15m 14s

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 · 2022-10-20T13:45:45Z

plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js

+				.on(
+					'click',
+					'button.add_price_for_variations',
+					this.open_modal_to_set_variations_price


Just underlining the lines added to this file. The lint modified a lot of things here.

octaedro · 2022-10-20T13:46:02Z

plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js

+		/**
+		 * Sets a price for every variation
+		 */
+		set_variations_price: function () {
+			var variation_price = $( '.wc_input_variations_price' ).val();
+			var product_type = $( 'select#product-type' ).val();
+			var input_type =
+				'variable-subscription' === product_type
+					? 'variable_subscription_sign_up_fee'
+					: 'variable_regular_price';
+			var input = $( `.wc_input_price[name^=${ input_type }]` );
+
+			// We don't want to override prices already set
+			input.each( function ( index, el ) {
+				if ( '0' === $( el ).val() || '' === $( el ).val() ) {
+					$( el ).val( variation_price ).trigger( 'change' );
+				}
+			} );
+		},
+
+		/**
+		 * Opens the modal to set a price for every variation
+		 */
+		open_modal_to_set_variations_price: function () {
+			$( this ).WCBackboneModal( {
+				template: 'wc-modal-set-price-variations',
+			} );
+			$( '.add_variations_price_button' ).on(
+				'click',
+				wc_meta_boxes_product_variations_actions.set_variations_price
+			);
+		},


These lines have been added too.

octaedro · 2022-10-20T13:46:28Z

plugins/woocommerce/client/legacy/js/admin/woocommerce_admin.js

-				'.wc_input_price[type=text], .wc_input_decimal[type=text], .wc-order-totals #refund_amount[type=text]',
-				function() {
-					var regex, decimalRegex,
+				'.wc_input_price[type=text], .wc_input_decimal[type=text], .wc-order-totals #refund_amount[type=text], ' +


Everything related to the class wc_input_variations_price in this file has been added by this PR.

Is this part suppose to handle the number parsing?
When I enter a number of 1231231.1231231 it gets used as is, and doesn't get formatted nor does the decimal get trimmed.

Is this part suppose to handle the number parsing?

Nope, it has the same behavior that the variations fields have. The numbers aren't formatted, we were only checking the thousands and decimal pointing.

I'm not sure if it will worth adding that validation to the old product management experience, but if you think that we need it I'll be happy to add it in a follow-up PR.

louwie17

Stellar work on this @octaedro, this was testing very well.
I did notice the number formatting wasn't working for me though, also left a comment in the code about that.

I also have a couple suggestions:

Update the copy of the Modal to be more specific that we are only updating the variations that don't have a price
Triggering Save changes after the user clicks OK in the add price modal, so the notice disappears automatically and this would follow inline with the Set regular prices action.
Lastly if we should change the OK in the modal to be more specific? like Add prices? This is not a must, as I assume we are trying to simulate the same thing as a window.confirm which usually only contains cancel and ok.

One last thing I noticed, the wireframe updated the Default Form Values: copy to Default Variations:, something that wasn't explicitly mentioned in the issue, but I guess should also be added as part of this? cc @jarekmorawski
Just note that if we update this name, we should update the popover to, to reflect his name change. As the popover explicitly mentioned default form values.

Although I find both Default Form Values or Default Variations semi confusing, the tooltip seems to be really necessary.

louwie17 · 2022-10-20T17:37:30Z

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

+				</div>
+				<div class="woocommerce-usage-modal__wrapper">
+					<div class="woocommerce-usage-modal__message">
+						<span>Add price to all variations (<?php echo esc_attr( get_woocommerce_currency_symbol() ); ?> <?php echo esc_textarea( get_woocommerce_currency() ); ?>)</span>


Maybe we want to update this copy to be a bit more specific, given we won't be updating the price of all variations, if some of the variations already contain a price.
Maybe we should tag on: that don't have a price:
Add price to all variations that don't have a price?

I like this suggestion. If it's ok with @jarekmorawski I'll change the text to Add price to variations that don't have a price.

louwie17 · 2022-10-20T17:47:43Z

plugins/woocommerce/client/legacy/js/admin/woocommerce_admin.js

-				'.wc_input_price[type=text], .wc_input_decimal[type=text], .wc-order-totals #refund_amount[type=text]',
-				function() {
-					var regex, decimalRegex,
+				'.wc_input_price[type=text], .wc_input_decimal[type=text], .wc-order-totals #refund_amount[type=text], ' +


Is this part suppose to handle the number parsing?
When I enter a number of 1231231.1231231 it gets used as is, and doesn't get formatted nor does the decimal get trimmed.

louwie17 · 2022-10-20T17:55:47Z

plugins/woocommerce/includes/admin/wc-admin-functions.php

 			?>
 			</p>
+			<div class="woocommerce-add-variation-price-container">
+				<button type="button" class="button add_price_for_variations"><?php esc_html_e( 'Add price', 'woocommerce' ); ?></button>


Could we potentially hide the Add price button after the price has been added?
Or maybe we want to auto trigger the Save changes button, just like what happens when a user clicks Go for the Set regular prices option, this in turn auto triggers a save changes.
This would also auto hide the notice and provide a better feedback loop to the user.

As the Save changes is not very obvious and its confusing for the user to see the notice of the prices still missing after they just added them.

Could we potentially hide the Add price button after the price has been added?

Good idea!

As mentioned above, could we change Add price to Add prices depending if their are multiple or not?

jarekmorawski · 2022-10-21T12:02:35Z

Thanks for the ping, y'all! I can see great work has happened here, but I wonder why we've decided to build a custom modal instead of the browser's default one? It is already used for bulk variation editing and it'd be good to keep these actions consistent.

Update the copy of the Modal to be more specific that we are only updating the variations that don't have a price

The yellow notice already mentions the number of variations that don't have the price added yet. We could show it in the modal, too (image above).

Triggering Save changes after the user clicks OK in the add price modal, so the notice disappears automatically and this would follow inline with the Set regular prices action.

I agree and confirm that this is the expected behavior.

Lastly if we should change the OK in the modal to be more specific? like Add prices? This is not a must, as I assume we are trying to simulate the same thing as a window.confirm which usually only contains cancel and ok.

I wasn't sure if that's possible for browser-managed modals. 🤔 If it is, we should definitely change OK to Add.

One last thing I noticed, the wireframe updated the Default Form Values: copy to Default Variations:, something that wasn't explicitly mentioned in the issue, but I guess should also be added as part of this?

It's a very minor change, so we can leave it out for now.

octaedro · 2022-10-21T20:40:45Z

Hi @jarekmorawski, thank you for your feedback.

I wonder why we've decided to build a custom modal instead of the browser's default one?

We didn't build a custom modal. The modal we use is the same one we use to add a product and add taxes when creating an order, but I get your point. I was confused because my prompt modal has a black background and the one in the design was white 😄.

If we lean towards the prompt modal, we won't be able to add the on-the-fly validation that we are using for the price inputs. Is that expected?
This PR also adds a validation that disables the OK button.

Lastly if we should change the OK in the modal to be more specific? like Add prices? This is not a must, as I assume we are trying to simulate the same thing as a window.confirm which usually only contains cancel and ok.
I wasn't sure if that's possible for browser-managed modals. 🤔 If it is, we should definitely change OK to Add.

I think that there is a little confusion here; the window.confirm was meant for boolean answers. On the other hand the window.prompt allows asking a question but it doesn't allow setting the buttons' texts.

IMO the modal used in this PR provides a better user experience, it allows us to add the on-the-fly validation (it feels pretty handy), the button renaming, and the OK button enabling/disabling when the field is empty.

jarekmorawski · 2022-10-25T10:32:24Z

Good call. I didn't want to add the extra work, but it looks better with our version of the modal. 😃 Let's change the button's label from OK to Add prices and wrap up. Nice work on this, @octaedro!

octaedro · 2022-10-25T13:58:02Z

Hi @louwie17, thank you for your review!
I just addressed the changes you mentioned. Could you give this PR another look?

louwie17

Thanks for the updates @octaedro, those updates look good!
I just noticed one piece of text not wrapped around the translation function, and was curious if we could update the Add price to Add prices and the other way around depending on how many variations need prices.

louwie17 · 2022-10-27T13:09:54Z

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

+				</div>
+				<div class="woocommerce-usage-modal__wrapper">
+					<div class="woocommerce-usage-modal__message">
+						<span>Add price to all variations that don't have a price (<?php echo esc_attr( get_woocommerce_currency_symbol() ); ?> <?php echo esc_textarea( get_woocommerce_currency() ); ?>)</span>


This should be surround with the esc_html_e right?

Nice catch! I add it in commit 6af604f

louwie17 · 2022-10-27T13:12:49Z

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

+					</div>
+					<div class="woocommerce-usage-modal__actions">
+						<button class="modal-close components-button is-secondary"><?php esc_html_e( 'Cancel', 'woocommerce' ); ?></button>
+						<button class="modal-close button components-button add_variations_price_button is-primary" disabled><?php esc_html_e( 'Add prices', 'woocommerce' ); ?></button>


Does PHP provide a helper function for esc_html_e for multiple as well?
It might be nice to show either Add price or Add prices depending on how many are missing their price.
Same in the notice, the notice might be easier to do given you already have access to the count.

This is not a must, but might be nice, if it's not to difficult.

Hi @louwie17,

This change is not trivial and it would imply calling the database again.

The number of variations without price is calculated here. Because html-product-data-variations.php is being loaded before wc-admin-functions.php we're not able to use a global variable, since the value is calculated after the first file is processed.

I'm not sure if the change is worth the effort. What do you think about merging this PR as it is and maybe tackling that change in a follow-up PR?

A follow-up PR sounds good with me.

louwie17 · 2022-10-27T13:13:28Z

plugins/woocommerce/includes/admin/wc-admin-functions.php

 			?>
 			</p>
+			<div class="woocommerce-add-variation-price-container">
+				<button type="button" class="button add_price_for_variations"><?php esc_html_e( 'Add price', 'woocommerce' ); ?></button>


As mentioned above, could we change Add price to Add prices depending if their are multiple or not?

louwie17

Thanks for the updates @octaedro, this is very close to done, I noticed one tiny issue with the styling especially on larger screens, see comment below.
After that fix, this looks good to me.

louwie17 · 2022-10-28T13:23:58Z

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

+	.woocommerce-add-variation-price-container {
+		width: 15%;
+		display: flex;
+		justify-content: center;


We may want to use flex-end here? the spacing on the right looks a little weird, this seems to happen on larger screens it appears.

Good idea! I fixed it in the commit 82e59b8

louwie17

Thanks @octaedro this looks good, appreciate all your hard work on this!

In terms of the failing Github action it does show as required, so I do hope you can still merge without it passing, otherwise you maybe have to fix all those lint errors, or add an exception for that file for now.

louwie17

@octaedro thanks for the rebase, but something may have gone slightly wrong the add price button styling is off now within the Modal:

louwie17

Thanks for the final update and adding those exceptions for the PHP errors.

louwie17

LGTM 🚀

louwie17

Thanks for fixing the lint errors, LGTM 🚀

github-actions · 2022-11-02T12:32:07Z

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

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Oct 4, 2022

octaedro force-pushed the add/33554_variation-price branch from 13dc88e to a1aee3d Compare October 13, 2022 19:24

github-actions bot added the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Oct 18, 2022

octaedro changed the title ~~Add/33554 variation price~~ Product creation experience: shortcut to add variation price Oct 19, 2022

octaedro requested a review from a team October 19, 2022 20:52

octaedro marked this pull request as ready for review October 19, 2022 20:53

octaedro commented Oct 20, 2022

View reviewed changes

louwie17 reviewed Oct 20, 2022

View reviewed changes

octaedro requested a review from louwie17 October 25, 2022 13:58

jarekmorawski mentioned this pull request Oct 26, 2022

[Enhancement]: Add a confirmation modal when the user tries to navigate away with unsaved changes #35194

Closed

2 tasks

louwie17 reviewed Oct 27, 2022

View reviewed changes

github-actions bot removed the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Oct 27, 2022

octaedro force-pushed the add/33554_variation-price branch from 0773200 to 6c6c75b Compare October 27, 2022 21:55

louwie17 reviewed Oct 28, 2022

View reviewed changes

octaedro force-pushed the add/33554_variation-price branch from f445c95 to 82e59b8 Compare October 28, 2022 16:25

louwie17 previously approved these changes Oct 28, 2022

View reviewed changes

octaedro dismissed louwie17’s stale review via 625b5d1 October 28, 2022 19:39

louwie17 requested changes Oct 31, 2022

View reviewed changes

octaedro added 5 commits November 1, 2022 15:26

Add modal

cb008d8

Add modal style

c3fa3b4

Open modal

2b26c9a

Add validations

b153559

Add button

edfc519

octaedro added 11 commits November 1, 2022 15:27

Change row actions

768975e

Fix actions styles

b0150e5

Fix error text

cabe545

Remove console.logs

d4ca752

Add save after setting prices

4dc28f3

Modify modal copy

445e9f4

Change modal button text

ace58c4

Fix text

4c0cedd

Fix text

0ce0a65

Fix styles

6839344

Fix button

3de4b8c

octaedro force-pushed the add/33554_variation-price branch from 8dad80b to 3de4b8c Compare November 1, 2022 18:29

octaedro added 3 commits November 1, 2022 15:35

Fix code sniff errors

3d92bb7

Fix more code sniff errors

e52ec02

Fix code sniff errors

1ccb967

louwie17 previously approved these changes Nov 1, 2022

View reviewed changes

Fix comments

3b88a8d

octaedro dismissed louwie17’s stale review via 3b88a8d November 1, 2022 20:18

louwie17 previously approved these changes Nov 1, 2022

View reviewed changes

Fix comment

4adb1a3

octaedro dismissed louwie17’s stale review via 4adb1a3 November 1, 2022 20:52

octaedro added 2 commits November 1, 2022 22:15

Fix lint

9fcf61e

Fix lint

fc2777a

louwie17 approved these changes Nov 2, 2022

View reviewed changes

octaedro merged commit d77f8fc into trunk Nov 2, 2022

octaedro deleted the add/33554_variation-price branch November 2, 2022 12:31

github-actions bot added this to the 7.2.0 milestone Nov 2, 2022

github-actions bot mentioned this pull request Dec 15, 2022

Test WooCommerce 7.2 UVLabs/location-picker-at-checkout-for-woocommerce#57

Closed

github-actions bot mentioned this pull request Jan 25, 2023

Test WooCommerce 7.3 UVLabs/location-picker-at-checkout-for-woocommerce#59

Closed

Product creation experience: shortcut to add variation price #34948

Product creation experience: shortcut to add variation price #34948

Uh oh!

Conversation

octaedro commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

github-actions bot commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octaedro Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

louwie17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octaedro Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarekmorawski commented Oct 21, 2022

Uh oh!

octaedro commented Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jarekmorawski commented Oct 25, 2022

Uh oh!

octaedro commented Oct 25, 2022

Uh oh!

louwie17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

louwie17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

louwie17 left a comment

Choose a reason for hiding this comment

octaedro commented Oct 4, 2022 •

edited

Loading

github-actions bot commented Oct 4, 2022 •

edited

Loading

octaedro Oct 24, 2022 •

edited

Loading

octaedro Oct 24, 2022 •

edited

Loading

octaedro commented Oct 21, 2022 •

edited

Loading