-
Notifications
You must be signed in to change notification settings - Fork 117
Shipping Labels: Show shipment details on order details screen #15889
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
Conversation
Generated by 🚫 Danger |
|
…merce-ios into woomob-754-ui-updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Tested on Woo Shipping extension only. Works as described. Left just a couple of nits.
And an observation - the "Create shipping label" is presented for a fraction of a second when the order details screen is presented 1st time after separate shipments already created.
Simulator.Screen.Recording.-.iPhone.16.-.2025-07-16.at.13.29.47.mp4
return nil | ||
} | ||
return Section( | ||
category: .payment, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be .wooShipping
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 7414f16
@@ -446,6 +452,7 @@ private extension OrderDetailsViewController { | |||
} | |||
|
|||
let shippingLabelCreationVM = WooShippingCreateLabelsViewModel(order: viewModel.order, | |||
selectedShipmentIndex: shipmentIndex, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if we somehow pass an index that's different from the passed label's shipment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we’d like to address those 2 types arguments, I’d say it could be as single argument with an enum type of .shipmentIndex(Int) and .shippingLabel(ShippingLabel)
cases. With that we could explicitly specify the logic of pre-selecting a shipment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 6fa79d4 👍
Thanks for the review @RafaelKayumov! Regarding this:
I'm unsure how to fix this, I hope it's fine to leave it this way for now. I added changes as suggested in subsequent commits. Could you please take another look to see if they're all good? 🙏 |
@@ -5,6 +5,18 @@ import Combine | |||
import struct Networking.WooShippingAccountSettings | |||
import enum Networking.DotcomError | |||
|
|||
enum WooShippingCreateLabelSelection { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
Closes WOOMOB-754
Description
This PR updates the shipping label section on the order details screen. The screen now displays details of the shipments created for an order. Shipments are synced with the local storage to ensure fetched data are displayed instantly when reopening an order.
More specifically:
WooShippingStore
to persist purchased labels and update shipments after requesting refund for a label. This is to ensure that the order details screen is updated instantly after purchasing/refunding labels.Testing steps
TC1: Store with Woo Shipping extension
TC2: Store with the legacy Woo Tax extension
Testing information
Screenshots
RELEASE-NOTES.txt
if necessary.