Add auto connect and lockdown mode screen #5592

MaryamShaghaghi · 2023-12-13T13:32:32Z

This change is

Pururun

Reviewed 25 of 25 files at r1, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @MaryamShaghaghi)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 48 at r1 (raw file):

@Composable
fun AutoConnectCarousel() {
    val pagerState = rememberPagerState(pageCount = { 3 })

Should define this as a constant, such as const val CAROUSEL_PAGE_SIZE = 3

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 62 at r1 (raw file):

                pageIndicatorRef) =
                createRefs()
            HtmlText(

Use annotated string instead of HtmlText

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 78 at r1 (raw file):

                            }
                    ),
                textSize = 12.sp.value,

Try to use a style instead.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 98 at r1 (raw file):

                        contentDescription = null,
                        tint = Color.Unspecified,
                        modifier = Modifier.rotate(180f).alpha(AlphaDescription)

Magic number

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 121 at r1 (raw file):

            )

            if (page != 2) {

This is better to write as page < CAROUSEL_PAGE_SIZE

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 142 at r1 (raw file):

            }

            HtmlText(

Same as above user AnnotatedString

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 158 at r1 (raw file):

                            }
                    ),
                textSize = 12.sp.value,

Use style instead (which you can do if you use annotated string)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 175 at r1 (raw file):

                repeat(pagerState.pageCount) { iteration ->
                    val color =
                        if (pagerState.currentPage == iteration) Color.LightGray else Color.DarkGray

Use theme colors instead.

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

            state = lazyListState
        ) {
            item {

Can we wrap this in requireActivity(Intent("android.settings.VPN_SETTINGS")) ?

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

    <string name="auto_connect_and_lockdown_mode_footer">Makes sure the device is always on the VPN tunnel.</string>
    <string name="go_to_vpn_settings">Go to VPN settings</string>
    <string name="carousel_slide_1_text_1">The Auto-connect and Lockdown mode settings can be found in the Android system settings, follow this guide to enable one or both.</string>

Is it possible to make this a bit more expressive? Like auto_connect_carousel_first_slide_top_text or something? The same is applicable to all of these.

android/lib/theme/src/main/kotlin/net/mullvad/mullvadvpn/lib/theme/dimensions/Dimensions.kt line 11 at r1 (raw file):

    val backButtonSideMargin: Dp = 30.dp,
    val bigIconSize: Dp = 44.dp,
    val bottomPadding: Dp = 3.dp,

Preferable use something divided by 2 or even better 4.

MaryamShaghaghi

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

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/AutoConnectCarousel.kt line 48 at r1 (raw file):