[OMS] Add cancel button and modal @W-18998059#2775
[OMS] Add cancel button and modal @W-18998059#2775sf-emmyzhang merged 11 commits intofeature/order-management-pluginfrom
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
…der-btn Signed-off-by: sf-emmyzhang <emmyzhang@salesforce.com>
1814eb7 to
0f886ec
Compare
ccc0c81 to
5c5a5e7
Compare
…der-btn Signed-off-by: sf-emmyzhang <emmyzhang@salesforce.com>
vmarta
left a comment
There was a problem hiding this comment.
Just some minor comments. Other than that, PR is good. Button works and opens up the modal.
| - Show Automatic Bonus Products on Cart Page [#2704](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2704) | ||
| - Support Standard Products [2697](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2697) | ||
| - Fix passwordless race conditions in form submission [#2758](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2758) | ||
| - Add cancel button and modal @W-18998059 [#2775](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2775) |
There was a problem hiding this comment.
Do we need our ticket number here? The changelog is meant for public audience.
| order = null, | ||
| onRequestCancellation = null, |
There was a problem hiding this comment.
| order = null, | |
| onRequestCancellation = null, | |
| order, | |
| onRequestCancellation, |
There's a difference betweennull vs undefined. I think of null as something intentionally passed in.
But in this case, I don't think it's necessary to add null or undefined. Your component propTypes already specifies which props are required and optional.
There was a problem hiding this comment.
I've also seen noops set as default for some modals, not sure what difference that makes because the values always seem to be populated (required)
| ...props | ||
| }) => { | ||
| const handleRequestCancellation = () => { | ||
| if (onRequestCancellation) { |
There was a problem hiding this comment.
Should onRequestCancellation allowed to be falsy at all? That means we don't do anything, which sounds wrong?
| /** | ||
| * Order object for cancellation | ||
| */ | ||
| order: PropTypes.object, |
There was a problem hiding this comment.
Yes, as we'll eventually need order information to be displayed in the cancel order modal
There was a problem hiding this comment.
Can we add .isRequired to it, then?
| id="account_order_detail.title.order_details" | ||
| /> | ||
| </Heading> | ||
| <Button variant="link" size="sm" onClick={onCancelModalOpen}> |
There was a problem hiding this comment.
Not all orders are cancellable, right? How do we make sure this doesn't show up at all times?
There was a problem hiding this comment.
Yes, I haven't added that logic bit yet. I recently learned that SOM has a field called QuantityAvailableToCancel on each orderItemSummary. I will incorporate that in the next iteration, and add a todo comment to address that here if that works?
There was a problem hiding this comment.
Other than quantity (why would quantity matter when canceling an order?), I'd imagine the order status also matters? I am good with doing it in the next iteration. Thanks.
There was a problem hiding this comment.
Good question, I'll follow up on that
| if (!onRequestCancellation) { | ||
| console.error('Cancel order modal: onRequestCancellation is required') | ||
| return | ||
| } |
There was a problem hiding this comment.
I think you can safely remove this check now that it's marked as isRequired.
d789bcf
into
feature/order-management-plugin
* add button and modal * Initial empty commit for order-management-plugin * separate modal component; changelog * unit tests * labels * remove redundancy * chakra? * update changelog * mark cancellation props as required --------- Signed-off-by: sf-emmyzhang <emmyzhang@salesforce.com> Co-authored-by: sf-deepali-bharmal <deepali.bharmal@salesforce.com>
* add button and modal * Initial empty commit for order-management-plugin * separate modal component; changelog * unit tests * labels * remove redundancy * chakra? * update changelog * mark cancellation props as required --------- Signed-off-by: sf-emmyzhang <emmyzhang@salesforce.com> Co-authored-by: sf-deepali-bharmal <deepali.bharmal@salesforce.com>
* add button and modal * Initial empty commit for order-management-plugin * separate modal component; changelog * unit tests * labels * remove redundancy * chakra? * update changelog * mark cancellation props as required --------- Signed-off-by: sf-emmyzhang <emmyzhang@salesforce.com> Co-authored-by: sf-deepali-bharmal <deepali.bharmal@salesforce.com>
Description
Added a "Cancel order" button next to the Order Details heading that opens a "Request Cancellation" modal dialog.
Types of Changes
Changes
How to Test-Drive This PR
Deployed to https://cc-sharks-dev-emmy-test.mobify-storefront-staging.com
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization