NMA-615 | Contacts Page | Unregistered Users #519

desamtralized · 2020-10-01T19:18:42Z

Issue being fixed or feature implemented

Related PR's and Dependencies

Screenshots / Videos

How Has This Been Tested?

QA (Mobile Team)

Checklist:

I have performed a self-review of my own code and added comments where necessary
I have added or updated relevant unit/integration/functional/e2e tests

…velop

wallet/src/de/schildbach/wallet/ui/MainActivity.kt

wallet/src/de/schildbach/wallet/util/WalletExtensions.kt

HashEngineering

I left a few comments. It seems that this PR is not based on the most recent commit from the evonet-develop branch, though, GitHub does not point this out.

This PR will have conflicts with #516 (Merging the Master Branch into evonet-develop to add many of the fixes for the past few releases of Dash Wallet). We will need to determine the best merge order.

One issue found is this:

From Home Screen with no username, click on the contacts Screen.
On Contacts Screen, click upgrade.
Cancel / go back to the home screen, the Contacts button in the footer is still highlighted
Clicking on the Contacts button on the footer doesn't return to the Contacts Screen

desamtralized · 2020-10-02T01:37:27Z

I left a few comments. It seems that this PR is not based on the most recent commit from the evonet-develop branch, though, GitHub does not point this out.

This PR will have conflicts with #516 (Merging the Master Branch into evonet-develop to add many of the fixes for the past few releases of Dash Wallet). We will need to determine the best merge order.

One issue found is this:

From Home Screen with no username, click on the contacts Screen.

On Contacts Screen, click upgrade.

Cancel / go back to the home screen, the Contacts button in the footer is still highlighted

Clicking on the Contacts button on the footer doesn't return to the Contacts Screen

Good find, @HashEngineering! Issue addressed.

…ity and fragments with modern one based on shared view model.

tomasz-ludek · 2020-10-02T08:31:19Z

I'm all for refactoring WalletFragment and MainActivity but let's do it right!
I've limited time today so decided to commit proposed improvements instead of just describing them in comments.
Please have a look at ef6db8a
First of all callback based communication between fragment and activity is so old school... Let's switch to shared view model: https://developer.android.com/topic/libraries/architecture/viewmodel#sharing
Also, in DashPayViewModel we should keep only common methods/LiveDatas used by different Activities/Fragments whereas less common stuff should be kept in dedicated ViewModels. That is why isPlatformAvailable has been moved to MainActivityViewModel.

@sambarboza I think UpgradeToEvolutionFragment should listen to WalletCoinsReceivedEvent and WalletCoinsSentEvent in order to enable Upgrade button when wallet.canAffordIdentityCreation() (similarly as in WalletFragment), however instead of registering OnCoinsSentReceivedListener and CoinsSentEventListener in both fragments you could create a dedicated LiveData in MainActivityViewModel based on awesome ContactsBasedLiveData designed by @HashEngineering (instead of platformRepo.addContactsUpdatedListener you will have wallet.addCoinsReceivedEventListener, wallet.addCoinsSentEventListener).

Hopefully, all the changes I made are fully correct since I'm in hurry...

@HashEngineering let's merge this PR first (after improving it a bit) ant then l'll resolve all the conflicts with #516.

tomasz-ludek · 2020-10-02T08:34:46Z

wallet/src/de/schildbach/wallet/ui/MainActivity.kt

+        viewModel.blockchainStateData.observe(this, Observer {
+            // just to trigger data loading
+        })
+        viewModel.isPlatformAvailableData.observe(this, Observer {
+            // just to trigger data loading
+        })


Those empty observers could be replaced by other way of triggering data loading but let's leave it as it is
I'll fix it when working on "JoinDash" shortcut when #516 in merged (it will be merged after this PR)

tomasz-ludek · 2020-10-02T08:37:22Z

wallet/src/de/schildbach/wallet/ui/WalletFragment.kt

@@ -119,27 +132,7 @@ class WalletFragment : Fragment() {
        showHideJoinDashPayAction()
    }

-    fun setBlockchainState(blockchainState: BlockchainState?) {
-        this.blockchainState = blockchainState
-        if (isDetached || !isVisible) {


this shouldn't be needed when using LiveDatas

tomasz-ludek · 2020-10-02T10:39:04Z

I couldn't resist myself from implementing further improvements in MainActivityViewModel.
Please have a look 3cc66d3
The most important part is MainActivityViewModel.isAbleToCreateIdentityData which combines all the needed data all together.
WalletBalanceBasedLiveData based on mentioned before Eric's ContactsBasedLiveData.

…no-user # Conflicts: # wallet/res/values-th/strings-extra.xml # wallet/src/de/schildbach/wallet/ui/WalletFragment.kt

Minor code refactoring.

HashEngineering

Looks good with the new LiveData's. Works well.

desamtralized added 13 commits July 28, 2020 22:29

Removing wallet fragment before recreating activity

0698f10

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

6f08871

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

6bbb759

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

42c99c4

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

af16683

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

4dde9e0

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

909ef0b

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

e4ce62c

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

262c0bf

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

80e5932

…velop

Merge remote-tracking branch 'upstream/evonet-develop' into evonet-de…

088bbbb

…velop

Upgrade to Evolution page.

da1f349

Removed commented code

de3d53e

HashEngineering requested a review from tomasz-ludek October 1, 2020 21:39

HashEngineering reviewed Oct 1, 2020

View reviewed changes

wallet/src/de/schildbach/wallet/ui/MainActivity.kt Outdated Show resolved Hide resolved

HashEngineering reviewed Oct 1, 2020

View reviewed changes

wallet/src/de/schildbach/wallet/util/WalletExtensions.kt Show resolved Hide resolved

HashEngineering reviewed Oct 1, 2020

View reviewed changes

desamtralized added 2 commits October 1, 2020 19:59

Using Async version of db fetch

964b74e

Fixed stuck contacts icon

a50998a

Replaced old school callbacks-based communication model between activ…

ef6db8a

…ity and fragments with modern one based on shared view model.

tomasz-ludek reviewed Oct 2, 2020

View reviewed changes

Further improvements of MainActivityViewModel

3cc66d3

Minor code refactoring

620cc98

tomasz-ludek mentioned this pull request Oct 6, 2020

DashPay: Contacts Page / No User - View Model Changes #521

Closed

3 tasks

tomasz-ludek added 2 commits October 6, 2020 13:35

Merge branch 'master-into-evonet-develop' into dashpay-contacts-page-…

cd1d505

…no-user # Conflicts: # wallet/res/values-th/strings-extra.xml # wallet/src/de/schildbach/wallet/ui/WalletFragment.kt

Merge branch 'evonet-develop' into dashpay-contacts-page-no-user

5f7afd4

Final improvements of handling Join DashPay button visibility.

b912e2c

Minor code refactoring.

HashEngineering approved these changes Oct 6, 2020

View reviewed changes

HashEngineering merged commit 19a683e into dashpay:evonet-develop Oct 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NMA-615 | Contacts Page | Unregistered Users #519

NMA-615 | Contacts Page | Unregistered Users #519

desamtralized commented Oct 1, 2020

HashEngineering left a comment •

edited

Loading

desamtralized commented Oct 2, 2020

tomasz-ludek commented Oct 2, 2020 •

edited

Loading

tomasz-ludek Oct 2, 2020

tomasz-ludek Oct 2, 2020

tomasz-ludek commented Oct 2, 2020

HashEngineering left a comment

NMA-615 | Contacts Page | Unregistered Users #519

NMA-615 | Contacts Page | Unregistered Users #519

Conversation

desamtralized commented Oct 1, 2020

Issue being fixed or feature implemented

Related PR's and Dependencies

Screenshots / Videos

How Has This Been Tested?

Checklist:

HashEngineering left a comment • edited Loading

Choose a reason for hiding this comment

desamtralized commented Oct 2, 2020

tomasz-ludek commented Oct 2, 2020 • edited Loading

tomasz-ludek Oct 2, 2020

Choose a reason for hiding this comment

tomasz-ludek Oct 2, 2020

Choose a reason for hiding this comment

tomasz-ludek commented Oct 2, 2020

HashEngineering left a comment

Choose a reason for hiding this comment

HashEngineering left a comment •

edited

Loading

tomasz-ludek commented Oct 2, 2020 •

edited

Loading