Migrate SelectLocationView to use items for each relay item #6488

Rawa · 2024-07-22T13:08:55Z

This change is

linear · 2024-07-22T13:09:00Z

DROID-901 Custom list added animation causes custom list description to slide in from the right

Rawa

Reviewable status: 0 of 25 files reviewed, 1 unresolved discussion

android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt line 90 at r2 (raw file):

            customList =
                CustomList(
                    name = CustomListName.fromString("Empty List list"),

Fixed

Pururun

Reviewed 6 of 14 files at r1, 15 of 19 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @Rawa)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/FilterCell.kt line 36 at r3 (raw file):

@Composable
fun FilterRow(

Name of file should be changed to FilterRow

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/RelayLocationCell.kt line 106 at r3 (raw file):

                    when {
                        isSelected -> MaterialTheme.colorScheme.selected
                        //                        item is RelayItem.CustomList && !relayItem.active

If we have an empty custom list I think we should have a different background

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 267 at r3 (raw file):

        { _, _ ->
        },
    onRemoveLocationFromList: (location: RelayItem.Location, customList: CustomListId) -> Unit =

Nit: customList should be called customListId

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 302 at r3 (raw file):

            val filterChips =
                (state as? SelectLocationUiState.Content)?.filterChips?.toList() ?: emptyList()

Could this be made into a function in the ui state?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 337 at r3 (raw file):

                            items = state.relayListItems,
                            key = { index: Int, item: RelayListItem -> item.key },
                            contentType = { _, _ -> null },

I think we could get some performance improvement here by using content type? Just an idea, not sure how much it matters in actuality.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 432 at r3 (raw file):

        location,
        relayItem.isSelected,
        { onSelectRelay() },

Does this makes it clickable even when it should not be? Like if a relay is down?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 451 at r3 (raw file):

        customListItem,
        itemState.isSelected,
        { onSelectRelay(customListItem) },

Same as above, maybe I missed that code?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 615 at r3 (raw file):

private fun SelectLocationUiState.indexOfSelectedRelayItem(): Int =
    if (this is SelectLocationUiState.Content) {
        relayListItems.indexOfFirst {

Nice ⭐

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 797 at r3 (raw file):

    customListId: CustomListId,
    item: RelayItem.Location,
    onRemoveLocationFromList: (location: RelayItem.Location, customList: CustomListId) -> Unit,

Nit: Could be changed to customListId

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 882 at r3 (raw file):

}

private const val EXTRA_ITEMS_LOCATION =

🎉

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt line 19 at r3 (raw file):

sealed interface FilterChip {
    data class Ownership(val ownership: net.mullvad.mullvadvpn.lib.model.Ownership) : FilterChip

Import alias.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListsRelayItemUseCase.kt line 27 at r3 (raw file):

}

class FilterCustomListsRelayItemUseCase(

File should renamed

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt line 90 at r3 (raw file):

            uiState
                .map { it is Content }
                .filter { it }

What does this do?

Pururun

Reviewed 3 of 6 files at r4, 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @Rawa)

Rawa

Reviewable status: 24 of 27 files reviewed, 11 unresolved discussions (waiting on @Pururun)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/RelayLocationCell.kt line 106 at r3 (raw file):