Skip to content
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

Allow users to go back to crypto payment #1859

Merged
merged 54 commits into from
Jan 24, 2022
Merged

Allow users to go back to crypto payment #1859

merged 54 commits into from
Jan 24, 2022

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Jan 14, 2022

closes #1846

  • fixes a small issue in the crypto PR where "copied" flags were not displayed
  • adds a "(awaiting payment)" info and a "See payment info" button when a crypto payment has been initialized
  • disable switching to any other plan when a crypto payment is pending.
  • shifted most of the logic to expect a pending crypto invoice when the current plan has the status "pending_update"

Submission checklist:

Layout

  • Change looks good in the desktop web ui
  • Change looks good in the mobile web ui

Theme

  • Components / elements inspected in light mode
  • Components / elements inspected in dark mode

FSM1 and others added 28 commits December 16, 2021 16:12
Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
Co-authored-by: Tanmoy Basak Anjan <tanmoy3399@gmail.com>
…Tab/ChangePlan/CryptoPayment.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@render
Copy link

render bot commented Jan 14, 2022

@render
Copy link

render bot commented Jan 14, 2022

@Tbaut Tbaut added the Status: Review Needed 👀 Added to PRs when they need more review label Jan 18, 2022
@Tbaut Tbaut marked this pull request as ready for review January 18, 2022 13:18
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Code looks great and operational ! 💯

It would be great to have the pending state show up in the current subscription here.
Screenshot 2022-01-18 at 10 21 57 PM

@Tbaut
Copy link
Collaborator Author

Tbaut commented Jan 18, 2022

Agreed, the users should know before clicking the "Change plan". I'll add a button there.

@tanmoyAtb
Copy link
Contributor

I pushed a really small commit, for the go back button style in ConfirmPlan slide.

Copy link
Contributor

@FSM1 FSM1 left a comment

Choose a reason for hiding this comment

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

working really well, missed a few translations which I have added suggestions for.

@Tbaut Tbaut requested a review from tanmoyAtb January 20, 2022 11:29
@Tbaut
Copy link
Collaborator Author

Tbaut commented Jan 20, 2022

I added the same loading button on the subscription page (effectively under the new plan, than is awaiting a payment). It still feels a little weird to me to have a loader on a button, but I can't come up with something better, and I guess it does the job for now.

Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

The overall functionality is great !

But you're right UX could improve. What comes to my mind,
Show the current plan like this

Standard plan (awaiting payment confirmation)
Show a see pending invoice button below, remove the change plan modal view.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Jan 21, 2022

Show the current plan like this
Standard plan (awaiting payment confirmation)

This we can do now

Show a see pending invoice button below, remove the change plan modal view.

Asking them to see below would not be a better UX IMHO. I think what could be improved would be to take them directly to the pending crypto payment info, and skip the plan selection. I can certainly hack that in now. Let me give it a go before we merge.

@tanmoyAtb
Copy link
Contributor

Show the current plan like this
Standard plan (awaiting payment confirmation)

This we can do now

Show a see pending invoice button below, remove the change plan modal view.

Asking them to see below would not be a better UX IMHO. I think what could be improved would be to take them directly to the pending crypto payment info, and skip the plan selection. I can certainly hack that in now. Let me give it a go before we merge.

Thats sounds much better. In stead of showing plan selection again, directly show the invoice details :+1

@Tbaut
Copy link
Collaborator Author

Tbaut commented Jan 21, 2022

Ok here we go I ditched the loading circle all together, ping @serenaho
That's how it goes now:

outstanding.mp4

@FSM1 FSM1 merged commit 0dc1f80 into dev Jan 24, 2022
@FSM1 FSM1 deleted the mnt/tbaut-crypto-1846 branch January 24, 2022 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed 👀 Added to PRs when they need more review Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Awaiting payment (crypto) notification & status
4 participants