edge-react-gui icon indicating copy to clipboard operation
edge-react-gui copied to clipboard

Published 20 hours ago • verified publisher icon EdgeApp

Enable hidden wallets

Open bruscantini opened this issue 5 years ago • 11 comments
trafficstars

PR Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • [ ] Tested on iOS Tablet
  • [ ] Tested on small Android
  • [x] n/a

Not show "hidden" wallets on WalletListScene. Depends on changes from https://github.com/EdgeApp/edge-core-js/pull/267

bruscantini avatar Dec 19 '19 08:12 bruscantini

Hi @swansontec or @paullinator . The integration test is failing because of 'environment variables'. What should I do here? Thanks,

bruscantini avatar Jan 07 '20 10:01 bruscantini

@bruscantini Can you send a screenshot or copy/paste of the environment variable error you are seeing?

paullinator avatar Jan 10 '20 01:01 paullinator

As well, are you seeing this error when building the develop branch without your changes?

paullinator avatar Jan 10 '20 01:01 paullinator

Hi @paullinator ! I'm actually not seeing the error at all during building. But the Travis CI for this PR is throwing the error. I just want the PR to pass all automated checks before your review/merge. :)

Here's the link to the Travis CI error: https://travis-ci.org/EdgeApp/edge-react-gui/jobs/633649267?utm_medium=notification&utm_source=github_status

bruscantini avatar Jan 13 '20 07:01 bruscantini

@paullinator I know what this is. The way we store encrypted API keys in the Travis build system doesn't work when third parties initiate a build. Therefore, any PR's from outside the company are going to fail Travis automatically. We should probably just put some plain-text dummy API keys into Travis, rather than using encryption.

If the changes look good to you, Paul, feel free to merge, even without Travis passing.

swansontec avatar Jan 13 '20 15:01 swansontec

@swansontec Thanks for the info. I will review the pr in the next couple days.

paullinator avatar Jan 13 '20 23:01 paullinator

Hey @paullinator can you review and merge this PR when you get a chance? Once react-gui can hide wallets, we can link Edge in the Ember Fund app. Thanks!

bruscantini avatar Feb 05 '20 04:02 bruscantini

These changes look good but you also need to add the logic to exclude hidden wallets from the Wallet picker Modal

paullinator avatar Feb 05 '20 06:02 paullinator

@swansontec Can you confirm that the appropriate hidden wallets parameter is now in the Edge account object that comes from the core?

paullinator avatar Feb 05 '20 06:02 paullinator

Thanks @paullinator . I updated based on your comment. One thing, though. When running the edge-react-gui locally, account.hiddenWalletIds is empty. But when I run the Ember app, the EdgeAccount object I get has hiddenWalletIds populated.

Is this because I am setting the wallet state to hidden from the Ember app? The goal is to be able to hide the wallets from Ember app so that they do not appear in the edge-react-gui.

bruscantini avatar Feb 19 '20 10:02 bruscantini

It looks like this is blocked on the fact that the hidden flag is stored per-account, so setting it on the Ember account isn't setting it on the Edge Wallet account. We need to fix edge-core-js to sync this flag between accounts, and then we can refresh & merge this PR.

swansontec avatar May 11 '20 18:05 swansontec