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

Plan details yearly monthly styles #1872

Merged
merged 17 commits into from
Jan 27, 2022
Merged

Plan details yearly monthly styles #1872

merged 17 commits into from
Jan 27, 2022

Conversation

tanmoyAtb
Copy link
Contributor

closes #1856


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

@render
Copy link

render bot commented Jan 21, 2022

@render
Copy link

render bot commented Jan 21, 2022

@render
Copy link

render bot commented Jan 21, 2022

@github-actions github-actions bot added the Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. label Jan 21, 2022
@tanmoyAtb tanmoyAtb requested review from FSM1, Tbaut and asnaith January 21, 2022 14:28
Copy link
Contributor

@asnaith asnaith left a comment

Choose a reason for hiding this comment

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

This is working well 👍

I noticed this branch had conflicts that needed resolving and it was because I'd just merged some tests to dev (after your pr submission) that related to the plan details. I was checking to make sure the label changed when the toggle was enabled etc.

I resolved the conflicts and updated the tests to reflect current behavior in this PR, thought it would be quicker for me to do it as I knew exactly what was going on.

@tanmoyAtb
Copy link
Contributor Author

This is working well 👍

I noticed this branch had conflicts that needed resolving and it was because I'd just merged some tests to dev (after your pr submission) that related to the plan details. I was checking to make sure the label changed when the toggle was enabled etc.

I resolved the conflicts and updated the tests to reflect current behavior in this PR, thought it would be quicker for me to do it as I knew exactly what was going on.

Thanks a lot for taking care of this ! The changes look good 👍

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.

Looks good, just could we add the % discount next to the yearly billing as seen in the mock #1856 (comment)? That's very informative IMHO.

@tanmoyAtb
Copy link
Contributor Author

tanmoyAtb commented Jan 25, 2022

#1856 (comment)

Added the "% off" text, I have added a task to #1873 to make sure that the correct "% off" is displayed before deployment.

@Tbaut
Copy link
Collaborator

Tbaut commented Jan 25, 2022

Thanks, what I meant is that we should calculate programmatically what percentage it is, we have both prices so that shouldn't be a problem to calculate,((monthly * 12) - yearly)/(monthly * 12) * 100 so that we are consistent with what we show. Even if it shows 99% right now because our yearly plan is 1$, that's fine. I think this shouldn't be hard-coded :)

@tanmoyAtb
Copy link
Contributor Author

Thanks, what I meant is that we should calculate programmatically what percentage it is, we have both prices so that shouldn't be a problem to calculate,((monthly * 12) - yearly)/(monthly * 12) * 100 so that we are consistent with what we show. Even if it shows 99% right now because our yearly plan is 1$, that's fine. I think this shouldn't be hard-coded :)

The percentage update is up !

@tanmoyAtb tanmoyAtb requested review from Tbaut and asnaith January 26, 2022 17:42
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.

looks great, thanks a lot!

@Tbaut Tbaut merged commit eee506c into dev Jan 27, 2022
@Tbaut Tbaut deleted the mnt/style-change-billing-1856 branch January 27, 2022 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Copy and style change for yearly/monthly payment
5 participants