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.

Sign up for GitHub

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

Jump to bottom

Billing credit card flow #1788

Merged
merged 43 commits into from
Dec 8, 2021
Merged

Billing credit card flow #1788

merged 43 commits into from
Dec 8, 2021

Conversation

tanmoyAtb
Copy link
Contributor

@tanmoyAtb tanmoyAtb commented Nov 30, 2021 •
edited
Loading

closes #1757

API updates are now in.

Submission checklist:

  • Select plan page

  • plan details page

    • monthly
    • yearly

  • Card or wallet page

  • Add card functionality

  • payment page

  • Plan change success page

  • 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

@render
Copy link

render bot commented Nov 30, 2021

Your Render PR Server URL is https://files-billing-stage-pr-1788.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c6iqi4cobjdb9bk1qtv0.

@github-actions github-actions bot added the Type: Feature Added to PRs to identify that the change is a new feature. label Nov 30, 2021
@lgtm-com
Copy link

lgtm-com bot commented Nov 30, 2021

This pull request introduces 1 alert when merging ff9f82c into 79e4549 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Nov 30, 2021

This pull request introduces 1 alert when merging 8cc8847 into 79e4549 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@tanmoyAtb tanmoyAtb marked this pull request as ready for review December 3, 2021 09:35
Tbaut
Tbaut reviewed Dec 3, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Went through the code and found only some lint things. Will play with now :)

packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/AddCard/AddCard.tsx Outdated Show resolved Hide resolved
...ages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/ChangePlanModal.tsx Outdated Show resolved Hide resolved
...ages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/ChangePlanModal.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/PaymentMethod.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/PaymentMethod.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/PaymentMethod.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/SelectPlan.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/SelectPlan.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/SelectPlan.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/CurrentPlan.tsx Outdated Show resolved Hide resolved
Tbaut
Tbaut reviewed Dec 3, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

🎉 It's wired very well, awesome work Tanmoy!

a couple remarks more on the design that the implementation itself
I found this drop-down a little surprising since I've never seen anything similar anywhere else.
Also I think the / instead of > from the original design does make a visual difference for the breadcrumb.
image

Should we place this "go back" next to the "use this card" button, because it's effectively canceling the "add card" process
image

packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/AddCard/AddCard.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/AddCard/AddCard.tsx Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SubscriptionTab/ChangePlan/PaymentMethod.tsx Outdated Show resolved Hide resolved
tanmoyAtb and others added 6 commits December 3, 2021 22:12
@tanmoyAtb
merged default card changes
004b692
@tanmoyAtb
client updates
805c901
@tanmoyAtb
API updates
54ba7c6
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
e346d92 
…Tab/AddCard/AddCard.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
9d57fa4 
…Tab/ChangePlan/PaymentMethod.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
38d536e 
…Tab/ChangePlan/PaymentMethod.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
tanmoyAtb and others added 19 commits December 3, 2021 22:58
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
910a530 
…Tab/ChangePlan/SelectPlan.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
dabf9bc 
…Tab/ChangePlan/SelectPlan.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
610c254 
…Tab/ChangePlan/PaymentMethod.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
e305b6a 
…Tab/ChangePlan/PaymentMethod.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
51927e8 
…Tab/CurrentPlan.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
f75bf66 
…Tab/ChangePlan/PaymentMethod.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
0270244 
…Tab/ChangePlan/PaymentMethod.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
3dbd082 
…Tab/ChangePlan/PlanSuccess.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
f8b9500 
…Tab/ChangePlan/PlanSuccess.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
c61f494 
…Tab/ChangePlan/PlanSuccess.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
cad6e96 
…Tab/ChangePlan/PlanSuccess.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
347faa3 
…Tab/ChangePlan/SelectPlan.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/Subscription…
66d4a16 
…Tab/ChangePlan/ConfirmPlan.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@tanmoyAtb
apply feedbacks
325ae93
@tanmoyAtb
resolved feedbacks
579e15e
@FSM1
Merge branch 'epic/files-billing' into feat/credit-card-flow-1757
d6c3072
@FSM1
fix loader
17b65f4
@tanmoyAtb
go back change
77c1683
@tanmoyAtb
Merge branch 'feat/credit-card-flow-1757' of github.com:ChainSafe/ui-…
36a8db0 
…monorepo into feat/credit-card-flow-1757
@tanmoyAtb
Copy link
Contributor Author

🎉 It's wired very well, awesome work Tanmoy!

a couple remarks more on the design that the implementation itself I found this drop-down a little surprising since I've never seen anything similar anywhere else. Also I think the / instead of > from the original design does make a visual difference for the breadcrumb. image

Should we place this "go back" next to the "use this card" button, because it's effectively canceling the "add card" process image

Hey, I made the changes to the Go back button, makes sense there.

So we already have these breadcrumbs in the file browsers. If you keep moving inside folders after folders, you will see these breadcrumbs in mobile screen. I did not want to add new design of breadcrumbs here. We should stick to one.

What we have in browsers:

Screenshot 2021-12-08 at 11 30 31 AM

Screenshot 2021-12-08 at 11 31 45 AM

Tbaut
Tbaut approved these changes Dec 8, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Pushed plenty of small things to prevent back & forth for the review. Looking great 🎉

@Tbaut Tbaut merged commit 72d72c7 into epic/files-billing Dec 8, 2021
@Tbaut Tbaut deleted the feat/credit-card-flow-1757 branch December 8, 2021 15:49
@Tbaut Tbaut mentioned this pull request Dec 10, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tbaut Tbaut Tbaut approved these changes

@FSM1 FSM1 FSM1 approved these changes

@RyRy79261 RyRy79261 Awaiting requested review from RyRy79261

@asnaith asnaith Awaiting requested review from asnaith

Assignees
No one assigned
Labels
Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tanmoyAtb @FSM1 @Tbaut