Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use constraints in UI state for FilterScreen #7389

Merged
merged 3 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.mockk.verify
import net.mullvad.mullvadvpn.compose.createEdgeToEdgeComposeExtension
import net.mullvad.mullvadvpn.compose.setContentWithTheme
import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState
import net.mullvad.mullvadvpn.lib.model.Constraint
import net.mullvad.mullvadvpn.lib.model.Ownership
import net.mullvad.mullvadvpn.lib.model.ProviderId
import org.junit.jupiter.api.Test
Expand All @@ -27,7 +28,7 @@ class FilterScreenTest {
state: RelayFilterUiState = RelayFilterUiState(),
onBackClick: () -> Unit = {},
onApplyClick: () -> Unit = {},
onSelectedOwnership: (ownership: Ownership?) -> Unit = {},
onSelectedOwnership: (ownership: Constraint<Ownership>) -> Unit = {},
onAllProviderCheckChange: (isChecked: Boolean) -> Unit = {},
onSelectedProvider: (checked: Boolean, provider: ProviderId) -> Unit = { _, _ -> },
) {
Expand All @@ -50,8 +51,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = null,
selectedProviders = DUMMY_SELECTED_PROVIDERS,
selectedOwnership = Constraint.Any,
selectedProviders = Constraint.Only(DUMMY_SELECTED_PROVIDERS),
)
)
onNodeWithText("Ownership").assertExists()
Expand All @@ -65,8 +66,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = null,
selectedProviders = DUMMY_SELECTED_PROVIDERS,
selectedOwnership = Constraint.Any,
selectedProviders = Constraint.Only(DUMMY_SELECTED_PROVIDERS),
)
)
onNodeWithText("Ownership").performClick()
Expand All @@ -80,8 +81,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = Ownership.MullvadOwned,
selectedProviders = DUMMY_SELECTED_PROVIDERS,
selectedOwnership = Constraint.Only(Ownership.MullvadOwned),
selectedProviders = Constraint.Only(DUMMY_SELECTED_PROVIDERS),
)
)
onNodeWithText("Ownership").performClick()
Expand All @@ -95,8 +96,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = Ownership.Rented,
selectedProviders = DUMMY_SELECTED_PROVIDERS,
selectedOwnership = Constraint.Only(Ownership.Rented),
selectedProviders = Constraint.Only(DUMMY_SELECTED_PROVIDERS),
)
)
onNodeWithText("Ownership").performClick()
Expand All @@ -110,8 +111,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = null,
selectedProviders = DUMMY_SELECTED_PROVIDERS,
selectedOwnership = Constraint.Any,
selectedProviders = Constraint.Only(DUMMY_SELECTED_PROVIDERS),
)
)

Expand All @@ -128,8 +129,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = null,
selectedProviders = listOf(ProviderId("31173")),
selectedOwnership = Constraint.Any,
selectedProviders = Constraint.Only(setOf(ProviderId("31173"))),
),
onApplyClick = mockClickListener,
)
Expand All @@ -145,8 +146,8 @@ class FilterScreenTest {
state =
RelayFilterUiState(
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
selectedOwnership = null,
selectedProviders = listOf(ProviderId("1RemovedProvider")),
selectedOwnership = Constraint.Any,
selectedProviders = Constraint.Only(setOf(ProviderId("1RemovedProvider"))),
)
)

Expand Down Expand Up @@ -178,6 +179,6 @@ class FilterScreenTest {
ProviderId("xtom") to setOf(Ownership.Rented),
)

private val DUMMY_SELECTED_PROVIDERS = DUMMY_RELAY_ALL_PROVIDERS.keys.toList()
private val DUMMY_SELECTED_PROVIDERS = DUMMY_RELAY_ALL_PROVIDERS.keys.drop(3).toSet()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package net.mullvad.mullvadvpn.compose.preview

import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState
import net.mullvad.mullvadvpn.lib.model.Constraint
import net.mullvad.mullvadvpn.lib.model.Ownership
import net.mullvad.mullvadvpn.lib.model.ProviderId

Expand All @@ -12,8 +13,8 @@ class FilterUiStatePreviewParameterProvider : PreviewParameterProvider<RelayFilt
sequenceOf(
RelayFilterUiState(
providerToOwnerships = PROVIDER_TO_OWNERSHIPS,
selectedOwnership = Ownership.MullvadOwned,
selectedProviders = PROVIDER_TO_OWNERSHIPS.keys.toList(),
selectedOwnership = Constraint.Only(Ownership.MullvadOwned),
selectedProviders = Constraint.Only(PROVIDER_TO_OWNERSHIPS.keys),
)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ import net.mullvad.mullvadvpn.compose.preview.FilterUiStatePreviewParameterProvi
import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState
import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.lib.model.Constraint
import net.mullvad.mullvadvpn.lib.model.Ownership
import net.mullvad.mullvadvpn.lib.model.ProviderId
import net.mullvad.mullvadvpn.lib.model.Providers
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.lib.theme.Dimens
import net.mullvad.mullvadvpn.viewmodel.FilterScreenSideEffect
Expand Down Expand Up @@ -96,7 +98,7 @@ fun FilterScreen(
state: RelayFilterUiState,
onBackClick: () -> Unit,
onApplyClick: () -> Unit,
onSelectedOwnership: (ownership: Ownership?) -> Unit,
onSelectedOwnership: (ownership: Constraint<Ownership>) -> Unit,
onAllProviderCheckChange: (isChecked: Boolean) -> Unit,
onSelectedProvider: (checked: Boolean, provider: ProviderId) -> Unit,
) {
Expand All @@ -121,14 +123,14 @@ fun FilterScreen(
}
if (ownershipExpanded) {
item(key = Keys.OWNERSHIP_ALL, contentType = ContentType.ITEM) {
AnyOwnership(state, onSelectedOwnership)
AnyOwnership(state, { onSelectedOwnership(Constraint.Any) })
}
itemsWithDivider(
key = { it.name },
contentType = { ContentType.ITEM },
items = state.selectableOwnerships,
) { ownership ->
Ownership(ownership, state, onSelectedOwnership)
Ownership(ownership, state, { onSelectedOwnership(Constraint.Only(it)) })
}
}
itemWithDivider(key = Keys.PROVIDERS_TITLE, contentType = ContentType.HEADER) {
Expand Down Expand Up @@ -171,14 +173,11 @@ private fun LazyItemScope.OwnershipHeader(expanded: Boolean, onToggleExpanded: (
}

@Composable
private fun LazyItemScope.AnyOwnership(
state: RelayFilterUiState,
onSelectedOwnership: (ownership: Ownership?) -> Unit,
) {
private fun LazyItemScope.AnyOwnership(state: RelayFilterUiState, onSelectedOwnership: () -> Unit) {
SelectableCell(
title = stringResource(id = R.string.any),
isSelected = state.selectedOwnership == null,
onCellClicked = { onSelectedOwnership(null) },
isSelected = state.selectedOwnership is Constraint.Any,
onCellClicked = { onSelectedOwnership() },
modifier = Modifier.animateItem(),
)
}
Expand All @@ -187,11 +186,11 @@ private fun LazyItemScope.AnyOwnership(
private fun LazyItemScope.Ownership(
ownership: Ownership,
state: RelayFilterUiState,
onSelectedOwnership: (ownership: Ownership?) -> Unit,
onSelectedOwnership: (ownership: Ownership) -> Unit,
) {
SelectableCell(
title = stringResource(id = ownership.stringResource()),
isSelected = ownership == state.selectedOwnership,
isSelected = ownership == state.selectedOwnership.getOrNull(),
onCellClicked = { onSelectedOwnership(ownership) },
modifier = Modifier.animateItem(),
)
Expand Down Expand Up @@ -230,19 +229,26 @@ private fun LazyItemScope.Provider(
) {
CheckboxCell(
title = providerId.value,
checked = providerId in state.selectedProviders,
checked = providerId.isChecked(state.selectedProviders),
onCheckedChange = { checked -> onSelectedProvider(checked, providerId) },
modifier = Modifier.animateItem(),
)
}

private fun ProviderId.isChecked(constraint: Constraint<Providers>) =
when (constraint) {
Constraint.Any -> true
is Constraint.Only -> this in constraint.value
}

@Composable
private fun LazyItemScope.RemovedProvider(
providerId: ProviderId,
state: RelayFilterUiState,
onSelectedProvider: (checked: Boolean, providerId: ProviderId) -> Unit,
) {
val checked = providerId in state.selectedProviders
val checked =
state.selectedProviders is Constraint.Only && providerId in state.selectedProviders.value
CheckboxCell(
title = stringResource(R.string.removed_provider, providerId.value),
checked = checked,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package net.mullvad.mullvadvpn.compose.state

import net.mullvad.mullvadvpn.lib.model.Constraint
import net.mullvad.mullvadvpn.lib.model.Ownership
import net.mullvad.mullvadvpn.lib.model.ProviderId
import net.mullvad.mullvadvpn.lib.model.Providers

fun Ownership?.toOwnershipConstraint(): Constraint<Ownership> =
Expand All @@ -11,15 +10,9 @@ fun Ownership?.toOwnershipConstraint(): Constraint<Ownership> =
else -> Constraint.Only(this)
}

fun Constraint<Providers>.toSelectedProviders(allProviders: List<ProviderId>): List<ProviderId> =
when (this) {
Constraint.Any -> allProviders
is Constraint.Only -> value.providers.toList()
}

fun List<ProviderId>.toConstraintProviders(allProviders: List<ProviderId>): Constraint<Providers> =
fun Providers.toConstraintProviders(allProviders: Providers): Constraint<Providers> =
if (size == allProviders.size) {
Constraint.Any
} else {
Constraint.Only(Providers(toHashSet()))
Constraint.Only(this)
}
Original file line number Diff line number Diff line change
@@ -1,37 +1,45 @@
package net.mullvad.mullvadvpn.compose.state

import net.mullvad.mullvadvpn.lib.model.Constraint
import net.mullvad.mullvadvpn.lib.model.Ownership
import net.mullvad.mullvadvpn.lib.model.ProviderId
import net.mullvad.mullvadvpn.lib.model.Providers

data class RelayFilterUiState(
private val providerToOwnerships: Map<ProviderId, Set<Ownership>> = emptyMap(),
val selectedOwnership: Ownership? = null,
val selectedProviders: List<ProviderId> = emptyList(),
val selectedOwnership: Constraint<Ownership> = Constraint.Any,
val selectedProviders: Constraint<Providers> = Constraint.Any,
) {
val allProviders: List<ProviderId> = providerToOwnerships.keys.toList().sorted()
val allProviders: Providers = providerToOwnerships.keys

val selectableOwnerships: List<Ownership> =
if (selectedProviders.isEmpty()) {
Ownership.entries
} else {
providerToOwnerships
.filterKeys { it in selectedProviders }
.values
.flatten()
.distinct()
}
.sorted()
when (selectedProviders) {
Constraint.Any -> Ownership.entries
is Constraint.Only ->
if (selectedProviders.value.isEmpty()) {
Ownership.entries
} else {
providerToOwnerships
.filterKeys { it in selectedProviders.value }
.values
.flatten()
.distinct()
}
}.sorted()

val selectableProviders: List<ProviderId> =
if (selectedOwnership != null) {
providerToOwnerships.filterValues { selectedOwnership in it }.keys.toList().sorted()
} else {
allProviders
}

val removedProviders: List<ProviderId> = selectedProviders - allProviders
when (selectedOwnership) {
Constraint.Any -> allProviders.toList()
is Constraint.Only ->
providerToOwnerships.filterValues { selectedOwnership.value in it }.keys
}.sorted()

val isApplyButtonEnabled = selectedProviders.isNotEmpty()
val isApplyButtonEnabled = selectedProviders.getOrNull()?.isNotEmpty() != false
val removedProviders: List<ProviderId> =
when (selectedProviders) {
Constraint.Any -> emptyList()
is Constraint.Only -> selectedProviders.value.toList() - allProviders
}

val isAllProvidersChecked = allProviders.size == selectedProviders.size
val isAllProvidersChecked = selectedProviders is Constraint.Any
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ private fun RelayItem.Location.hasProvider(providersConstraint: Constraint<Provi
when (this) {
is RelayItem.Location.Country -> cities.any { it.hasProvider(providersConstraint) }
is RelayItem.Location.City -> relays.any { it.hasProvider(providersConstraint) }
is RelayItem.Location.Relay -> provider in providersConstraint.value.providers
is RelayItem.Location.Relay -> provider in providersConstraint.value
}
} else {
true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class FilterChipUseCase(
when (selectedConstraintProviders) {
is Constraint.Any -> null
is Constraint.Only ->
selectedConstraintProviders.value.providers
selectedConstraintProviders.value
.filter { providerId ->
if (ownershipFilter == null) {
true
Expand Down
Loading
Loading