From 5f6e7839ab15e2188867bb28430634ef12b4f8ab Mon Sep 17 00:00:00 2001 From: Pavlos Tzegiannakis Date: Mon, 27 Jan 2025 15:39:52 +0200 Subject: [PATCH] FE-1564 Feature/fe 1498 refactor rangeselectorview as a composable (#474) * [WIP] Range selector view as composable * Fix issue of charts not loading properly when API replies very fast --- .../java/com/weatherxm/ui/common/Contracts.kt | 1 + .../ui/components/RangeSelectorView.kt | 54 ------------ .../components/compose/RangeSelectorView.kt | 87 +++++++++++++++++++ .../ui/devicesrewards/DeviceRewardsAdapter.kt | 31 ++++--- .../devicesrewards/DevicesRewardsActivity.kt | 20 +++-- .../devicesrewards/DevicesRewardsViewModel.kt | 47 +++++++--- .../ui/networkstats/NetworkStatsViewModel.kt | 7 ++ .../drawable/background_rounded_layer_1.xml | 7 -- .../res/layout/activity_devices_rewards.xml | 5 +- .../res/layout/list_item_device_rewards.xml | 5 +- .../main/res/layout/view_range_selector.xml | 39 --------- app/src/main/res/values/styles_widget.xml | 12 --- .../DevicesRewardsViewModelTest.kt | 4 +- 13 files changed, 170 insertions(+), 149 deletions(-) delete mode 100644 app/src/main/java/com/weatherxm/ui/components/RangeSelectorView.kt create mode 100644 app/src/main/java/com/weatherxm/ui/components/compose/RangeSelectorView.kt delete mode 100644 app/src/main/res/drawable/background_rounded_layer_1.xml delete mode 100644 app/src/main/res/layout/view_range_selector.xml diff --git a/app/src/main/java/com/weatherxm/ui/common/Contracts.kt b/app/src/main/java/com/weatherxm/ui/common/Contracts.kt index 14b68facf..fc0f1d82c 100644 --- a/app/src/main/java/com/weatherxm/ui/common/Contracts.kt +++ b/app/src/main/java/com/weatherxm/ui/common/Contracts.kt @@ -1,6 +1,7 @@ package com.weatherxm.ui.common object Contracts { + const val LOADING_DELAY = 200L const val EMPTY_VALUE = "?" const val DEGREES_MARK = "°" const val NOT_AVAILABLE_VALUE = "N/A" diff --git a/app/src/main/java/com/weatherxm/ui/components/RangeSelectorView.kt b/app/src/main/java/com/weatherxm/ui/components/RangeSelectorView.kt deleted file mode 100644 index 22c256bd4..000000000 --- a/app/src/main/java/com/weatherxm/ui/components/RangeSelectorView.kt +++ /dev/null @@ -1,54 +0,0 @@ -package com.weatherxm.ui.components - -import android.content.Context -import android.util.AttributeSet -import android.view.LayoutInflater -import androidx.constraintlayout.widget.ConstraintLayout -import com.weatherxm.R -import com.weatherxm.databinding.ViewRangeSelectorBinding - -class RangeSelectorView : ConstraintLayout { - - private lateinit var binding: ViewRangeSelectorBinding - - constructor(context: Context) : super(context) { - onCreate(context) - } - - constructor(context: Context, attrs: AttributeSet?) : super(context, attrs) { - onCreate(context) - } - - constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super( - context, attrs, defStyleAttr - ) { - onCreate(context) - } - - private fun onCreate(context: Context) { - binding = ViewRangeSelectorBinding.inflate(LayoutInflater.from(context), this) - } - - fun checkWeek() = binding.chartRangeSelector.check(R.id.week) - fun checkMonth() = binding.chartRangeSelector.check(R.id.month) - - fun checkedChipId() = binding.chartRangeSelector.checkedChipId - - fun listener(listener: (Int) -> Unit) { - binding.chartRangeSelector.setOnCheckedStateChangeListener { _, checkedIds -> - if (checkedIds.isNotEmpty()) { - listener.invoke(checkedIds[0]) - } - } - } - - fun enable() { - binding.week.isEnabled = true - binding.month.isEnabled = true - } - - fun disable() { - binding.week.isEnabled = false - binding.month.isEnabled = false - } -} diff --git a/app/src/main/java/com/weatherxm/ui/components/compose/RangeSelectorView.kt b/app/src/main/java/com/weatherxm/ui/components/compose/RangeSelectorView.kt new file mode 100644 index 000000000..136eadf14 --- /dev/null +++ b/app/src/main/java/com/weatherxm/ui/components/compose/RangeSelectorView.kt @@ -0,0 +1,87 @@ +package com.weatherxm.ui.components.compose + +import androidx.compose.foundation.layout.Arrangement.spacedBy +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.selection.toggleable +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults +import androidx.compose.material3.Surface +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.colorResource +import androidx.compose.ui.res.dimensionResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview +import com.weatherxm.R + +@Suppress("FunctionNaming") +@Preview +@Composable +fun RangeSelectorView( + selectedChipLabelResId: Int = R.string.seven_days_abbr, + enabledToggle: Boolean = true, + onSelectedChanged: (Int) -> Unit = {}, +) { + val sevenDaysAbbrResId = R.string.seven_days_abbr + val oneMonthAbbrResId = R.string.one_month_abbr + Card( + colors = CardDefaults.cardColors( + containerColor = colorResource(R.color.layer1) + ), + shape = RoundedCornerShape(dimensionResource(R.dimen.radius_small)) + ) { + Row( + modifier = Modifier.padding(dimensionResource(R.dimen.padding_extra_small)), + horizontalArrangement = spacedBy(dimensionResource(R.dimen.margin_extra_small)) + ) { + Chip( + labelResId = sevenDaysAbbrResId, + isSelected = sevenDaysAbbrResId == selectedChipLabelResId, + enabledToggle = enabledToggle, + onSelectionChanged = { onSelectedChanged(it) }, + ) + Chip( + labelResId = oneMonthAbbrResId, + isSelected = oneMonthAbbrResId == selectedChipLabelResId, + enabledToggle = enabledToggle, + onSelectionChanged = { onSelectedChanged(it) }, + ) + } + } +} + +@Suppress("FunctionNaming") +@Preview +@Composable +fun Chip( + labelResId: Int = R.string.seven_days_abbr, + isSelected: Boolean = false, + enabledToggle: Boolean = true, + onSelectionChanged: (Int) -> Unit = {}, +) { + Surface( + shape = RoundedCornerShape(dimensionResource(R.dimen.radius_small)), + color = if (isSelected) colorResource(R.color.layer2) else colorResource(R.color.layer1) + ) { + Row(modifier = Modifier + .padding( + dimensionResource(R.dimen.padding_normal), + dimensionResource(R.dimen.padding_small_to_normal) + ) + .toggleable( + value = isSelected, + enabled = enabledToggle, + onValueChange = { + onSelectionChanged(labelResId) + } + ) + ) { + MediumText( + text = stringResource(labelResId), + colorRes = R.color.darkGrey, + ) + } + } +} diff --git a/app/src/main/java/com/weatherxm/ui/devicesrewards/DeviceRewardsAdapter.kt b/app/src/main/java/com/weatherxm/ui/devicesrewards/DeviceRewardsAdapter.kt index 6c4dc2660..47f0f8ca6 100644 --- a/app/src/main/java/com/weatherxm/ui/devicesrewards/DeviceRewardsAdapter.kt +++ b/app/src/main/java/com/weatherxm/ui/devicesrewards/DeviceRewardsAdapter.kt @@ -2,6 +2,8 @@ package com.weatherxm.ui.devicesrewards import android.view.LayoutInflater import android.view.ViewGroup +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf import androidx.core.view.isVisible import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.ListAdapter @@ -18,6 +20,7 @@ import com.weatherxm.ui.common.hide import com.weatherxm.ui.common.invisible import com.weatherxm.ui.common.show import com.weatherxm.ui.common.visible +import com.weatherxm.ui.components.compose.RangeSelectorView import com.weatherxm.util.NumberUtils.formatTokens import com.weatherxm.util.initRewardsBreakdownChart import org.koin.core.component.KoinComponent @@ -58,14 +61,22 @@ class DeviceRewardsAdapter( val adapter = DeviceRewardsBoostAdapter() private var ignoreRangeChipListener = false + private val selectedRangeChip = mutableIntStateOf(R.string.seven_days_abbr) + private val enabledRangeToggle = mutableStateOf(true) + fun bind(item: DeviceTotalRewards) { binding.headerCard.setOnClickListener { onExpandClick(item) } - binding.chartRangeSelector.listener { - if (!ignoreRangeChipListener) { - onFetchNewData.invoke(item.id, absoluteAdapterPosition, it) + binding.chartRangeSelector.setContent { + RangeSelectorView( + selectedChipLabelResId = selectedRangeChip.intValue, + enabledToggle = enabledRangeToggle.value + ) { + if (!ignoreRangeChipListener && selectedRangeChip.intValue != it) { + onFetchNewData.invoke(item.id, absoluteAdapterPosition, it) + } } } @@ -87,7 +98,7 @@ class DeviceRewardsAdapter( onError(item.id) } Status.LOADING -> { - binding.chartRangeSelector.disable() + enabledRangeToggle.value = false binding.detailsContainer.invisible() binding.earnedBy.invisible() binding.retryCard.visible(false) @@ -99,8 +110,8 @@ class DeviceRewardsAdapter( private fun preCheckMode(mode: RewardsSummaryMode?) { ignoreRangeChipListener = true when (mode) { - RewardsSummaryMode.WEEK -> binding.chartRangeSelector.checkWeek() - RewardsSummaryMode.MONTH -> binding.chartRangeSelector.checkMonth() + RewardsSummaryMode.WEEK -> selectedRangeChip.intValue = R.string.seven_days_abbr + RewardsSummaryMode.MONTH -> selectedRangeChip.intValue = R.string.one_month_abbr else -> throw NotImplementedError("Unknown rewards mode $mode") }.also { ignoreRangeChipListener = false @@ -126,7 +137,7 @@ class DeviceRewardsAdapter( binding.othersRewardsLegend.visible(details.otherChartData.isDataValid()) binding.retryCard.visible(false) binding.detailsStatus.visible(false) - binding.chartRangeSelector.enable() + enabledRangeToggle.value = true binding.earnedBy.visible(true) binding.detailsContainer.visible(true) } @@ -159,7 +170,7 @@ class DeviceRewardsAdapter( onFetchNewData.invoke( item.id, absoluteAdapterPosition, - binding.chartRangeSelector.checkedChipId() + selectedRangeChip.intValue ) } else { binding.detailsWithLoadingContainer.show() @@ -187,11 +198,11 @@ class DeviceRewardsAdapter( onFetchNewData.invoke( deviceId, absoluteAdapterPosition, - binding.chartRangeSelector.checkedChipId() + selectedRangeChip.intValue ) } } - binding.chartRangeSelector.enable() + enabledRangeToggle.value = true binding.retryCard.visible(true) } } diff --git a/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsActivity.kt b/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsActivity.kt index 825ce54b0..8b020b613 100644 --- a/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsActivity.kt +++ b/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsActivity.kt @@ -30,6 +30,7 @@ import com.weatherxm.ui.common.parcelable import com.weatherxm.ui.common.visible import com.weatherxm.ui.components.BaseActivity import com.weatherxm.ui.components.compose.LargeText +import com.weatherxm.ui.components.compose.RangeSelectorView import com.weatherxm.util.NumberUtils.formatTokens import com.weatherxm.util.initTotalEarnedChart import org.koin.androidx.viewmodel.ext.android.viewModel @@ -56,8 +57,15 @@ class DevicesRewardsActivity : BaseActivity() { navigator.openWebsite(this, getString(R.string.shop_url)) } - binding.totalEarnedRangeSelector.listener { - model.getDevicesRewardsByRangeTotals(it) + binding.totalEarnedRangeSelector.setContent { + RangeSelectorView( + selectedChipLabelResId = model.totalSelectedRangeChip(), + enabledToggle = model.totalRangeChipsToggleEnabled() + ) { + if (model.totalSelectedRangeChip() != it) { + model.getDevicesRewardsByRangeTotals(it) + } + } } model.onRewardsByRange().observe(this) { @@ -65,7 +73,6 @@ class DevicesRewardsActivity : BaseActivity() { Status.SUCCESS -> { binding.totalEarnedStatus.visible(false) binding.retryCard.visible(false) - binding.totalEarnedRangeSelector.enable() binding.totalEarned.text = getString(R.string.wxm_amount, formatTokens(it.data?.total)) it.data?.let { data -> @@ -81,7 +88,6 @@ class DevicesRewardsActivity : BaseActivity() { onError() } Status.LOADING -> { - binding.totalEarnedRangeSelector.disable() binding.totalEarnedChart.invisible() binding.totalEarned.invisible() binding.retryCard.visible(false) @@ -119,7 +125,6 @@ class DevicesRewardsActivity : BaseActivity() { adapter.submitList(model.rewards.devices) - binding.totalEarnedRangeSelector.checkWeek() model.getDevicesRewardsByRangeTotals() model.getDeviceRewardsByRange(model.rewards.devices[0].id, 0) } else { @@ -133,15 +138,12 @@ class DevicesRewardsActivity : BaseActivity() { } private fun onError() { - binding.totalEarnedRangeSelector.enable() binding.totalEarnedChart.invisible() binding.totalEarned.invisible() binding.totalEarnedStatus.visible(false) binding.retryCard.setContent { RetryCard { - model.getDevicesRewardsByRangeTotals( - binding.totalEarnedRangeSelector.checkedChipId() - ) + model.getDevicesRewardsByRangeTotals() } } binding.retryCard.visible(true) diff --git a/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModel.kt b/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModel.kt index d2d363878..14c73fc38 100644 --- a/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModel.kt +++ b/app/src/main/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModel.kt @@ -1,5 +1,7 @@ package com.weatherxm.ui.devicesrewards +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel @@ -7,6 +9,7 @@ import androidx.lifecycle.viewModelScope import com.weatherxm.R import com.weatherxm.analytics.AnalyticsWrapper import com.weatherxm.data.repository.RewardsRepositoryImpl +import com.weatherxm.ui.common.Contracts.LOADING_DELAY import com.weatherxm.ui.common.DeviceTotalRewardsDetails import com.weatherxm.ui.common.DevicesRewards import com.weatherxm.ui.common.DevicesRewardsByRange @@ -17,6 +20,7 @@ import com.weatherxm.util.Failure.getDefaultMessage import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.launch class DevicesRewardsViewModel( @@ -25,6 +29,12 @@ class DevicesRewardsViewModel( private val analytics: AnalyticsWrapper, private val dispatcher: CoroutineDispatcher = Dispatchers.IO ) : ViewModel() { + private val totalSelectedRangeChip = mutableIntStateOf(R.string.seven_days_abbr) + private val totalRangeChipsToggleEnabled = mutableStateOf(true) + + fun totalSelectedRangeChip(): Int = totalSelectedRangeChip.intValue + fun totalRangeChipsToggleEnabled(): Boolean = totalRangeChipsToggleEnabled.value + private val devicesJobs = mutableMapOf() /** @@ -38,31 +48,43 @@ class DevicesRewardsViewModel( fun onRewardsByRange(): LiveData> = onRewardsByRange - fun getDevicesRewardsByRangeTotals(checkedRangeChipId: Int = R.id.week) { + fun getDevicesRewardsByRangeTotals(selectedRangeChipId: Int = totalSelectedRangeChip.intValue) { + totalSelectedRangeChip.intValue = selectedRangeChipId + viewModelScope.launch(dispatcher) { onRewardsByRange.postValue(Resource.loading()) - - val mode = chipToMode(checkedRangeChipId) + totalRangeChipsToggleEnabled.value = false + /** + * Needed due to an issue with the chart drawing if the API replies very fast: + * https://linear.app/weatherxm/issue/FE-1564/fix-chart-in-devices-rewards-showing-no-data-at-first-open + */ + delay(LOADING_DELAY) + val mode = labelResIdToMode(selectedRangeChipId) usecase.getDevicesRewardsByRange(mode).onRight { onRewardsByRange.postValue(Resource.success(it)) }.onLeft { onRewardsByRange.postValue(Resource.error(it.getDefaultMessage())) analytics.trackEventFailure(it.code) } + totalRangeChipsToggleEnabled.value = true } } fun getDeviceRewardsByRange( deviceId: String, position: Int, - checkedRangeChipId: Int = R.id.week + selectedRangeChipId: Int = R.string.seven_days_abbr ) { val job = viewModelScope.launch(dispatcher) { - val selectedMode = chipToMode(checkedRangeChipId) + val selectedMode = labelResIdToMode(selectedRangeChipId) rewards.devices[position].details.status = Status.LOADING rewards.devices[position].details.mode = selectedMode onDeviceRewardDetails.postValue(Pair(position, rewards.devices[position].details)) - + /** + * Needed due to an issue with the chart drawing if the API replies very fast: + * https://linear.app/weatherxm/issue/FE-1564/fix-chart-in-devices-rewards-showing-no-data-at-first-open + */ + delay(LOADING_DELAY) usecase.getDeviceRewardsByRange(deviceId, selectedMode).onRight { rewards.devices[position].details = it onDeviceRewardDetails.postValue(Pair(position, it)) @@ -83,12 +105,13 @@ class DevicesRewardsViewModel( devicesJobs[position]?.cancel() } - private fun chipToMode(chipId: Int): RewardsRepositoryImpl.Companion.RewardsSummaryMode { - return when (chipId) { - R.id.week -> RewardsRepositoryImpl.Companion.RewardsSummaryMode.WEEK - R.id.month -> RewardsRepositoryImpl.Companion.RewardsSummaryMode.MONTH - else -> throw NotImplementedError("Unknown chip ID $chipId") + private fun labelResIdToMode( + labelResId: Int + ): RewardsRepositoryImpl.Companion.RewardsSummaryMode { + return when (labelResId) { + R.string.seven_days_abbr -> RewardsRepositoryImpl.Companion.RewardsSummaryMode.WEEK + R.string.one_month_abbr -> RewardsRepositoryImpl.Companion.RewardsSummaryMode.MONTH + else -> throw NotImplementedError("Unknown chip ID $labelResId") } } - } diff --git a/app/src/main/java/com/weatherxm/ui/networkstats/NetworkStatsViewModel.kt b/app/src/main/java/com/weatherxm/ui/networkstats/NetworkStatsViewModel.kt index 21aaa7bf5..a1dbb8ed7 100644 --- a/app/src/main/java/com/weatherxm/ui/networkstats/NetworkStatsViewModel.kt +++ b/app/src/main/java/com/weatherxm/ui/networkstats/NetworkStatsViewModel.kt @@ -3,11 +3,13 @@ package com.weatherxm.ui.networkstats import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.weatherxm.ui.common.Contracts.LOADING_DELAY import com.weatherxm.ui.common.Resource import com.weatherxm.usecases.StatsUseCase import com.weatherxm.util.Failure.getDefaultMessage import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import timber.log.Timber @@ -23,6 +25,11 @@ class NetworkStatsViewModel( fun getNetworkStats() { onNetworkStats.postValue(Resource.loading()) viewModelScope.launch(dispatcher) { + /** + * Needed due to an issue with the chart drawing if the API replies very fast: + * https://linear.app/weatherxm/issue/FE-1564/fix-chart-in-devices-rewards-showing-no-data-at-first-open + */ + delay(LOADING_DELAY) usecase.getNetworkStats() .onRight { onNetworkStats.postValue(Resource.success(it)) diff --git a/app/src/main/res/drawable/background_rounded_layer_1.xml b/app/src/main/res/drawable/background_rounded_layer_1.xml deleted file mode 100644 index a4593d8b2..000000000 --- a/app/src/main/res/drawable/background_rounded_layer_1.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - - - diff --git a/app/src/main/res/layout/activity_devices_rewards.xml b/app/src/main/res/layout/activity_devices_rewards.xml index 598f46463..9d0cd1c58 100644 --- a/app/src/main/res/layout/activity_devices_rewards.xml +++ b/app/src/main/res/layout/activity_devices_rewards.xml @@ -143,12 +143,13 @@ app:layout_constraintTop_toBottomOf="@id/totalEarnedTitle" tools:text="389,023.54 $WXM" /> - + app:layout_constraintTop_toTopOf="parent" + tools:composableName="com.weatherxm.ui.components.compose.RangeSelectorViewKt.RangeSelectorView" /> - + app:layout_constraintTop_toBottomOf="@id/rewardsBreakdown" + tools:composableName="com.weatherxm.ui.components.compose.RangeSelectorViewKt.RangeSelectorView" /> - - - - - - - - - - diff --git a/app/src/main/res/values/styles_widget.xml b/app/src/main/res/values/styles_widget.xml index 538acf52d..2f78ea9b0 100644 --- a/app/src/main/res/values/styles_widget.xml +++ b/app/src/main/res/values/styles_widget.xml @@ -160,18 +160,6 @@ 16dp - - diff --git a/app/src/test/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModelTest.kt b/app/src/test/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModelTest.kt index 824798297..248def520 100644 --- a/app/src/test/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModelTest.kt +++ b/app/src/test/java/com/weatherxm/ui/devicesrewards/DevicesRewardsViewModelTest.kt @@ -73,7 +73,7 @@ class DevicesRewardsViewModelTest : BehaviorSpec({ failure ) testHandleFailureViewModel( - { viewModel.getDevicesRewardsByRangeTotals(R.id.week) }, + { viewModel.getDevicesRewardsByRangeTotals(R.string.seven_days_abbr) }, analytics, viewModel.onRewardsByRange(), 1, @@ -101,7 +101,7 @@ class DevicesRewardsViewModelTest : BehaviorSpec({ failure ) erroneousDetails.mode = RewardsSummaryMode.MONTH - runTest { viewModel.getDeviceRewardsByRange(deviceId, 0, R.id.month) } + runTest { viewModel.getDeviceRewardsByRange(deviceId, 0, R.string.one_month_abbr) } then("the rewards object in the ViewModel should be updated") { viewModel.rewards.devices[0].details shouldBe erroneousDetails }