Prevent duplicate navigate up

[Rawa]

Makes use of newly added dropUnlessResumed to prevent user from clicking navigate up while we already are navigating up.

---

This change is 

[linear]

DROID-963 Service killed if user navigates up twice quickly from Select Location

[albin-mullvad]

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

a discussion (no related file):
Is there any way we can ensure we don't miss adding dropUnlessResumed?

---

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/WireguardCustomPortDialog.kt line 19 at r1 (raw file):

import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.lifecycle.compose.dropUnlessResumed

Used in this class? Maybe reviewable acting up

Code quote:

dropUnlessResumed

---

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

    LaunchedEffectCollect(vm.uiSideEffect) {
        when (it) {
            SelectLocationSideEffect.CloseScreen -> navigateBack()

Why was this specific call broken broken out? 🤔

Code quote:

navigateBack()

[Rawa]

Reviewable status: 8 of 44 files reviewed, 3 unresolved discussions (waiting on @albin-mullvad)

a discussion (no related file):

Previously, albin-mullvad wrote…

Is there any way we can ensure we don't miss adding dropUnlessResumed?

Not straight of that I can think of I'm afraid :( It is also a bit of case by case where we need to apply it.

So things to worth noting in this PR:

• dropUnlessResumed needs to be called in a composable context (thus we can't call it from within side-effects. unless we create the lambda outside)
• dropUnlessResumed returns a lambda with return type () -> Unit does we can apply it do places where we pass an argument.

My approach for applying it:

• We apply it everywhere it makes sense (especially crucial is the navigate up I feel)
• For side effect we expect it do collect in a sane manner (e.g only if Resumed) or make us of onlyIfResume argument for navigate
• I've removed launchSingleTopin a lot of places since it feels a bit duplicated, open for discussion to add this back if we feel like it adds something.

---

---

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

Previously, albin-mullvad wrote…

Why was this specific call broken broken out? 🤔

Removed, also see comment for main PR

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/WireguardCustomPortDialog.kt line 19 at r1 (raw file):

Previously, albin-mullvad wrote…

Used in this class? Maybe reviewable acting up

Fixed.

[Pururun]

Reviewed 1 of 15 files at r1, 35 of 35 files at r4, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/CustomListsScreen.kt line 105 at r4 (raw file):

            navigator.navigate(
                EditCustomListDestination(customListId = customList.id),
                onlyIfResumed = true

This uses both launchSingleTop and onlyIfResumed which in this case mostly do the same?

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceRevokedScreen.kt line 68 at r4 (raw file):

        state = state,
        onSettingsClicked =
            dropUnlessResumed {

Same as above, both dropUnlessResumed and launchSingleTop

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt line 39 at r4 (raw file):

import com.ramcosta.composedestinations.result.ResultRecipient
import kotlinx.coroutines.launch
import mullvad_daemon.management_interface.portRange

This should probably not be here?

[Rawa]

Reviewable status: 9 of 44 files reviewed, 6 unresolved discussions (waiting on @albin-mullvad and @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/CustomListsScreen.kt line 105 at r4 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

This uses both launchSingleTop and onlyIfResumed which in this case mostly do the same?

Fixed.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceRevokedScreen.kt line 68 at r4 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Same as above, both dropUnlessResumed and launchSingleTop

Thanks, fixed 👍

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt line 39 at r4 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

This should probably not be here?

Correct, mistake, fixed.

[Pururun]

Reviewed 35 of 35 files at r6, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @albin-mullvad)

[albin-mullvad]

Reviewed 2 of 35 files at r6, 2 of 2 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, Rawa (David Göransson) wrote…

Not straight of that I can think of I'm afraid :( It is also a bit of case by case where we need to apply it.

So things to worth noting in this PR:

• dropUnlessResumed needs to be called in a composable context (thus we can't call it from within side-effects. unless we create the lambda outside)
• dropUnlessResumed returns a lambda with return type () -> Unit does we can apply it do places where we pass an argument.

My approach for applying it:

• We apply it everywhere it makes sense (especially crucial is the navigate up I feel)
• For side effect we expect it do collect in a sane manner (e.g only if Resumed) or make us of onlyIfResume argument for navigate
• I've removed launchSingleTopin a lot of places since it feels a bit duplicated, open for discussion to add this back if we feel like it adds something.

Alright! Let's go with this approach for now 👍

But I really think that we long term should come up with a plan for making sure this is done correctly, either by using konsist or other types of test automation.

---

[Pururun]

Reviewed 5 of 5 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[Pururun]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

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

                onlyIfResumed = true
            ) {
                launchSingleTop = true

Not needed to have both onlyIfResumed and launchSingleTop

[albin-mullvad]

Reviewable status: 43 of 44 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

Not needed to have both onlyIfResumed and launchSingleTop

Nice find, fixed!

[Pururun]

Reviewed 1 of 1 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved