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

fix(earn): Fix Earn Deposit bottom sheet bug on Android #6236

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jophish
Copy link
Contributor

@jophish jophish commented Nov 15, 2024

Description

For ACT-1398. Fixes an issue where the deposit bottom sheet was obscuring the PIN entry screen.

Test plan

Unit and manual tested, see video below:

earn-bottom-sheet-2024-11-15_16.08.44.mp4

Related issues

  • Fixes #[issue number here]

Backwards compatibility

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

  • Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.93%. Comparing base (ca09824) to head (25ac7d7).

Files with missing lines Patch % Lines
src/earn/EarnDepositBottomSheet.tsx 96.15% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main    #6236    +/-   ##
========================================
  Coverage   88.93%   88.93%            
========================================
  Files         737      737            
  Lines       31431    31440     +9     
  Branches     5837     5527   -310     
========================================
+ Hits        27952    27961     +9     
- Misses       3280     3435   +155     
+ Partials      199       44   -155     
Files with missing lines Coverage Δ
src/earn/EarnEnterAmount.tsx 89.45% <100.00%> (-0.10%) ⬇️
src/navigator/Screens.tsx 100.00% <100.00%> (ø)
src/earn/EarnDepositBottomSheet.tsx 97.80% <96.15%> (-0.95%) ⬇️

... and 66 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca09824...25ac7d7. Read the comment docs.

---- 🚨 Try these New Features:

@MuckT
Copy link
Collaborator

MuckT commented Nov 15, 2024

@jophish is it possible to leave the bottom sheet displayed after the pin input and close prior or shortly after the navigation to home?

@satish-ravi
Copy link
Contributor

+1 to Tom's comment, this means that if the user doesn't press continue again, they'll never know a tx was submitted right?

@jophish
Copy link
Contributor Author

jophish commented Nov 20, 2024

@satish-ravi @MuckT I've been trying to get this to work in various ways, but I just can't seem to figure out a way to get this to work reliably unfortunately. The PIN entry always seems to dismiss the bottom sheet screen.

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.

3 participants