feat(dashdirect): add login function with temporary UI #1068
Conversation
| @@ -111,6 +111,9 @@ public class Configuration { | |||
| public static final String PREFS_KEY_HAS_LOCATION_DIALOG_BEEN_SHOWN = "has_location_dialog_been_shown"; | |||
| public static final String PREFS_KEY_EXPLORE_DATABASE_NAME = "explore_database_name"; | |||
|
|
|||
| // DashDirect | |||
| public static final String PREFS_KEY_LAST_DASHDIRECT_ACCESS_TOKEN = "last_dash_direct_access_token"; | |||
There was a problem hiding this comment.
@hadia let's not add any new prefs into this file, it will end up being very large otherwise. We want to move feature-specific preferences into dedicated datastore configs. Please see CoinbaseConfig and CrowdNodeConfig for reference.
So you can create an ExploreConfig for all of the explore preferences and put this in there. The rest of Explore prefs will be migrated there with time.
| viewModel.dashDirectSignIn.observe(viewLifecycleOwner) { | ||
| if (it) { | ||
| // TODO open Buy card UI | ||
| Toast.makeText(requireContext(), "Open Buy Card", Toast.LENGTH_SHORT).show() | ||
| viewModel.logEvent(AnalyticsConstants.Explore.MERCHANT_DETAILS_BUY_GIFT_CARD) | ||
| } | ||
| } |
There was a problem hiding this comment.
The pattern of calling a function and observing result in LiveData is not great. It makes the execution flow more complicated and hard to track.
Instead, simply call a suspend function in a lifecycleScope and process the result.
| private val _dashDirectSignIn = MutableLiveData<Boolean>() | ||
| val dashDirectSignIn: LiveData<Boolean> | ||
| get() = _dashDirectSignIn |
| return | ||
| } | ||
|
|
||
| viewModelScope.launch { |
There was a problem hiding this comment.
Just make signInToDashDirect suspend and return the result directly. See below.
There was a problem hiding this comment.
This is better. Some more points to improve in the future:
runBlockingusage is discouraged. It's better to have the method that deals with settings assuspend. Reading from prefs is a IO operation and should be treated in an async manner.- Returning
ResponseResourcefrom the viewModel method is unnecessary. Just return the result or throw an error, thentry/catchin the caller.
|
The base branch is called |
HashEngineering
changed the base branch from
feature/DashDirect
to
feature-dashdirect
HashEngineering
changed the title
HashEngineering
changed the title
|
new branch created named |
Issue being fixed or feature implemented
Add sign in dialog for dashdirect Api
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist: