Skip to content

Commit c1ea62c

Browse files
PururunPururun
committed
Fix review issues
1 parent c8820ca commit c1ea62c

File tree

13 files changed

+47
-56
lines changed

13 files changed

+47
-56
lines changed

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/FilterRow.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ fun FilterRow(
4646
Modifier.padding(horizontal = Dimens.searchFieldHorizontalPadding)
4747
.fillMaxWidth()
4848
.horizontalScroll(scrollState),
49-
horizontalArrangement = Arrangement.spacedBy(Dimens.chipSpace, alignment = Alignment.Start),
49+
horizontalArrangement = Arrangement.spacedBy(Dimens.chipSpace),
5050
) {
5151
if (showTitle) {
5252
Text(

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/SettingsUiStatePreviewParameterProvider.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ class SettingsUiStatePreviewParameterProvider : PreviewParameterProvider<Setting
1111
isLoggedIn = true,
1212
isSupportedVersion = true,
1313
isPlayBuild = true,
14-
useMultihop = false,
14+
multihopEnabled = false,
1515
),
1616
SettingsUiState(
1717
appVersion = "9000.1",
1818
isLoggedIn = false,
1919
isSupportedVersion = false,
2020
isPlayBuild = false,
21-
useMultihop = false,
21+
multihopEnabled = false,
2222
),
2323
)
2424
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SettingsScreen.kt

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ fun SettingsScreen(
102102
if (state.isLoggedIn) {
103103
itemWithDivider {
104104
Multihop(
105-
isMultihopEnabled = state.useMultihop,
105+
isMultihopEnabled = state.multihopEnabled,
106106
onMultihopClick = onMultihopClick,
107107
)
108108
}
@@ -191,7 +191,7 @@ private fun FaqAndGuides() {
191191
NavigationComposeCell(
192192
title = faqGuideLabel,
193193
bodyView =
194-
@Composable {
194+
{
195195
DefaultExternalLinkView(
196196
chevronContentDescription = faqGuideLabel,
197197
tint = MaterialTheme.colorScheme.onPrimary,
@@ -213,7 +213,7 @@ private fun PrivacyPolicy(state: SettingsUiState) {
213213
NavigationComposeCell(
214214
title = privacyPolicyLabel,
215215
bodyView =
216-
@Composable {
216+
{
217217
DefaultExternalLinkView(
218218
chevronContentDescription = privacyPolicyLabel,
219219
tint = MaterialTheme.colorScheme.onPrimary,
@@ -229,21 +229,20 @@ private fun Multihop(isMultihopEnabled: Boolean, onMultihopClick: () -> Unit) {
229229
NavigationComposeCell(
230230
title = title,
231231
onClick = onMultihopClick,
232-
bodyView =
233-
@Composable {
234-
NavigationCellBody(
235-
content =
236-
stringResource(
237-
if (isMultihopEnabled) {
238-
R.string.on
239-
} else {
240-
R.string.off
241-
}
242-
),
243-
contentBodyDescription = title,
244-
textColor = MaterialTheme.colorScheme.onSurfaceVariant,
245-
isExternalLink = false,
246-
)
247-
},
232+
bodyView = {
233+
NavigationCellBody(
234+
content =
235+
stringResource(
236+
if (isMultihopEnabled) {
237+
R.string.on
238+
} else {
239+
R.string.off
240+
}
241+
),
242+
contentBodyDescription = title,
243+
textColor = MaterialTheme.colorScheme.onSurfaceVariant,
244+
isExternalLink = false,
245+
)
246+
},
248247
)
249248
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/location/RelayListContent.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ fun LazyListScope.relayListContent(
7474
listItem,
7575
relayListSelection,
7676
{ onSelectRelay(listItem.item) },
77+
// Only direct children can be removed
7778
if (listItem.depth == 1) {
7879
{
7980
onUpdateBottomSheetState(
@@ -99,7 +100,6 @@ fun LazyListScope.relayListContent(
99100
relayListSelection = relayListSelection,
100101
{ onSelectRelay(listItem.item) },
101102
{
102-
// Only direct children can be removed
103103
onUpdateBottomSheetState(
104104
ShowLocationBottomSheet(customLists, listItem.item)
105105
)

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/location/SearchLocationScreen.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import androidx.compose.material3.IconButton
1919
import androidx.compose.material3.MaterialTheme
2020
import androidx.compose.material3.Scaffold
2121
import androidx.compose.material3.SearchBarDefaults
22-
import androidx.compose.material3.SnackbarDuration
2322
import androidx.compose.material3.SnackbarHost
2423
import androidx.compose.material3.SnackbarHostState
2524
import androidx.compose.material3.Text
@@ -119,8 +118,7 @@ fun SearchLocation(
119118
SearchLocationSideEffect.GenericError ->
120119
launch {
121120
snackbarHostState.showSnackbarImmediately(
122-
message = context.getString(R.string.error_occurred),
123-
duration = SnackbarDuration.Short,
121+
message = context.getString(R.string.error_occurred)
124122
)
125123
}
126124
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/location/SelectLocationList.kt

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ fun SelectLocationList(
4444
val lazyListState = rememberLazyListState()
4545
val stateActual = state
4646
RunOnKeyChange(stateActual is SelectLocationListUiState.Content) {
47-
val index = stateActual.indexOfSelectedRelayItem()
48-
if (index != -1) {
47+
stateActual.indexOfSelectedRelayItem()?.let { index ->
4948
lazyListState.scrollToItem(index)
5049
lazyListState.animateScrollAndCentralizeItem(index)
5150
}
@@ -85,21 +84,21 @@ private fun LazyListScope.loading() {
8584
}
8685
}
8786

88-
private fun SelectLocationListUiState.indexOfSelectedRelayItem(): Int =
87+
private fun SelectLocationListUiState.indexOfSelectedRelayItem(): Int? =
8988
if (this is SelectLocationListUiState.Content) {
9089
relayListItems.indexOfFirst {
9190
when (it) {
9291
is RelayListItem.CustomListItem -> it.isSelected
9392
is RelayListItem.GeoLocationItem -> it.isSelected
94-
is RelayListItem.CustomListEntryItem -> false
95-
is RelayListItem.CustomListFooter -> false
96-
RelayListItem.CustomListHeader -> false
97-
RelayListItem.LocationHeader -> false
93+
is RelayListItem.CustomListEntryItem,
94+
is RelayListItem.CustomListFooter,
95+
RelayListItem.CustomListHeader,
96+
RelayListItem.LocationHeader,
9897
is RelayListItem.LocationsEmptyText -> false
9998
}
10099
}
101100
} else {
102-
-1
101+
null
103102
}
104103

105104
private suspend fun LazyListState.animateScrollAndCentralizeItem(index: Int) {

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/location/SelectLocationScreen.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import androidx.compose.material3.Icon
1818
import androidx.compose.material3.IconButton
1919
import androidx.compose.material3.MaterialTheme
2020
import androidx.compose.material3.SingleChoiceSegmentedButtonRow
21-
import androidx.compose.material3.SnackbarDuration
2221
import androidx.compose.material3.SnackbarHostState
2322
import androidx.compose.runtime.Composable
2423
import androidx.compose.runtime.LaunchedEffect
@@ -118,8 +117,7 @@ fun SelectLocation(
118117
SelectLocationSideEffect.GenericError ->
119118
launch {
120119
snackbarHostState.showSnackbarImmediately(
121-
message = context.getString(R.string.error_occurred),
122-
duration = SnackbarDuration.Short,
120+
message = context.getString(R.string.error_occurred)
123121
)
124122
}
125123
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/RelayListItem.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ sealed interface RelayListItem {
6262
}
6363

6464
data object LocationHeader : RelayListItem {
65-
override val key: Any = "location_header"
65+
override val key = "location_header"
6666
override val contentType = RelayListItemContentType.LOCATION_HEADER
6767
}
6868

@@ -78,7 +78,7 @@ sealed interface RelayListItem {
7878
}
7979

8080
data class LocationsEmptyText(val searchTerm: String) : RelayListItem {
81-
override val key: Any = "locations_empty_text"
81+
override val key = "locations_empty_text"
8282
override val contentType = RelayListItemContentType.LOCATIONS_EMPTY_TEXT
8383
}
8484
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SettingsUiState.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ data class SettingsUiState(
55
val isLoggedIn: Boolean,
66
val isSupportedVersion: Boolean,
77
val isPlayBuild: Boolean,
8-
val useMultihop: Boolean,
8+
val multihopEnabled: Boolean,
99
)

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SettingsViewModel.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class SettingsViewModel(
3030
appVersion = versionInfo.currentVersion,
3131
isSupportedVersion = versionInfo.isSupported,
3232
isPlayBuild = isPlayBuild,
33-
useMultihop = wireguardConstraints?.useMultihop ?: false,
33+
multihopEnabled = wireguardConstraints?.useMultihop ?: false,
3434
)
3535
}
3636
.stateIn(
@@ -41,7 +41,7 @@ class SettingsViewModel(
4141
isLoggedIn = false,
4242
isSupportedVersion = true,
4343
isPlayBuild = isPlayBuild,
44-
useMultihop = false,
44+
multihopEnabled = false,
4545
),
4646
)
4747
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/location/RelayItemListCreator.kt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,32 +7,32 @@ import net.mullvad.mullvadvpn.lib.model.GeoLocationId
77
import net.mullvad.mullvadvpn.lib.model.RelayItem
88
import net.mullvad.mullvadvpn.lib.model.RelayItemId
99
import net.mullvad.mullvadvpn.lib.model.SelectedLocation
10+
import net.mullvad.mullvadvpn.relaylist.MIN_SEARCH_LENGTH
1011
import net.mullvad.mullvadvpn.relaylist.filterOnSearchTerm
1112

1213
// Creates a relay list to be displayed by RelayListContent
13-
// Search input as null is defined as not searching
1414
internal fun relayListItems(
15-
searchTerm: String? = null,
15+
searchTerm: String = "",
1616
relayCountries: List<RelayItem.Location.Country>,
1717
customLists: List<RelayItem.CustomList>,
1818
selectedItem: RelayItemId?,
1919
disabledItem: RelayItemId?,
2020
expandedItems: Set<String>,
2121
): List<RelayListItem> {
22-
val filteredCustomLists = customLists.filterOnSearchTerm(searchTerm ?: "")
22+
val filteredCustomLists = customLists.filterOnSearchTerm(searchTerm)
2323

2424
return buildList {
2525
val relayItems =
2626
createRelayListItems(
27-
isSearching = searchTerm != null,
27+
isSearching = searchTerm.isSearching(),
2828
selectedItem = selectedItem,
2929
disabledItem = disabledItem,
3030
customLists = filteredCustomLists,
3131
countries = relayCountries,
3232
) {
3333
it in expandedItems
3434
}
35-
if (relayItems.isEmpty() && searchTerm != null) {
35+
if (relayItems.isEmpty()) {
3636
add(RelayListItem.LocationsEmptyText(searchTerm))
3737
} else {
3838
addAll(relayItems)
@@ -266,3 +266,5 @@ private fun RelayItemId?.singleRelayId(customLists: List<RelayItem.CustomList>):
266266
?.id as? GeoLocationId.Hostname
267267
else -> null
268268
}
269+
270+
private fun String.isSearching() = length >= MIN_SEARCH_LENGTH

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/location/SearchLocationViewModel.kt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,12 @@ class SearchLocationViewModel(
140140
.map { it.second }
141141

142142
private fun filterChips() =
143-
filterChipUseCase().map { filterChips: List<FilterChip> ->
143+
combine(filterChipUseCase(), wireguardConstraintsRepository.wireguardConstraints) {
144+
filterChips,
145+
constraints ->
144146
filterChips.toMutableList().apply {
145147
// Do not show entry and exit filter chips if multihop is disabled
146-
if (multihopEnabled()) {
148+
if (constraints?.useMultihop == true) {
147149
add(
148150
when (relayListSelection) {
149151
RelayListSelection.Entry -> FilterChip.Entry
@@ -154,9 +156,6 @@ class SearchLocationViewModel(
154156
}
155157
}
156158

157-
private fun multihopEnabled() =
158-
wireguardConstraintsRepository.wireguardConstraints.value?.useMultihop == true
159-
160159
fun addLocationToList(item: RelayItem.Location, customList: RelayItem.CustomList) {
161160
viewModelScope.launch {
162161
val result =

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/location/SelectLocationListViewModel.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package net.mullvad.mullvadvpn.viewmodel.location
22

33
import androidx.lifecycle.ViewModel
44
import androidx.lifecycle.viewModelScope
5-
import co.touchlab.kermit.Logger
65
import kotlinx.coroutines.flow.MutableStateFlow
76
import kotlinx.coroutines.flow.SharingStarted
87
import kotlinx.coroutines.flow.StateFlow
@@ -65,18 +64,15 @@ class SelectLocationListViewModel(
6564
private fun initialExpand(item: RelayItemId?): Set<String> = buildSet {
6665
when (item) {
6766
is GeoLocationId.City -> {
68-
Logger.d("GC item: $item")
6967
add(item.country.code)
7068
}
7169
is GeoLocationId.Hostname -> {
72-
Logger.d("GH item: $item")
7370
add(item.country.code)
7471
add(item.city.code)
7572
}
7673
is CustomListId,
7774
is GeoLocationId.Country,
7875
null -> {
79-
Logger.d("NO item: $item")
8076
/* No expands */
8177
}
8278
}

0 commit comments

Comments
 (0)