-
Notifications
You must be signed in to change notification settings - Fork 46
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
(feat) : added a refund option to the bill manager. #296
(feat) : added a refund option to the bill manager. #296
Conversation
…-3.x into feat/refund-option
Thanks, @amosmachora. Can we only shade credited row(s)? |
The only rows shaded are the credited rows. Maybe the screenshot doesn't do the best job in showing that but I only shade the credited rows. |
Yeah it is by design. Remember we add a new lineitem with a negative value so that it gets subtracted from the total. Thats why both are shaded implying they are refunded items. I dont know maybe we should filter out one to avoid confusion? |
just shade the one with negative value. |
Fixed it @ojwanganto |
Thanks, @amosmachora. CC @donaldkibet |
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.
Thanks @amosmachora Am merging this please do a follow up PR to address the feedback.
packages/esm-billing-app/src/billable-services/bill-manager/modals/refund-bill.modal.tsx
Show resolved
Hide resolved
packages/esm-billing-app/src/billable-services/bill-manager/modals/refund-bill.modal.tsx
Show resolved
Hide resolved
@@ -12,3 +12,15 @@ | |||
.dataTableSkeleton { | |||
margin-top: 0.625rem; | |||
} | |||
|
|||
.refundedItem { | |||
background-color: #fee2e2; |
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 should be using colors from carbon token system e.g
@use '@cabon/colors';
} | ||
|
||
.refundedItem:hover { | ||
background-color: #fecaca !important; |
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.
Using !important
isn't recommend and used as the last resort. I would discourage its usage 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.
It was actually a last resort. I couldnt get anything else to work. But I did not try the carbon/colors
option so maybe that will work. Thanks for the feedback will work on the rest of the feedback before the end of today.
Requirements
Summary
In this PR I have added a refund option to the bill manager. Now administrators could be able to refund line items. This means that refunded items will be consequentially not count towards the total of the bill.
Also the UI marks the refunded lineitem rows with a pale red for accountability purposes. This approach was chosen over filtering out the refunded items. Thanks @ojwanganto for the help.
I also added checks to make sure we do not refund already refunded items.
Screenshots
Refund Option
Screen With Refunded Items
Refund Item Modal
Related Issue
None.
Other
None.