Fix incorrect content of device name info dialog

[sabercodic]

This change is 

[linear]

DROID-373 Incorrect account view device info dialog text

Apart from the styling being incorrect, which will be handled in DROID-371 the device info dialogs should look identical and have the same texts. The dialog in the account view currently show local network sharing information and also treats the text as a single paragraph.

Account view:

Screenshot_20230927_173408.png

Welcome view:

Screenshot_20230927_173418.png

[Rawa]

Reviewed 1 of 21 files at r2.
Reviewable status: 0 of 28 files reviewed, 4 unresolved discussions (waiting on @sabercodic)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeviceNameInfoDialog.kt line 9 at r2 (raw file):

@Composable
fun DeviceNameInfoDialog(onDismiss: () -> Unit) {
    InfoDialog(

This dialog now exists here:

mullvadvpn-app/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt

Line 256 in 6541297

InfoDialog(

Let's migrate that one over.

---

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

                accountNumber = "1234123412341234",
                accountExpiry = null,
                dialogState = AccountScreenDialogState.NoDialog

Generally I like keeping all the state in the ViewState. But in this case I think we can avoid it. When we migrate to compose navigation, a dialog with just info would be a destination (much like a dialog fragment) thus it would not have to be in the ViewState.

For now I think we can simplify the showing and non showing of the dialog just in compose.
var showDeviceNameInfoDialog by remember { mutableStateOf(false) }
and then just set to false on dismiss:
https://github.com/mullvad/mullvadvpn-app/blob/6541297ffc1b1a4f2330b589cadfe81fffdd0e4a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt#L265C20-L265C20

Later once we have the navigation in compose we can replace enabling of showDeviceNameInfoDialog ( https://github.com/mullvad/mullvadvpn-app/blob/6541297ffc1b1a4f2330b589cadfe81fffdd0e4a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt#L247C16-L247C16 ) with just a call to navigate to the dialog.

---

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

                        contentDescription = null,
                        tint = MaterialTheme.colorScheme.inverseSurface
                    )

We should use a IconButton, if it is a clickable button.

mullvadvpn-app/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt

Line 245 in 6541297

IconButton(

It will help us with touch target size etc.

---

android/lib/resource/src/main/res/values/strings.xml line 224 at r1 (raw file):

    <string name="device_name_info_part1">This is the name assigned to the device. Each device logged in on a Mullvad account gets a unique name that helps you identify it when you manage your devices in the app or on the website.</string>
    <string name="device_name_info_part2">You can have up to 5 devices logged in on one Mullvad account.</string>
    <string name="device_name_info_part3">If you log out, the device and the device name is removed. When you log back in again, the device will get a new name.</string>

These are already in there now with device_name_info_first_paragraph etc. lets remove them.

[sabercodic]

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

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeviceNameInfoDialog.kt line 9 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

This dialog now exists here:

mullvadvpn-app/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt

Line 256 in 6541297

InfoDialog(

Let's migrate that one over.

Done.

---

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

Previously, Rawa (David Göransson) wrote…

Generally I like keeping all the state in the ViewState. But in this case I think we can avoid it. When we migrate to compose navigation, a dialog with just info would be a destination (much like a dialog fragment) thus it would not have to be in the ViewState.

For now I think we can simplify the showing and non showing of the dialog just in compose.
var showDeviceNameInfoDialog by remember { mutableStateOf(false) }
and then just set to false on dismiss:
https://github.com/mullvad/mullvadvpn-app/blob/6541297ffc1b1a4f2330b589cadfe81fffdd0e4a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt#L265C20-L265C20

Later once we have the navigation in compose we can replace enabling of showDeviceNameInfoDialog ( https://github.com/mullvad/mullvadvpn-app/blob/6541297ffc1b1a4f2330b589cadfe81fffdd0e4a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt#L247C16-L247C16 ) with just a call to navigate to the dialog.

Done.

---

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

Previously, Rawa (David Göransson) wrote…

We should use a IconButton, if it is a clickable button.

mullvadvpn-app/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/WelcomeScreen.kt

Line 245 in 6541297

IconButton(

It will help us with touch target size etc.

Done.

---

android/lib/resource/src/main/res/values/strings.xml line 224 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

These are already in there now with device_name_info_first_paragraph etc. lets remove them.

Done.

[Pururun]

Reviewed 3 of 28 files at r1, 20 of 21 files at r2, 1 of 1 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Rawa and @sabercodic)

---

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

            .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), AccountUiState.default())
    val uiState =
        vmState.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), AccountUiState.default())

We could probably remove vmState, it serves no purpose.

[Rawa]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @sabercodic)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/fragment/AccountFragment.kt line 40 at r4 (raw file):

                        onManageAccountClick = vm::onManageAccountClick,
                        onLogoutClick = vm::onLogoutClick,
                        onBackClick = { activity?.onBackPressedDispatcher?.onBackPressed() }

Nice 👍

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 7 at r4 (raw file):

import org.joda.time.DateTime

data class AccountUiState(

Why did we move this class to viewmodel package?

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 9 at r4 (raw file):

data class AccountUiState(
    val deviceName: String?,
    val accountNumber: String?,

I see that deviceName & accountNumber wen't from non-null to nullable, is that avoidable? If we don't have an account number or a device name that it maybe should be fatal for the AccountScreen?

[sabercodic]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Pururun and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 7 at r4 (raw file):

Previously, Rawa (David Göransson) wrote…

Why did we move this class to viewmodel package?

Done.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 9 at r4 (raw file):

Previously, Rawa (David Göransson) wrote…

I see that deviceName & accountNumber wen't from non-null to nullable, is that avoidable? If we don't have an account number or a device name that it maybe should be fatal for the AccountScreen?

there was a feedback from Albin on previous PR and this was the result

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

We could probably remove vmState, it serves no purpose.

should I do it in this PR?

[Pururun]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Rawa and @sabercodic)

---

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

Previously, sabercodic wrote…

should I do it in this PR?

I would say so yes

[albin-mullvad]

Reviewed 1 of 2 files at r5.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Rawa and @sabercodic)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 9 at r4 (raw file):

Previously, sabercodic wrote…

there was a feedback from Albin on previous PR and this was the result

@sabercodic my feedback was that they should be unified since there was a mix of nullable and non-nullable similar properties.

@Rawa it's not a fatal state since we show spinners if the data is not present.

So to summarize, we should use a common unified way of representing the lack of data across these properties (which should result in a spinner), which can either be by having them be nullable or using some other mechanism.

[sabercodic]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Pururun and @Rawa)

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

I would say so yes

Done.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 9 at r4 (raw file):

Previously, albin-mullvad wrote…

@sabercodic my feedback was that they should be unified since there was a mix of nullable and non-nullable similar properties.

@Rawa it's not a fatal state since we show spinners if the data is not present.

So to summarize, we should use a common unified way of representing the lack of data across these properties (which should result in a spinner), which can either be by having them be nullable or using some other mechanism.

Spinner will shows in case of null (before loading the actual data)

[Rawa]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @albin-mullvad, @Pururun, and @sabercodic)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/AccountUiState.kt line 9 at r4 (raw file):

Previously, sabercodic wrote…

Spinner will shows in case of null (before loading the actual data)

👍

[Rawa]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @albin-mullvad, @Pururun, and @sabercodic)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeviceNameInfoDialog.kt line 16 at r5 (raw file):

                appendLine(stringResource(id = R.string.device_name_info_second_paragraph))
                appendLine()
                appendLine(stringResource(id = R.string.device_name_info_third_paragraph))

Last thing to fix! This is my fault, the last one should be append not appendLine

[sabercodic]

Reviewable status: 27 of 29 files reviewed, 2 unresolved discussions (waiting on @Pururun and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeviceNameInfoDialog.kt line 16 at r5 (raw file):

Previously, Rawa (David Göransson) wrote…

Last thing to fix! This is my fault, the last one should be append not appendLine

Done.

[Rawa]

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

[Rawa]

Good job! 👍

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

[Pururun]

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

---

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

Previously, sabercodic wrote…

Done.

🙌