Skip to content

Commit 4e1586c

Browse files
committed
 Fix list issue on VPN geoswitching page (#4712)
Task/Issue URL: https://app.asana.com/0/1201462763415876/1207704558359801/f ### Description Fixes issue related to view recycling in the VPN Geoswitching page ### Steps to test this PR See: https://app.asana.com/0/1201462763415876/1207717605286620/f
1 parent 98fd2c3 commit 4e1586c

File tree

9 files changed

+203
-384
lines changed

9 files changed

+203
-384
lines changed

network-protection/network-protection-impl/src/main/AndroidManifest.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
android:exported="false" />
3838
<activity
3939
android:name=".settings.geoswitching.NetpGeoswitchingActivity"
40+
android:configChanges="orientation|screenSize"
4041
android:label="@string/netpGeoswitchingTitle"
4142
android:parentActivityName="settings.NetPVpnSettingsActivity"
4243
android:exported="false" />

network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/settings/geoswitching/NetpGeoSwitchingViewModel.kt

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,8 @@ import com.duckduckgo.anvil.annotations.ContributesViewModel
2525
import com.duckduckgo.common.utils.DispatcherProvider
2626
import com.duckduckgo.di.scopes.ActivityScope
2727
import com.duckduckgo.networkprotection.api.NetworkProtectionState
28-
import com.duckduckgo.networkprotection.impl.R
2928
import com.duckduckgo.networkprotection.impl.configuration.WgServerDebugProvider
3029
import com.duckduckgo.networkprotection.impl.pixels.NetworkProtectionPixels
31-
import com.duckduckgo.networkprotection.impl.settings.geoswitching.GeoswitchingListItem.CountryItem
32-
import com.duckduckgo.networkprotection.impl.settings.geoswitching.GeoswitchingListItem.DividerItem
33-
import com.duckduckgo.networkprotection.impl.settings.geoswitching.GeoswitchingListItem.HeaderItem
34-
import com.duckduckgo.networkprotection.impl.settings.geoswitching.GeoswitchingListItem.RecommendedItem
3530
import com.duckduckgo.networkprotection.store.NetPGeoswitchingRepository
3631
import com.duckduckgo.networkprotection.store.NetPGeoswitchingRepository.UserPreferredLocation
3732
import javax.inject.Inject
@@ -55,17 +50,21 @@ class NetpGeoSwitchingViewModel @Inject constructor(
5550
internal fun viewState(): Flow<ViewState> = viewState.asStateFlow()
5651

5752
internal data class ViewState(
58-
val items: List<GeoswitchingListItem> = emptyList(),
53+
val currentSelectedCountry: String? = null,
54+
val items: List<CountryItem> = emptyList(),
55+
)
56+
57+
data class CountryItem(
58+
val countryCode: String,
59+
val countryEmoji: String,
60+
val countryName: String,
61+
val cities: List<String>,
5962
)
6063

6164
private var initialPreferredLocation: UserPreferredLocation? = null
6265
override fun onCreate(owner: LifecycleOwner) {
6366
super.onCreate(owner)
6467
networkProtectionPixels.reportGeoswitchingScreenShown()
65-
}
66-
67-
override fun onStart(owner: LifecycleOwner) {
68-
super.onStart(owner)
6968
viewModelScope.launch(dispatcherProvider.io()) {
7069
initialPreferredLocation = netPGeoswitchingRepository.getUserPreferredLocation()
7170
val countryItems = egressServersProvider.getServerLocations().map {
@@ -77,32 +76,21 @@ class NetpGeoSwitchingViewModel @Inject constructor(
7776
)
7877
}.sortedBy { it.countryName }
7978

80-
val completeList = mutableListOf(
81-
HeaderItem(R.string.netpGeoswitchingHeaderRecommended),
82-
RecommendedItem(
83-
title = R.string.netpGeoswitchingDefaultTitle,
84-
subtitle = R.string.netpGeoswitchingDefaultSubtitle,
85-
),
86-
).apply {
87-
if (countryItems.isNotEmpty()) {
88-
this.add(DividerItem)
89-
this.add(HeaderItem(R.string.netpGeoswitchingHeaderCustom))
90-
this.addAll(countryItems)
91-
} else {
92-
networkProtectionPixels.reportGeoswitchingNoLocations()
93-
}
79+
if (countryItems.isEmpty()) {
80+
networkProtectionPixels.reportGeoswitchingNoLocations()
9481
}
9582

9683
viewState.emit(
9784
ViewState(
98-
items = completeList,
85+
currentSelectedCountry = getSelectedCountryCode(),
86+
items = countryItems,
9987
),
10088
)
10189
}
10290
}
10391

104-
override fun onStop(owner: LifecycleOwner) {
105-
super.onStop(owner)
92+
override fun onDestroy(owner: LifecycleOwner) {
93+
super.onDestroy(owner)
10694
viewModelScope.launch(dispatcherProvider.io()) {
10795
val newPreferredLocation = netPGeoswitchingRepository.getUserPreferredLocation()
10896
if (networkProtectionState.isEnabled()) {
@@ -121,7 +109,7 @@ class NetpGeoSwitchingViewModel @Inject constructor(
121109
}
122110
}
123111

124-
fun getSelectedCountryCode(): String? {
112+
private fun getSelectedCountryCode(): String? {
125113
return runBlocking { netPGeoswitchingRepository.getUserPreferredLocation().countryCode }
126114
}
127115

network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/settings/geoswitching/NetpGeoswitchingActivity.kt

Lines changed: 107 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,25 @@
1717
package com.duckduckgo.networkprotection.impl.settings.geoswitching
1818

1919
import android.os.Bundle
20+
import android.view.LayoutInflater
21+
import android.widget.CompoundButton
2022
import androidx.lifecycle.Lifecycle
2123
import androidx.lifecycle.flowWithLifecycle
2224
import androidx.lifecycle.lifecycleScope
2325
import com.duckduckgo.anvil.annotations.ContributeToActivityStarter
2426
import com.duckduckgo.anvil.annotations.InjectWith
2527
import com.duckduckgo.common.ui.DuckDuckGoActivity
28+
import com.duckduckgo.common.ui.view.gone
29+
import com.duckduckgo.common.ui.view.show
2630
import com.duckduckgo.common.ui.viewbinding.viewBinding
2731
import com.duckduckgo.di.scopes.ActivityScope
2832
import com.duckduckgo.navigation.api.GlobalActivityStarter
33+
import com.duckduckgo.networkprotection.impl.R
2934
import com.duckduckgo.networkprotection.impl.databinding.ActivityNetpGeoswitchingBinding
35+
import com.duckduckgo.networkprotection.impl.databinding.ItemGeoswitchingCountryBinding
36+
import com.duckduckgo.networkprotection.impl.settings.geoswitching.NetpGeoSwitchingViewModel.CountryItem
3037
import com.duckduckgo.networkprotection.impl.settings.geoswitching.NetpGeoSwitchingViewModel.ViewState
38+
import kotlinx.coroutines.flow.distinctUntilChanged
3139
import kotlinx.coroutines.flow.launchIn
3240
import kotlinx.coroutines.flow.onEach
3341

@@ -36,13 +44,12 @@ import kotlinx.coroutines.flow.onEach
3644
class NetpGeoswitchingActivity : DuckDuckGoActivity() {
3745
private val binding: ActivityNetpGeoswitchingBinding by viewBinding()
3846
private val viewModel: NetpGeoSwitchingViewModel by bindViewModel()
39-
private lateinit var adapter: NetpGeoswitchingAdapter
47+
private lateinit var lastSelectedButton: CompoundButton
4048

4149
override fun onCreate(savedInstanceState: Bundle?) {
4250
super.onCreate(savedInstanceState)
4351
setContentView(binding.root)
4452
setupToolbar(binding.includeToolbar.toolbar)
45-
bindViews()
4653
observeViewModel()
4754
lifecycle.addObserver(viewModel)
4855
}
@@ -52,39 +59,118 @@ class NetpGeoswitchingActivity : DuckDuckGoActivity() {
5259
lifecycle.removeObserver(viewModel)
5360
}
5461

55-
private fun bindViews() {
56-
adapter = NetpGeoswitchingAdapter(
57-
viewModel.getSelectedCountryCode(),
58-
onItemMenuClicked = { country, cities ->
59-
NetpGeoswitchingCityChoiceDialogFragment.instance(
60-
country,
61-
ArrayList(cities),
62-
).show(supportFragmentManager, TAG_DIALOG_CITY_CHOICE)
63-
},
64-
onCountrySelected = {
65-
viewModel.onCountrySelected(it)
66-
},
67-
onNearestAvailableSelected = {
68-
viewModel.onNearestAvailableCountrySelected()
69-
},
70-
)
71-
binding.geoswitchingRecycler.adapter = adapter
62+
private fun onItemMenuClicked(
63+
country: String,
64+
cities: List<String>,
65+
) {
66+
NetpGeoswitchingCityChoiceDialogFragment.instance(
67+
country,
68+
ArrayList(cities),
69+
).show(supportFragmentManager, TAG_DIALOG_CITY_CHOICE)
7270
}
7371

7472
private fun observeViewModel() {
7573
viewModel.viewState()
7674
.flowWithLifecycle(lifecycle, Lifecycle.State.STARTED)
75+
.distinctUntilChanged()
7776
.onEach { renderViewState(it) }
7877
.launchIn(lifecycleScope)
7978
}
8079

8180
private fun renderViewState(viewState: ViewState) {
82-
adapter.submitList(viewState.items)
81+
bindRecommendedItem(viewState.currentSelectedCountry)
82+
83+
if (viewState.items.isEmpty()) {
84+
binding.customListHeader.gone()
85+
} else {
86+
binding.customListHeader.show()
87+
}
88+
89+
viewState.items.forEach {
90+
val itemBinding = ItemGeoswitchingCountryBinding.inflate(
91+
LayoutInflater.from(binding.geoswitchingList.context),
92+
binding.geoswitchingList,
93+
false,
94+
)
95+
96+
it.bindLocationItem(itemBinding, viewState.currentSelectedCountry)
97+
binding.geoswitchingList.addView(itemBinding.root)
98+
}
99+
}
100+
101+
private fun bindRecommendedItem(currentSelectedCountryCode: String?) {
102+
with(binding.recommendedLocationItem) {
103+
// Sets initial state
104+
this.radioButton.isChecked = currentSelectedCountryCode.isNullOrEmpty()
105+
106+
if (currentSelectedCountryCode.isNullOrEmpty()) {
107+
lastSelectedButton = this.radioButton
108+
}
109+
110+
this.radioButton.setOnCheckedChangeListener { view, isChecked ->
111+
if (isChecked && view != lastSelectedButton) {
112+
lastSelectedButton.isChecked = false
113+
lastSelectedButton = view
114+
viewModel.onNearestAvailableCountrySelected()
115+
}
116+
}
117+
// Automatically selects the country when the item is clicked
118+
this.setClickListener {
119+
this.radioButton.isChecked = true
120+
}
121+
}
122+
}
123+
124+
private fun CountryItem.bindLocationItem(
125+
itemBinding: ItemGeoswitchingCountryBinding,
126+
currentSelectedCountryCode: String?,
127+
) {
128+
// Sets initial state
129+
itemBinding.root.radioButton.isChecked = currentSelectedCountryCode == this.countryCode
130+
if (currentSelectedCountryCode == this.countryCode) {
131+
lastSelectedButton = itemBinding.root.radioButton
132+
}
133+
134+
itemBinding.root.setPrimaryText(this.countryName)
135+
itemBinding.root.setLeadingEmojiIcon(countryEmoji)
136+
137+
if (cities.size > 1) {
138+
itemBinding.root.setSecondaryText(
139+
String.format(
140+
this@NetpGeoswitchingActivity.getString(R.string.netpGeoswitchingHeaderCountrySubtitle),
141+
cities.size,
142+
),
143+
)
144+
itemBinding.root.trailingIconContainer.show()
145+
itemBinding.root.setTrailingIconClickListener {
146+
// Automatically select the country before the user can choose the specific city
147+
itemBinding.root.radioButton.isChecked = true
148+
onItemMenuClicked(countryName, cities)
149+
}
150+
} else {
151+
itemBinding.root.secondaryText.gone()
152+
itemBinding.root.trailingIconContainer.gone()
153+
}
154+
155+
itemBinding.root.radioButton.setOnCheckedChangeListener { view, isChecked ->
156+
if (isChecked && view != lastSelectedButton) {
157+
lastSelectedButton.isChecked = false
158+
lastSelectedButton = view
159+
viewModel.onCountrySelected(this.countryCode)
160+
}
161+
}
162+
163+
// Automatically selects the country when the item is clicked
164+
itemBinding.root.setClickListener {
165+
itemBinding.root.radioButton.isChecked = true
166+
}
83167
}
84168

85169
companion object {
86170
private const val TAG_DIALOG_CITY_CHOICE = "DIALOG_CITY_CHOICE"
87171
}
88172
}
89173

90-
internal object NetpGeoswitchingScreenNoParams : GlobalActivityStarter.ActivityParams
174+
internal object NetpGeoswitchingScreenNoParams : GlobalActivityStarter.ActivityParams {
175+
private fun readResolve(): Any = NetpGeoswitchingScreenNoParams
176+
}

0 commit comments

Comments
 (0)