-
Notifications
You must be signed in to change notification settings - Fork 16
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
Proration and credit calculated amounts #1878
Conversation
Your Render PR Server URL is https://storage-ui-stage-pr-1878.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c7o0gcjvog4or9lo8tkg. |
Your Render PR Server URL is https://chainsafe-components-stage-pr-1878.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c7o0gd3vog4or9lo8u10. |
Your Render PR Server URL is https://files-ui-stage-pr-1878.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c7o0gdrvog4or9lo8u5g. |
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 went briefly though the code, lgtm, I'll play with it now.
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/ConfirmPlan.tsx
Outdated
Show resolved
Hide resolved
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.
Looking good, it's simple and does the job well IMHO! Left some more nits.
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/ConfirmPlan.tsx
Outdated
Show resolved
Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/ConfirmPlan.tsx
Outdated
Show resolved
Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/ConfirmPlan.tsx
Outdated
Show resolved
Hide resolved
…Tab/ChangePlan/ConfirmPlan.tsx Co-authored-by: Thibaut Sardan <[email protected]>
…Tab/ChangePlan/ConfirmPlan.tsx Co-authored-by: Thibaut Sardan <[email protected]>
…Tab/ChangePlan/ConfirmPlan.tsx Co-authored-by: Thibaut Sardan <[email protected]>
…Tab/ChangePlan/ConfirmPlan.tsx Co-authored-by: Thibaut Sardan <[email protected]>
This seems to remember only the last payment when performing the calculations. Is this how it should work? I'm finding it a bit confusing so I'm sure users will too. I'm not sure this is a genuine flow that someone would do but currently - If I purchase a premium plan for $199 then if I downgrade to a standard plan then I don't owe anything because I paid more for the premium. However, if I then try to upgrade the standard plan back to premium I will have to pay an additional $100 as the standard account was only $99...but really the account should still have another $100 associated with it and no cost to go back to premium because the initial payment was $199. Similarly, I purchase an annual standard plan for $99 and then I switch to a monthly premium plan the cost is $0.00 but it's not clear to me if the previously paid $99 will mean I don't get billed for the first 5 months when on the premium plan. Hope this makes sense! |
I had this thought too regarding the fact that we don't say anything about future invoices that might be impacted by the current account balance, but I figured this is something that the user intuitively understands, when telling them how much their account balance is. I had a somewhat similar "issue" that my balance seemed to have been reset when switching to the free plan, but I guess this might be something to discuss with the api team. |
@asnaith Thanks for bringing this up. I noticed the first flow mismatch as well. However the calculations are fully done on the API side. Can you post the two flows on the files channel and we can discuss it with the team. also we're only getting the next payment amounts from the API. IMO thats sufficient for the user to understand that the next few payments will be adjusted. |
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.
@FSM1 do you mean |
I was just thinking of that... how about we go for something like the:
|
Looks relatable to me, I'll update this once API makes the changes for |
Hey guys, The PR is good to be reviewed IMO. As long as the terminology looks relatable to you, Its good to go. |
closes #1869
Submission checklist:
Layout
Theme