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

Feat/refactor amount screen components #188

Merged

Conversation

cnguyen812
Copy link
Contributor

Issue: Amount screen component had to dynamically handle 2 argument lists depending on usage as a screen or standalone component.

Refactored it into a standalone component with explicit event handler props, also created an AmountModal component to replace the duplicates for buy/swap/coinbase

@cmgustavo
Copy link
Member

Nice refactor! This PR needs rebase.

@gabrielbazan7
Copy link
Collaborator

gabrielbazan7 commented Jul 21, 2022

You can also delete /navigation/wallet/components/amountModal.tsx file and use the new amountModal component for the select inputs feature. ( select inputs feature was made after you send this PR )

Copy link
Collaborator

@gabrielbazan7 gabrielbazan7 left a comment

Choose a reason for hiding this comment

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

Currently not working when running the app

@cnguyen812 cnguyen812 force-pushed the feat/refactor-screen-components branch from db84b8a to ffdfce3 Compare July 25, 2022 20:42
@cnguyen812
Copy link
Contributor Author

Other amountModal replaced, merge conflicts fixed, ready for re-review

gabrielbazan7
gabrielbazan7 previously approved these changes Jul 26, 2022
Copy link
Collaborator

@gabrielbazan7 gabrielbazan7 left a comment

Choose a reason for hiding this comment

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

ACK!

Gamboster
Gamboster previously approved these changes Jul 26, 2022
Copy link
Collaborator

@Gamboster Gamboster left a comment

Choose a reason for hiding this comment

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

ACK. Great job!

@JohnathanWhite
Copy link
Collaborator

merge conflicts

@cnguyen812 cnguyen812 dismissed stale reviews from Gamboster and gabrielbazan7 via 3beef86 August 1, 2022 14:32
@JohnathanWhite JohnathanWhite merged commit 8809a65 into bitpay:master Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants