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

Feature: Initial Swaps amount view #1994

Merged
merged 24 commits into from
Dec 10, 2020
Merged

Conversation

wachunei
Copy link
Member

@wachunei wachunei commented Nov 16, 2020

Description

This PR adds the initial swaps amount view. For now slippage is fixed at 1%.

Draft Checklist

  • Upgrade swaps controller to working cache

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #???

@wachunei wachunei marked this pull request as ready for review November 17, 2020 16:13
@wachunei wachunei requested a review from a team as a code owner November 17, 2020 16:13
@ibrahimtaveras00 ibrahimtaveras00 added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Dec 7, 2020
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

Only padding issue I saw was on Android Nexus S (4 inch screen)

Screen Shot 2020-12-07 at 6 06 06 PM

Nonetheless, you can scroll on this view so it's not too big of a problem

Another issue I saw was that the title Convert to or Convert from is cut off on smaller Android devices when keyboard is expanded when searching token list

In this example, this is a pixel (5 inch screen) on left compared to iPhone SE 2nd generation

Screen Shot 2020-12-07 at 6 08 13 PM

@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Dec 7, 2020
@wachunei
Copy link
Member Author

wachunei commented Dec 9, 2020

I pushed an updated code with conditional height for the list depending wether the keyboard is open and the device size in order to keep the UI contained in the viewport at all times.

Screen Shot 2020-12-08 at 23 14 15

Screen Shot 2020-12-08 at 23 14 40

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Dec 10, 2020
@wachunei wachunei merged commit 2898607 into develop Dec 10, 2020
@wachunei wachunei deleted the feature/swaps-amount-view branch December 10, 2020 14:25
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
@tommasini tommasini mentioned this pull request Oct 14, 2024
7 tasks
github-merge-queue bot pushed a commit that referenced this pull request Oct 18, 2024
## **Description**
* '@ethersproject/abi' - Can be cleaned from package.json since it's
only used by dependencies

![image](https://github.com/user-attachments/assets/587575bf-8829-4854-8cc5-cc8b0c525fd6)
* @react-native-picker/picker - There is no usage for a picker anymore
in the mobile app ([Introduced
here](#3776))

![image](https://github.com/user-attachments/assets/8e283096-2d84-44a1-b8f7-fc1bc933af93)
* @rnhooks/keyboard - There is no usage for @rnhooks/keyboard
([introduced
here](#1994))

![image](https://github.com/user-attachments/assets/9a3164e1-38dd-40e8-bfc9-92f1cf1cae30)
* asyncstorage-down - there is no usage of asyncstorage-down
([introduced here]
This one was added back, but most likely isn't needed and we need to
think about replacing `react-native-level-fs` and use only one file
system library


* dnode - there is no usage of dnode ([introduced
here](#2798))

![image](https://github.com/user-attachments/assets/deb346d7-8de3-4681-831f-5aa8bdcf2924)##
**Related issues**

![image](https://github.com/user-attachments/assets/04336975-6bea-4f17-96b7-91c942ed4100)Fixes:
* obs-store - There is no usage of obs-store anymoer (introruced
[here](#1236))

![image](https://github.com/user-attachments/assets/da362f05-2c71-4628-9c94-2134bac48c18)

![image](https://github.com/user-attachments/assets/37af9c8c-566e-41d0-9501-5503aa8ab648)
* react-native-flash-message - there is no usage anymore ([introduced
here](#447))

![image](https://github.com/user-attachments/assets/5e76cad3-b675-4139-8884-6464a2a54115)

![image](https://github.com/user-attachments/assets/155d0283-bcff-43b2-8913-a16b440c2efc)
* react-native-redash - there is no usage anymore (react-native-redash)

![image](https://github.com/user-attachments/assets/62037135-ec89-42f2-9476-9db95b0144b3)

![image](https://github.com/user-attachments/assets/9da3f797-9f43-49a8-be91-f4860299158c)


## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants