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

Upgrade card creation #2699

Merged
merged 5 commits into from
May 21, 2024
Merged

Upgrade card creation #2699

merged 5 commits into from
May 21, 2024

Conversation

samaradel
Copy link
Contributor

@samaradel samaradel commented May 14, 2024 •
edited
Loading

Description

  • Calculate the upgrade to the gold price
  • Hide info card in the gold package
  • Add tooltip for each package card

Changes

image
image

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

samaradel added 2 commits May 14, 2024 12:20
@samaradel
Upgrade card creation
7146519
@samaradel
Remove coupon component
cd49cdc 
Calculate upgrade to gold price
Hide info card in gold package
@samaradel samaradel marked this pull request as ready for review May 14, 2024 14:08
Mahmoud-Emad
Mahmoud-Emad requested changes May 15, 2024
View reviewed changes
Copy link
Contributor

@Mahmoud-Emad Mahmoud-Emad left a comment

Choose a reason for hiding this comment

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

Good job ya Samar

packages/playground/src/calculator/pricing_calculator.vue Outdated Show resolved Hide resolved
packages/playground/src/calculator/pricing_calculator.vue Outdated Show resolved Hide resolved
packages/playground/src/calculator/pricing_calculator.vue Outdated Show resolved Hide resolved
zaelgohary
zaelgohary reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@zaelgohary zaelgohary left a comment

Choose a reason for hiding this comment

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

The UI looks good if both packages are gold. However, it looks a bit odd in other situations like the following.

image

What do you think if we replaced the Too expensive? line outside the card? I'm not sure where but I think it would look better.

@samaradel samaradel marked this pull request as draft May 15, 2024 08:44
@samaradel samaradel marked this pull request as ready for review May 15, 2024 10:58
@amiraabouhadid
Copy link
Contributor

the cards show up like this for me even after pulling and rebuilding
image

@samaradel
Copy link
Contributor Author

samaradel commented May 19, 2024 •
edited
Loading

@amiraabouhadid it's because you used your current balance which doesn't even reach the bronze package price

@samaradel samaradel requested a review from Mahmoud-Emad May 19, 2024 08:30
AlaaElattar
AlaaElattar reviewed May 19, 2024
View reviewed changes
Copy link
Contributor

@AlaaElattar AlaaElattar left a comment

Choose a reason for hiding this comment

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

  • I tested it while logging out, it said Too expensive? can upgrade to Gold package to get discount up to 60% when you fund your wallet with 2061 TFT when the balance was 1.
  • i changed it to 2061 and still no gold package

image

@samaradel samaradel marked this pull request as draft May 19, 2024 13:30
@samaradel samaradel marked this pull request as ready for review May 19, 2024 13:30
@samaradel
Copy link
Contributor Author

@AlaaElattar are you sure you're on my branch?

Screencast.from.19-05-24.16.29.44.webm

@AlaaElattar
Copy link
Contributor

@AlaaElattar are you sure you're on my branch?

Screencast.from.19-05-24.16.29.44.webm

yes I tested it again now and it's working right.

@samaradel samaradel merged commit e7bb580 into development May 21, 2024
3 checks passed
@samaradel samaradel deleted the development_upgrade_card branch May 21, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maayarosama maayarosama maayarosama approved these changes

@Mahmoud-Emad Mahmoud-Emad Mahmoud-Emad approved these changes

@AlaaElattar AlaaElattar AlaaElattar approved these changes

@AhmedHanafy725 AhmedHanafy725 Awaiting requested review from AhmedHanafy725 AhmedHanafy725 is a code owner

@MohamedElmdary MohamedElmdary Awaiting requested review from MohamedElmdary MohamedElmdary is a code owner

@amiraabouhadid amiraabouhadid Awaiting requested review from amiraabouhadid amiraabouhadid is a code owner

@0oM4R 0oM4R Awaiting requested review from 0oM4R 0oM4R is a code owner

@mohamedamer453 mohamedamer453 Awaiting requested review from mohamedamer453

@zaelgohary zaelgohary Awaiting requested review from zaelgohary zaelgohary is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@samaradel @amiraabouhadid @AlaaElattar @zaelgohary @Mahmoud-Emad @maayarosama