-
Notifications
You must be signed in to change notification settings - Fork 40
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
Improved: Order detail page on the open tab (#283) #302
Conversation
…on the Left, Order Information on the Right, a 'Ready to Pickup' Button () and created component for order rejected item history (hotwax#283)
…nd removed unwanted imports (hotwax#283)
…ode to open order item rejection history modal (hotwax#283)
…odal UI and some code Improvement (hotwax#283)
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.
See comments @sanskar345
src/locales/es.json
Outdated
@@ -47,6 +47,8 @@ | |||
"Instance Url": "URL de la instancia", | |||
"In store": "En tienda", | |||
"Inventory": "Inventario", | |||
"If you cannot fulfill this order, will be sent an email and the order item will be removed from your dashboard.": "If you cannot fulfill this order, { customerName } will be sent an email and the order item will be removed from your dashboard.", |
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.
I don't think this line is accurate @Dhiraj1405
src/theme/variables.css
Outdated
@@ -76,6 +76,23 @@ http://ionicframework.com/docs/theming/ */ | |||
--ion-color-light-tint: #f5f6f9; | |||
} | |||
|
|||
.empty-state { |
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.
Why are these added here?
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.
As sir, these styles need to be global as we can have multiple empty states in the app.
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.
yes, but I mean why on this section of the file?
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.
Sir, I thought these styles should be coded at last but before the media query, as the media query is written at last I think.
should I add thses at last??
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.
Did you check where the other custom styles are? I would think they should be grouped together in some kind of sequence. Try that @sanskar345
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.
Okay sir done.
src/components/OrderInfo.vue
Outdated
@@ -0,0 +1,132 @@ | |||
<template> | |||
<section> | |||
<ion-item color="light" lines="none"> |
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.
Wrong component
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.
Sir, any suggestion regarding this?
src/views/OrderDetail.vue
Outdated
</ion-button> | ||
</ion-item> | ||
<main :class="{ 'desktop-only' : isDesktop }"> | ||
<OrderInfo v-if="!isDesktop" :orderId="$route.params.orderId" /> |
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.
Is there a way to do this without using order info component twice?
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.
…n order detail page and removed unnecessary entries in locale files (hotwax#283)
…reated states for the same (hotwax#283)
…of isPlatform of ionic instead used ion-hide class and removed the orderInfo component (hotwax#283)
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.
See comments
src/views/OrderDetail.vue
Outdated
grid-area: right; | ||
/* Order Info section to stay fixed and not scrollable as there can be multiple order items which will increase the grid container size */ | ||
height: fit-content; | ||
position: sticky; |
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.
Doesn't need to be sticky. That will fix your height problem too. Try that to simplify the code
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.
Screencast.from.04-10-23.06.58.07.PM.IST.webm
@dt2patel Sir added sticky because we want order Information to be fixed and the user can scroll through the order items.
Added height because it was taking max height of the grid container
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.
Yes I understand that but it is better for user to scroll the entire page here instead of both elements having different behaviors.
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.
Okay sir.
src/views/OrderDetail.vue
Outdated
} | ||
|
||
aside { | ||
grid-area: right; |
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.
grid-area: right; | |
grid-column: 2; |
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.
src/views/OrderDetail.vue
Outdated
main { | ||
display: grid; | ||
grid-template-columns: 1fr 1fr; | ||
grid-template-areas: 'left right'; |
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.
grid-template-areas: 'left right'; |
src/views/OrderDetail.vue
Outdated
grid-template-columns: 1fr 1fr; | ||
grid-template-areas: 'left right'; | ||
gap: var(--spacer-base); | ||
margin: var(--spacer-lg) var(--spacer-lg) 0; |
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.
margin: var(--spacer-lg) var(--spacer-lg) 0; |
src/views/OrderDetail.vue
Outdated
section { | ||
grid-area: left; | ||
} |
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.
section { | |
grid-area: left; | |
} |
</ion-toolbar> | ||
</ion-header> | ||
<ion-content> | ||
<ion-list v-for="(history, index) in rejectionHistory" :key="index"> |
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.
We need to define for loop on the ion-item instead of defining it on ion-list
src/store/modules/order/actions.ts
Outdated
@@ -470,7 +473,7 @@ const actions: ActionTree<OrderState , RootState> ={ | |||
return await dispatch("rejectOrderItems", payload).then((resp) => { | |||
const refreshPickupOrders = resp.find((response: any) => !(response.data._ERROR_MESSAGE_ || response.data._ERROR_MESSAGE_LIST_)) | |||
if (refreshPickupOrders) { | |||
showToast(translate('All items were canceled from the order') + ' ' + payload.orderId); | |||
showToast(payload.part.items.length === 1 ? translate('Item has been rejected successfully') : translate('All items were canceled from the order') + ' ' + payload.orderId); |
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.
showToast(payload.part.items.length === 1 ? translate('Item has been rejected successfully') : translate('All items were canceled from the order') + ' ' + payload.orderId); | |
showToast(payload.part.items.length === 1 ? translate('Item has been rejected successfully') : translate('All items were rejected from the order') + ' ' + payload.orderId); |
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.
Also check on this condition.
src/store/modules/order/actions.ts
Outdated
@@ -450,6 +450,9 @@ const actions: ActionTree<OrderState , RootState> ={ | |||
} | |||
}) | |||
} | |||
// Mark current order as ready to handover |
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.
Update the comment that why we need readyToHandover
property on order
alertController, | ||
IonButton, | ||
IonButtons, | ||
IonContent, | ||
IonFab, | ||
IonFabButton, | ||
IonHeader, | ||
IonIcon, | ||
IonItem, | ||
IonLabel, | ||
IonList, | ||
IonListHeader, | ||
IonTitle, | ||
IonToolbar, | ||
modalController, | ||
IonRadio, | ||
IonRadioGroup |
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.
alertController, | |
IonButton, | |
IonButtons, | |
IonContent, | |
IonFab, | |
IonFabButton, | |
IonHeader, | |
IonIcon, | |
IonItem, | |
IonLabel, | |
IonList, | |
IonListHeader, | |
IonTitle, | |
IonToolbar, | |
modalController, | |
IonRadio, | |
IonRadioGroup | |
IonButton, | |
IonButtons, | |
IonContent, | |
IonFab, | |
IonFabButton, | |
IonHeader, | |
IonIcon, | |
IonItem, | |
IonLabel, | |
IonList, | |
IonListHeader, | |
IonTitle, | |
IonToolbar, | |
IonRadio, | |
IonRadioGroup, | |
alertController, | |
modalController, |
src/views/OrderDetail.vue
Outdated
<ion-item lines="none"> | ||
<aside> | ||
<ion-item v-if="order?.readyToHandover || order?.rejected" color="light" lines="none"> | ||
<ion-icon :icon="order?.readyToHandover ? checkmarkCircleOutline : order?.rejected ? closeCircleOutline : ''" :color="order?.readyToHandover ? 'success' : order?.rejected ? 'danger' : ''" slot="start" /> |
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.
There is no need to have the extra check for order.rejected in icon ternary condition as this block will only be displayed when we have readyToHandover or rejected order.
Also, conditional chaining is also not required inside the ion-item.
src/views/OrderDetail.vue
Outdated
<aside> | ||
<ion-item v-if="order?.readyToHandover || order?.rejected" color="light" lines="none"> | ||
<ion-icon :icon="order?.readyToHandover ? checkmarkCircleOutline : order?.rejected ? closeCircleOutline : ''" :color="order?.readyToHandover ? 'success' : order?.rejected ? 'danger' : ''" slot="start" /> | ||
<ion-label class="ion-text-wrap">{{ order?.readyToHandover ? $t("Order is now ready to handover.") : order?.rejected ? $t("Order has been rejected.") : '' }}</ion-label> |
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.
Same as above comment.
src/views/OrderDetail.vue
Outdated
<h2>{{ order.customer?.name }}</h2> | ||
<p>{{ order.orderName ? order.orderName : order.orderId }}</p> | ||
<p class="overline">{{ $t("Handling Instructions") }}</p> | ||
<p>{{ order?.shippingInstructions }}</p> |
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.
<p>{{ order?.shippingInstructions }}</p> | |
<p>{{ order.shippingInstructions }}</p> |
New UI -In Desktop: In Mobile: |
Related Issues
Closes #283
Short Description and Why It's Useful
Screenshots of Visual Changes before/after (If There Are Any)
In desktop -
Before -
After -
In mobile
Before -
After -
Order Rejected Item History Modal -
In desktop -
In mobile -
Report An Issue Modal -
In desktop -
In mobile -
Contribution and Currently Important Rules Acceptance