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

refactor: improve address list style when creating a new wallet #1109

Merged
merged 4 commits into from Mar 6, 2019

Conversation

@luciorubeens
Copy link
Member

commented Mar 1, 2019

Proposed changes

  • Improve the design when hovering the new address
  • Change checkmark flag
  • Update refresh address button

Types of changes

  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

@ItsANameToo @alexbarnsley @j-a-m-l - please review this in the next few days. Be sure to explicitly select labels so I know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@j-a-m-l
Copy link
Contributor

left a comment

The transition is too fast now, it feels a little abrupt. Maybe 0.3 or 0.4 would make it smoother.

In 1 occasion I saw 6 wallets when I was refreshing them. I haven't been able to reproduce it again.

@codecov-io

This comment has been minimized.

Copy link

commented Mar 6, 2019

Codecov Report

Merging #1109 into develop will increase coverage by 0.18%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #1109      +/-   ##
==========================================
+ Coverage    40.22%   40.4%   +0.18%     
==========================================
  Files          209     207       -2     
  Lines         5467    5291     -176     
  Branches      1073    1039      -34     
==========================================
- Hits          2199    2138      -61     
+ Misses        3147    3038     -109     
+ Partials       121     115       -6
Impacted Files Coverage Δ
src/renderer/components/Button/ButtonReload.vue 100% <ø> (ø) ⬆️
src/renderer/pages/Wallet/WalletNew.vue 35.55% <33.33%> (ø) ⬆️
...ponents/Wallet/WalletDelegates/WalletDelegates.vue 39.13% <0%> (-5.61%) ⬇️
src/renderer/components/Input/InputText.vue 50% <0%> (ø) ⬆️
src/renderer/components/Network/NetworkModal.vue 4.22% <0%> (ø) ⬆️
src/renderer/models/profile.js 0% <0%> (ø) ⬆️
src/renderer/components/Input/InputDelegate.vue
...enderer/components/Wallet/WalletSelectDelegate.vue
... and 8 more

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 26da980...ac94e9f. Read the comment docs.

@luciorubeens

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2019

@j-a-m-l True that, I did not notice the transitions when refreshing the addresses. Take a look now.

@luciorubeens luciorubeens requested a review from j-a-m-l Mar 6, 2019

@j-a-m-l

j-a-m-l approved these changes Mar 6, 2019

Copy link
Contributor

left a comment

👌

@j-a-m-l j-a-m-l merged commit 59bdb6d into develop Mar 6, 2019

1 check passed

ci/circleci: test-node-11 Your tests passed on CircleCI!
Details

@ArkEcosystemBot ArkEcosystemBot deleted the refactor/choose-address branch Mar 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.