Skip to content

Conversation

@charleshenryhugo
Copy link
Contributor

@charleshenryhugo charleshenryhugo commented Feb 22, 2020

description:
implemented order detail page for admin.
Admin opens order list page, and clicks an order block, then the order detail page will appear.

fixes #130
fixes #103

video link:
https://youtu.be/cxaZhzQ0S-Y

@charleshenryhugo charleshenryhugo requested review from a team and dragonfly91 February 22, 2020 10:07
@dragonfly91 dragonfly91 added this to the iOSMarchRelease milestone Feb 22, 2020
Copy link
Collaborator

@dragonfly91 dragonfly91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. Please feel free to discuss them!

@dragonfly91
Copy link
Collaborator

dragonfly91 commented Feb 22, 2020

@charleshenryhugo also some more things:

  1. In PR description, you can say fixes #103 so that when this PR gets merged, GitHub will auto close the issue.
  2. If possible, please add a demo video to give us a better idea of how the feature works.
  3. The button to update the order status seems to be missing in your PR. Are you planning to accomplish this at a later date?

Thanks!

@charleshenryhugo charleshenryhugo requested review from a team and dragonfly91 March 2, 2020 02:40
Copy link
Collaborator

@dragonfly91 dragonfly91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments. LGTM.

);
}

Widget _buildCustomerAddress(Customer customer) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there’s some code duplication between customer address and shipping address. Can we do something to reduce the duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dragonfly91
You are right, indeed there is duplication!
I added a Widget _buildAddressRow(Address address) and removed the duplication.

@dragonfly91 dragonfly91 requested review from a team March 3, 2020 04:22
@charleshenryhugo charleshenryhugo merged commit d7ba414 into master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Customers should be allowed to input shipping address and phone number when buying things. More info should be displayed in Order Detail page

3 participants