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

[Wallet] Fix disconnecting banner overlap in wallet home #1765

Merged
merged 9 commits into from
Nov 19, 2019

Conversation

annakaz
Copy link
Contributor

@annakaz annakaz commented Nov 19, 2019

Description

Tiny PR fixing the disconnected banner overlap in wallet home. This does not affect the UI when the banner is not shown

Tested

Added and ran jest tests

Related issues

Previously:
66563704-00004300-eb7c-11e9-9584-ec53a25de9dd

Now:
Screenshot_20191115-160300

@codecov
Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #1765 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1765      +/-   ##
==========================================
+ Coverage   74.17%   74.21%   +0.03%     
==========================================
  Files         278      278              
  Lines        7652     7652              
  Branches      957      957              
==========================================
+ Hits         5676     5679       +3     
+ Misses       1858     1856       -2     
+ Partials      118      117       -1
Flag Coverage Δ
#mobile 74.21% <ø> (+0.03%) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/home/WalletHome.tsx 90.8% <ø> (+1.14%) ⬆️
packages/mobile/src/home/useBalanceAutoRefresh.ts 87.5% <0%> (+25%) ⬆️

Continue to review full report at Codecov.

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

@jeanregisser
Copy link
Contributor

The "Connecting to Celo..." is also moved down a little, right?

Copy link
Contributor

@jeanregisser jeanregisser left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for fixing this! 🎉
I wish we had a less invasive UI connection status element. 👀

hideAlert={jest.fn()}
importContacts={jest.fn()}
loading={false}
appConnected={false}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this value gets overwritten by the value set in the provider store. I don't see the banner in the snapshot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updated to use createMockStoreAppDisconnected().

@annakaz annakaz added the automerge Have PR merge automatically when checks pass label Nov 19, 2019
@annakaz annakaz added automerge Have PR merge automatically when checks pass and removed automerge Have PR merge automatically when checks pass labels Nov 19, 2019
@celo-ci-bot-user celo-ci-bot-user merged commit 14813b0 into master Nov 19, 2019
@celo-ci-bot-user celo-ci-bot-user deleted the annakaz/minor-bugs branch November 19, 2019 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android] Error “Bad Connection...” is overlaps on QR code icon when device internet connection is bad.
6 participants