Store Orders: Edit customer info on an order #18579
Conversation
ryelle
added
Store
[Status] Needs Review
There was a problem hiding this comment.
Few comments above ( of course one about proptypes - heh ) but looking really great! I did encounter one oddity while working with the forms.
If you edit the billing address twice, so click edit on billing - make no changes, close the dialog. Then open it backup again, the phone number is no longer displayed in the modal form. The number is still displayed on the order details screen, it just will not show in the form anymore.
Likewise if you edit billing first, then open shipping, then back to billing the phone number is missing. No changes made in this flow, but I believe the same happens if you do make a change.
The modal held up well in smaller viewports too - nice work.
I do wonder if the edit button itself next to the address fieldset labels looks a bit "heavy" from a design standpoint, maybe @jameskoster or @kellychoffman can chime in on that though.
| last_name: PropTypes.string.isRequired, | ||
| phone: PropTypes.string, | ||
| } ), | ||
| closeDialog: PropTypes.func, |
There was a problem hiding this comment.
Do we want updateAddress() here along with a default below?
| import SectionHeader from 'components/section-header'; | ||
|
|
||
| class OrderCustomerInfo extends Component { | ||
| static propTypes = { | ||
| orderId: PropTypes.number.isRequired, |
There was a problem hiding this comment.
I know, I know, me and my proptype comments 😆 isEditing, site, and siteId are now being passed in the connect - though looks like site isn't being used, only the id.
|
@kellychoffman Let's chat about this. The edit -> edit flow feels weird to me. @ryelle if we tweak that behaviour would you prefer we open a new issue? |
|
Spoke with @kellychoffman decided to hold off on changing anything here until we've judged how the whole edit flow feels with the other actions implemented. As you were! xD Going to tweak some styling bits and pieces now. |
|
Yeah, I think the edit>edit flow only feels weird because its the only editable thing implemented right now. I think it will make more sense and feel fine once everything else is implemented. The original reason for having the editable customer info behind a button in a modal was to make the UI less overwhelming. I also don't think its the most common reason people edit orders. |
|
Idea: use an email validator like does |
|
I also suggest phone validation, but maybe in an advisory capacity only. It usually works well to flag malformed numbers, but I have seen it occasionally flag some valid numbers as invalid, e.g. |
|
Even though our merchants are presently restricted to be US or CA based, they might want to ship to customers in other countries, so we probably need to allow Country settings other than US and Canada. Probably need to handle State vs Province for those too (and some countries have neither)
One particular example is the case of Puerto Rico, which WooCommerce for the last two years has treated as a country and not a state. |
| super( props ); | ||
| this.state = { | ||
| address: props.address, | ||
| phoneCountry: 'US', |
There was a problem hiding this comment.
Might I suggest here (and above, elsewhere) that the default country (edit: i.e. for new orders) be informed by the merchant's country. It would be better for Canadian merchants, for example, to see Canada as the default so they don't have to keep changing it.
You might even want to go ahead and use the merchant's state/province to pre-select that too.
ryelle
force-pushed
the
update/store-order-edit-customer
branch
from
c2c769b to
b58fec2
Compare
|
@jameskoster Did you end up doing style tweaks? I think they got lost in one of my rebases 😟 |
😟 It's fine, I just tweaked the edit buttons for addresses. I'll do it again. |
|
@jameskoster Sorry, and thanks for bringing them back! @timmyc The "edit billing twice and phone disappears" issue is fixed in #18680, and I've addressed your proptypes feedback :) @allendav I'll do the email validation and state/country updates in a following PR, but I think the phone validation is overkill - this phone input gives some nice visual feedback in formatting the number when it's correctly entered, so if validation falsely flags some numbers it would be better to leave it out (IMO). |
timmyc
dismissed
their stale review
Will review #18680 for the phone input fix - nice job tracking that down!
ryelle
added
[Status] Ready to Merge
and removed
[Status] Needs Review
This PR adds the ability to edit the customer info - name, email, phone, and address (billing and shipping) for an order. Once in the "edit" state on the single order screen, two edit buttons show up next to the labels:
These open modals, which are slightly different between billing/shipping – billing has phone and email, along with a "copy to shipping" checkbox
The changes in the modals are saved in component state, and the redux state is only updated when "Save" is clicked. The reason for this is that we want to be able to discard address changes if the user clicks cancel, without accidentally discarding any other pending edits.
To test
This is still behind a feature flag, so you can review on development, but it won't be on staging or production once merged.
Test again with other combos of edits, without copy to shipping selected, just change shipping, cancel out of the modal, cancel out of the edit state, etc.