Update changelog presentation

mullvad · Jan 8, 2025 · 8ef9492 · 8ef9492
1 parent a915104
commit 8ef9492
Show file tree

Hide file tree

Showing 21 changed files with 551 additions and 210 deletions.
diff --git a/...vpn/compose/dialog/ChangelogDialogTest.kt → ...vpn/compose/screen/ChangelogScreenTest.kt b/...vpn/compose/dialog/ChangelogDialogTest.kt → ...vpn/compose/screen/ChangelogScreenTest.kt
@@ -1,8 +1,7 @@
-package net.mullvad.mullvadvpn.compose.dialog
+package net.mullvad.mullvadvpn.compose.screen
 
 import androidx.compose.ui.test.ExperimentalTestApi
 import androidx.compose.ui.test.onNodeWithText
-import androidx.compose.ui.test.performClick
 import de.mannodermaus.junit5.compose.ComposeContext
 import io.mockk.MockKAnnotations
 import io.mockk.impl.annotations.MockK
@@ -15,7 +14,7 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.RegisterExtension
 
 @OptIn(ExperimentalTestApi::class)
-class ChangelogDialogTest {
+class ChangelogScreenTest {
     @JvmField @RegisterExtension val composeExtension = createEdgeToEdgeComposeExtension()
 
     @MockK lateinit var mockedViewModel: AppInfoViewModel
@@ -25,29 +24,38 @@ class ChangelogDialogTest {
         MockKAnnotations.init(this)
     }
 
-    private fun ComposeContext.initDialog(state: ChangelogUiState, onDismiss: () -> Unit = {}) {
-        setContentWithTheme { ChangelogDialog(state = state, onDismiss = onDismiss) }
+    private fun ComposeContext.initScreen(
+        state: ChangelogUiState,
+        onSeeFullChangelog: () -> Unit = {},
+        onBackClick: () -> Unit = {},
+    ) {
+        setContentWithTheme {
+            ChangelogScreen(
+                state = state,
+                onSeeFullChangelog = onSeeFullChangelog,
+                onBackClick = onBackClick,
+            )
+        }
     }
 
     @Test
     fun testShowChangeLogWhenNeeded() =
         composeExtension.use {
             // Arrange
-            initDialog(
+            initScreen(
                 state =
                     ChangelogUiState(changes = listOf(CHANGELOG_ITEM), version = CHANGELOG_VERSION),
-                onDismiss = {},
+                onBackClick = {},
             )
 
+            // Check changelog version shown
+            onNodeWithText(CHANGELOG_VERSION).assertExists()
+
             // Check changelog content showed within dialog
             onNodeWithText(CHANGELOG_ITEM).assertExists()
-
-            // perform click on Got It button to check if dismiss occur
-            onNodeWithText(CHANGELOG_BUTTON_TEXT).performClick()
         }
 
     companion object {
-        private const val CHANGELOG_BUTTON_TEXT = "Got it!"
         private const val CHANGELOG_ITEM = "Changelog item"
         private const val CHANGELOG_VERSION = "1234.5"
     }

diff --git a/...oid/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt b/...oid/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt
@@ -17,6 +17,7 @@ import net.mullvad.mullvadvpn.compose.test.CIRCULAR_PROGRESS_INDICATOR
 import net.mullvad.mullvadvpn.compose.test.CONNECT_BUTTON_TEST_TAG
 import net.mullvad.mullvadvpn.compose.test.CONNECT_CARD_HEADER_TEST_TAG
 import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER_ACTION
+import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER_TEXT_ACTION
 import net.mullvad.mullvadvpn.compose.test.RECONNECT_BUTTON_TEST_TAG
 import net.mullvad.mullvadvpn.compose.test.SELECT_LOCATION_BUTTON_TEST_TAG
 import net.mullvad.mullvadvpn.compose.test.TOP_BAR_ACCOUNT_BUTTON
@@ -53,6 +54,7 @@ class ConnectScreenTest {
         unmockkAll()
     }
 
+    @Suppress("LongParameterList")
     private fun ComposeContext.initScreen(
         state: ConnectUiState = ConnectUiState.INITIAL,
         onDisconnectClick: () -> Unit = {},
@@ -65,6 +67,8 @@ class ConnectScreenTest {
         onSettingsClick: () -> Unit = {},
         onAccountClick: () -> Unit = {},
         onDismissNewDeviceClick: () -> Unit = {},
+        onChangelogClick: () -> Unit = {},
+        onDismissChangelogClick: () -> Unit = {},
     ) {
         setContentWithTheme {
             ConnectScreen(
@@ -79,6 +83,8 @@ class ConnectScreenTest {
                 onSettingsClick = onSettingsClick,
                 onAccountClick = onAccountClick,
                 onDismissNewDeviceClick = onDismissNewDeviceClick,
+                onChangelogClick = onChangelogClick,
+                onDismissChangelogClick = onDismissChangelogClick,
             )
         }
     }
@@ -631,6 +637,34 @@ class ConnectScreenTest {
         }
     }
 
+    @Test
+    fun testOnNewChangelogMessageClick() {
+        composeExtension.use {
+            // Arrange
+            val mockedClickHandler: () -> Unit = mockk(relaxed = true)
+            initScreen(
+                onChangelogClick = mockedClickHandler,
+                state =
+                    ConnectUiState(
+                        location = null,
+                        selectedRelayItemTitle = null,
+                        tunnelState = TunnelState.Connecting(null, null, emptyList()),
+                        showLocation = false,
+                        deviceName = "",
+                        daysLeftUntilExpiry = null,
+                        inAppNotification = InAppNotification.NewVersionChangelog,
+                        isPlayBuild = false,
+                    ),
+            )
+
+            // Act
+            onNodeWithTag(NOTIFICATION_BANNER_TEXT_ACTION).performClick()
+
+            // Assert
+            verify { mockedClickHandler.invoke() }
+        }
+    }
+
     @Test
     fun testOpenAccountView() {
         composeExtension.use {

diff --git a/.../kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationBanner.kt b/.../kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationBanner.kt
@@ -5,21 +5,25 @@ import androidx.compose.animation.animateContentSize
 import androidx.compose.animation.slideInVertically
 import androidx.compose.animation.slideOutVertically
 import androidx.compose.foundation.background
+import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.layout.wrapContentWidth
 import androidx.compose.foundation.shape.CircleShape
 import androidx.compose.material3.Icon
 import androidx.compose.material3.IconButton
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.vector.ImageVector
 import androidx.compose.ui.platform.testTag
+import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.text.toUpperCase
 import androidx.compose.ui.tooling.preview.Preview
@@ -29,6 +33,7 @@ import androidx.constraintlayout.compose.Dimension
 import net.mullvad.mullvadvpn.compose.component.MullvadTopBar
 import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER
 import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER_ACTION
+import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER_TEXT_ACTION
 import net.mullvad.mullvadvpn.compose.util.rememberPrevious
 import net.mullvad.mullvadvpn.lib.model.ErrorState
 import net.mullvad.mullvadvpn.lib.model.ErrorStateCause
@@ -56,8 +61,9 @@ private fun PreviewNotificationBanner() {
                         InAppNotification.TunnelStateError(
                             error = ErrorState(ErrorStateCause.FirewallPolicyError.Generic, true)
                         ),
+                        InAppNotification.NewVersionChangelog,
                     )
-                    .map { it.toNotificationData(false, {}, {}, {}) }
+                    .map { it.toNotificationData(false, {}, {}, {}, {}, {}) }
 
             bannerDataList.forEach {
                 MullvadTopBar(
@@ -80,6 +86,8 @@ fun NotificationBanner(
     isPlayBuild: Boolean,
     openAppListing: () -> Unit,
     onClickShowAccount: () -> Unit,
+    onClickShowChangelog: () -> Unit,
+    onClickDismissChangelog: () -> Unit,
     onClickDismissNewDevice: () -> Unit,
 ) {
     // Fix for animating to invisible state
@@ -97,6 +105,8 @@ fun NotificationBanner(
                     isPlayBuild = isPlayBuild,
                     openAppListing,
                     onClickShowAccount,
+                    onClickShowChangelog,
+                    onClickDismissChangelog,
                     onClickDismissNewDevice,
                 )
             )
@@ -153,21 +163,38 @@ private fun Notification(notificationBannerData: NotificationData) {
             maxLines = 1,
             overflow = TextOverflow.Ellipsis,
         )
-        message?.let {
+        message?.let { message ->
             Text(
-                text = message,
+                text = message.text,
                 modifier =
                     Modifier.constrainAs(textMessage) {
                             top.linkTo(textTitle.bottom)
                             start.linkTo(textTitle.start)
                             if (action != null) {
                                 end.linkTo(actionIcon.start)
+                                bottom.linkTo(actionIcon.bottom)
                             } else {
                                 end.linkTo(parent.end)
+                                bottom.linkTo(parent.bottom)
                             }
                             width = Dimension.fillToConstraints
+                            height = Dimension.fillToConstraints
                         }
-                        .padding(start = Dimens.smallPadding, top = Dimens.tinyPadding),
+                        .padding(start = Dimens.smallPadding, top = Dimens.tinyPadding)
+                        .wrapContentWidth(Alignment.Start)
+                        .let {
+                            if (message is NotificationMessage.ClickableText) {
+                                it.clickable(
+                                        onClickLabel = message.contentDescription,
+                                        role = Role.Button,
+                                    ) {
+                                        message.onClick()
+                                    }
+                                    .testTag(NOTIFICATION_BANNER_TEXT_ACTION)
+                            } else {
+                                it
+                            }
+                        },
                 color = MaterialTheme.colorScheme.onSurfaceVariant,
                 style = MaterialTheme.typography.labelMedium,
             )

diff --git a/...in/kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationData.kt b/...in/kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationData.kt
@@ -10,7 +10,10 @@ import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.AnnotatedString
 import androidx.compose.ui.text.SpanStyle
+import androidx.compose.ui.text.buildAnnotatedString
 import androidx.compose.ui.text.font.FontWeight
+import androidx.compose.ui.text.style.TextDecoration
+import androidx.compose.ui.text.withStyle
 import androidx.core.text.HtmlCompat
 import java.net.InetAddress
 import net.mullvad.mullvadvpn.R
@@ -25,7 +28,7 @@ import net.mullvad.mullvadvpn.ui.notification.StatusLevel
 
 data class NotificationData(
     val title: AnnotatedString,
-    val message: AnnotatedString? = null,
+    val message: NotificationMessage? = null,
     val statusLevel: StatusLevel,
     val action: NotificationAction? = null,
 ) {
@@ -34,7 +37,31 @@ data class NotificationData(
         message: String? = null,
         statusLevel: StatusLevel,
         action: NotificationAction? = null,
-    ) : this(AnnotatedString(title), message?.let { AnnotatedString(it) }, statusLevel, action)
+    ) : this(
+        AnnotatedString(title),
+        message?.let { NotificationMessage.Text(AnnotatedString(it)) },
+        statusLevel,
+        action,
+    )
+
+    constructor(
+        title: String,
+        message: NotificationMessage,
+        statusLevel: StatusLevel,
+        action: NotificationAction? = null,
+    ) : this(AnnotatedString(title), message, statusLevel, action)
+}
+
+sealed interface NotificationMessage {
+    val text: AnnotatedString
+
+    data class Text(override val text: AnnotatedString) : NotificationMessage
+
+    data class ClickableText(
+        override val text: AnnotatedString,
+        val onClick: () -> Unit,
+        val contentDescription: String,
+    ) : NotificationMessage
 }
 
 data class NotificationAction(
@@ -48,21 +75,25 @@ fun InAppNotification.toNotificationData(
     isPlayBuild: Boolean,
     openAppListing: () -> Unit,
     onClickShowAccount: () -> Unit,
-    onDismissNewDevice: () -> Unit,
+    onClickShowChangelog: () -> Unit,
+    onClickDismissChangelog: () -> Unit,
+    onClickDismissNewDevice: () -> Unit,
 ) =
     when (this) {
         is InAppNotification.NewDevice ->
             NotificationData(
                 title =
                     AnnotatedString(stringResource(id = R.string.new_device_notification_title)),
                 message =
-                    stringResource(id = R.string.new_device_notification_message, deviceName)
-                        .formatWithHtml(),
+                    NotificationMessage.Text(
+                        stringResource(id = R.string.new_device_notification_message, deviceName)
+                            .formatWithHtml()
+                    ),
                 statusLevel = StatusLevel.Info,
                 action =
                     NotificationAction(
                         Icons.Default.Clear,
-                        onDismissNewDevice,
+                        onClickDismissNewDevice,
                         stringResource(id = R.string.dismiss),
                     ),
             )
@@ -98,13 +129,40 @@ fun InAppNotification.toNotificationData(
                         stringResource(id = R.string.open_url),
                     ),
             )
+        is InAppNotification.NewVersionChangelog ->
+            NotificationData(
+                title = stringResource(id = R.string.new_changelog_notification_title),
+                message =
+                    NotificationMessage.ClickableText(
+                        text =
+                            buildAnnotatedString {
+                                withStyle(SpanStyle(textDecoration = TextDecoration.Underline)) {
+                                    append(
+                                        stringResource(
+                                            id = R.string.new_changelog_notification_message
+                                        )
+                                    )
+                                }
+                            },
+                        onClick = onClickShowChangelog,
+                        contentDescription =
+                            stringResource(id = R.string.new_changelog_notification_message),
+                    ),
+                statusLevel = StatusLevel.Info,
+                action =
+                    NotificationAction(
+                        Icons.Default.Clear,
+                        onClickDismissChangelog,
+                        stringResource(id = R.string.dismiss),
+                    ),
+            )
     }
 
 @Composable
 private fun errorMessageBannerData(error: ErrorState) =
     NotificationData(
         title = error.title().formatWithHtml(),
-        message = error.message().formatWithHtml(),
+        message = NotificationMessage.Text(error.message().formatWithHtml()),
         statusLevel = StatusLevel.Error,
     )