Update UI to support minimum and maximum shipping times.

[jorgemd24]

Changes proposed in this Pull Request:

Part of pcTzPl-2qP-p2

This PR introduces a UI to manage minimum and maximum shipping times, and it updates the input to include increase and decrease buttons (like a stepper input). Additionally, we're considering showing "Same Day" instead of 0 to make it clearer for merchants.

Since mixing a string like "Same Day" with the actual integer values for shipping times could make the code more complex, I opted to use the placeholder attribute to display "Same Day" instead of 0 days to keep things simple. However, I'm open to other suggestions or approaches.

For more context see this comment here: pcTzPl-2qP-p2#comment-3652

Other Steeper Inputs in WC

https://github.com/woocommerce/woocommerce/blob/64dcafe29b52cff2dee1d56f5e06caad09a0c470/packages/js/product-editor/src/components/number-control/number-control.tsx#L166-L222

Screenshots:

Detailed test instructions:

1. Follow the steps mentioned in this PR to update the database: Add Shipping Max time column in gla_shipping_times Table #2520
2. Navigate to Dashboard -> Free Listings -> Edit
3. Test the new input buttons by increasing or decreasing the values. If the value is 0, it should display "Same Day".
4. Verify that the data is being stored correctly.
5. Add and modify shipping times as needed.
6. Disconnect all your accounts.
7. Do the onboarding, and see if everything works correctly.

Additional details:

I haven't updated the add and edit modals to use the stepper component yet to make reviewing this PR easier. Also, I want to ask for confirmation from the designers to see if we want to implement it there as well.

Changelog entry

[codecov]

Codecov Report

Attention: Patch coverage is 66.00000% with 17 lines in your changes missing coverage. Please review.

Project coverage is 63.3%. Comparing base (af40ca3) to head (6a31818).
Report is 27 commits behind head on update/shippings-settings-phase-1.

Files with missing lines	Patch %	Lines
...-time/shipping-time-setup/getCountriesTimeArray.js	0.0%	3 Missing and 1 partial ⚠️
...ponents/free-listings/setup-free-listings/index.js	0.0%	2 Missing and 2 partials ⚠️
js/src/hooks/useSaveShippingTimes.js	0.0%	3 Missing and 1 partial ⚠️
...hipping-time/shipping-time-setup/countries-form.js	0.0%	3 Missing ⚠️
js/src/data/actions.js	0.0%	1 Missing ⚠️
js/src/utils/getShippingTimeMapKey.js	50.0%	1 Missing ⚠️

Additional details and impacted files

@@                          Coverage Diff                           @@
##             update/shippings-settings-phase-1   #2594      +/-   ##
======================================================================
- Coverage                                 66.3%   63.3%    -2.9%     
======================================================================
  Files                                      475     322     -153     
  Lines                                    17927    5119   -12808     
  Branches                                     0    1251    +1251     
======================================================================
- Hits                                     11879    3242    -8637     
+ Misses                                    6048    1704    -4344     
- Partials                                     0     173     +173     

Flag	Coverage Δ
js-unit-tests	63.3% <66.0%> (?)
php-unit-tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...listings/configure-product-listings/checkErrors.js	100.0% <100.0%> (ø)
...time-setup/add-time-button/add-time-modal/index.js	11.1% <ø> (ø)
...me-input/edit-time-button/edit-time-modal/index.js	6.7% <ø> (ø)
.../shipping-time-setup/countries-time-input/index.js	91.7% <100.0%> (ø)
...ing-time/shipping-time-setup/time-stepper/index.js	100.0% <100.0%> (ø)
js/src/data/actions.js	7.9% <0.0%> (ø)
js/src/utils/getShippingTimeMapKey.js	50.0% <50.0%> (ø)
...hipping-time/shipping-time-setup/countries-form.js	0.0% <0.0%> (ø)
...-time/shipping-time-setup/getCountriesTimeArray.js	11.1% <0.0%> (ø)
...ponents/free-listings/setup-free-listings/index.js	6.5% <0.0%> (ø)
... and 1 more

... and 786 files with indirect coverage changes

[puntope]

It's handled by the error checker but I think it would be even better if we add some checks on the onChange method to verify that time is not bigger than max_time.

[puntope]

I haven't updated the add and edit modals to use the stepper component yet to make reviewing this PR easier. Also, I want to ask for confirmation from the designers to see if we want to implement it there as well.

Potentially this will be handled separately in other PR. And maybe not setting the stepper... But at least we want to set also the placeholder "Same day" instead of 0?

[jorgemd24]

Thanks @puntope for looking at this, I've addressed your comments, could you please take another look?

Regards these points:

It's handled by the error checker but I think it would be even better if we add some checks on the onChange method to verify that time is not bigger than max_time.

I think to keep consistency with the rest of the form, we could use this change to all fields or leave it as it is. I'm not sure what was the reason to show the validation only when clicking "continue," but if we think it's worth changing, we could explore it in a separate PR.

Potentially this will be handled separately in other PR. And maybe not setting the stepper... But at least we want to set also the placeholder "Same day" instead of 0?

Thanks for the suggestion! I'll discuss this with the designers to see what's best for this form, for now I think we can keep it as it is and then I can adjust it.

[puntope]

Thanks @jorgemd24 I found a new issue when setting timeor max_time as "Same day"

Screen.Recording.2024-09-13.at.13.23.48.mov

[jorgemd24]

Hi @puntope, thanks for reviewing this! The issue was linked to a change I made in the other PR, where 0 was being treated as an invalid value. I've removed that check since the schema already requires those fields. e4686c5

I also found a small issue with the handleBlur function and the original value, which I've fixed here: f218b47

Could you please take another look? Thanks!

PS: The PHP tests failed because of this issue: pb22l9-3cx-p2#comment-3657 but they are OK locally.

[puntope]

Thx @jorgemd24 for the tweaks. Now it's working as expected.

Just one thing about the UI. I see the minus symbol in the right. When normally the standard is to place it in the left.

I believe maybe we can discuss with UI. But not a blocker.

[jorgemd24]

Thanks @puntope for the review

Just one thing about the UI. I see the minus symbol in the right. When normally the standard is to place it in the left.

I understand your point, however, the new version of the product editor uses the minus symbol on the right, so the plan is to maintain that style for consistency.