Migrate out of time view to compose

[Pururun]

This change is 

[linear]

DROID-55 Migrate out-of-time view to compose

[Rawa]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/button/ExternalActionButton.kt line 46 at r2 (raw file):

        isEnabled = isEnabled,
    ) {
        Box(modifier = Modifier.fillMaxSize()) {

If we use box layout here we need to account for the Image afterwards, I believe if there is a slim screen or a long button text it will overlap with the image. Since we know the size of the image I guess we could compensate for it in a way like this, another alternative would possibly be to use a constraintlayout:

    ActionButton(
        onClick = onClick,
        colors = colors,
        modifier = modifier,
        isEnabled = isEnabled,
    ) {
        Text(
            text = text,
            textAlign = TextAlign.Center,
            style = MaterialTheme.typography.bodyMedium,
            maxLines = 1,
            overflow = TextOverflow.Ellipsis,
            modifier = Modifier.padding(start = 24.dp).weight(1f)
        )
        Image(
            painter = painterResource(id = R.drawable.icon_extlink),
            contentDescription = null,
            modifier =
                Modifier.padding(horizontal = Dimens.smallPadding)
                    .alpha(if (isEnabled) AlphaVisible else AlphaDisabled)
        )
    }

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt line 103 at r2 (raw file):

            when (viewAction) {
                is OutOfTimeViewModel.ViewAction.OpenAccountView ->
                    context.openAccountPageInBrowser(viewAction.token)

Is it possible to avoid using context and instead use something more Compose native? Saw this one:
https://stackoverflow.com/a/69103918/1724097

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/fragment/OutOfTimeFragment.kt line 32 at r2 (raw file):

                    val state = vm.uiState.collectAsState().value
                    OutOfTimeScreen(
                        showSitePayment = BuildTypes.RELEASE != BuildConfig.BUILD_TYPE,

We should adapt this to the IS_PLAY flag @albin-mullvad is looking to add.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModel.kt line 59 at r2 (raw file):

            accountRepository.accountExpiryState.collectLatest { accountExpiry ->
                accountExpiry.date()?.let { expiry ->
                    val tomorrow = DateTime.now().plusHours(20)

Any reason why we add 20h to the expiry? Can we avoid it? Shouldn't expiry.isAfter(DateTime.now()) be enough?

[Pururun]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/button/ExternalActionButton.kt line 46 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

If we use box layout here we need to account for the Image afterwards, I believe if there is a slim screen or a long button text it will overlap with the image. Since we know the size of the image I guess we could compensate for it in a way like this, another alternative would possibly be to use a constraintlayout:

    ActionButton(
        onClick = onClick,
        colors = colors,
        modifier = modifier,
        isEnabled = isEnabled,
    ) {
        Text(
            text = text,
            textAlign = TextAlign.Center,
            style = MaterialTheme.typography.bodyMedium,
            maxLines = 1,
            overflow = TextOverflow.Ellipsis,
            modifier = Modifier.padding(start = 24.dp).weight(1f)
        )
        Image(
            painter = painterResource(id = R.drawable.icon_extlink),
            contentDescription = null,
            modifier =
                Modifier.padding(horizontal = Dimens.smallPadding)
                    .alpha(if (isEnabled) AlphaVisible else AlphaDisabled)
        )
    }

Done.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt line 103 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Is it possible to avoid using context and instead use something more Compose native? Saw this one:
https://stackoverflow.com/a/69103918/1724097

I tried to use that, but since we we need to get the string from resources at least for me I needed to add some more code that made it kinda not as elegant.

Sort of like this:

val uriHandler \= LocalUriHandler.current  
val accountPath \= stringResource(id = R.string.account\_url) + "?token=%s"  
LaunchedEffect(key1 = Unit) {  
viewActions.collect { viewAction \->  
when (viewAction) {  
is OutOfTimeViewModel.ViewAction.OpenAccountView \->  
uriHandler.openUri(String.format(viewAction.token, accountPath))  
OutOfTimeViewModel.ViewAction.OpenConnectScreen \-> openConnectScreen()  
}  
}  
}

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/fragment/OutOfTimeFragment.kt line 32 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

We should adapt this to the IS_PLAY flag @albin-mullvad is looking to add.

Yes, hopefully he will merge that soon. :)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModel.kt line 59 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Any reason why we add 20h to the expiry? Can we avoid it? Shouldn't expiry.isAfter(DateTime.now()) be enough?

The reason we have that is because if the user time is not set correctly, I guess if they are travelling or something, this function can fail pretty badly. It was set to 24 hours before but that was causing issues with one-day vouchers so I changed it to 20 instead.

The long term plan is to do something similar to what we have on desktop, where it needs to changed from the default value, but I would probably do that in another PR.

[Pururun]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/fragment/OutOfTimeFragment.kt line 32 at r2 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Yes, hopefully he will merge that soon. :)

Done

[Rawa]

Great work! 👍

Reviewed 5 of 18 files at r1.
Reviewable status: 3 of 18 files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt line 103 at r2 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I tried to use that, but since we we need to get the string from resources at least for me I needed to add some more code that made it kinda not as elegant.

Sort of like this:

val uriHandler \= LocalUriHandler.current  
val accountPath \= stringResource(id = R.string.account\_url) + "?token=%s"  
LaunchedEffect(key1 = Unit) {  
viewActions.collect { viewAction \->  
when (viewAction) {  
is OutOfTimeViewModel.ViewAction.OpenAccountView \->  
uriHandler.openUri(String.format(viewAction.token, accountPath))  
OutOfTimeViewModel.ViewAction.OpenConnectScreen \-> openConnectScreen()  
}  
}  
}

Yeah I see what you mean, maybe we break some of it out as a lambda? I Believe this should work:

@Composable
fun createLaunchUrlHook(uri: String): () -> Unit {
    val uriHandler = LocalUriHandler.current
    return { uriHandler.openUri(uri) }
}

@Composable
private fun MyScreenWithLaunch(uri: String) {
    val launchUrlHook = createLaunchUrlHook(uri = uri)
    LaunchedEffect(Unit) {
        launchUrlHook()
    }
}

It is not the nicest but it would get rid of the context.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/OutOfTimeViewModel.kt line 59 at r2 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

The reason we have that is because if the user time is not set correctly, I guess if they are travelling or something, this function can fail pretty badly. It was set to 24 hours before but that was causing issues with one-day vouchers so I changed it to 20 instead.

The long term plan is to do something similar to what we have on desktop, where it needs to changed from the default value, but I would probably do that in another PR.

I see, yeah sounds like another PR.

[sabercodic]

Reviewed 11 of 18 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

[albin-mullvad]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/fragment/OutOfTimeFragment.kt line 32 at r2 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Done

👍

[Pururun]

Reviewable status: 17 of 19 files reviewed, 2 unresolved discussions (waiting on @Rawa and @sabercodic)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt line 103 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

Yeah I see what you mean, maybe we break some of it out as a lambda? I Believe this should work:

@Composable
fun createLaunchUrlHook(uri: String): () -> Unit {
    val uriHandler = LocalUriHandler.current
    return { uriHandler.openUri(uri) }
}

@Composable
private fun MyScreenWithLaunch(uri: String) {
    val launchUrlHook = createLaunchUrlHook(uri = uri)
    LaunchedEffect(Unit) {
        launchUrlHook()
    }
}

It is not the nicest but it would get rid of the context.

Thanks, I made a solution that at least did ont add a huge amount of code :)

[Rawa]

Reviewable status: 17 of 19 files reviewed, all discussions resolved (waiting on @sabercodic)