Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update changelog presentation #7394

Merged
merged 4 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions android/BuildInstructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,6 @@ rm ./gradle/verification-metadata.xml
## Gradle properties
Some gradle properties can be set to simplify development. These are listed below.

### Always show changelog
For development purposes, `ALWAYS_SHOW_CHANGELOG` can be set in `local.properties` to always show
the changelog dialog on each app start. For example:
```
ALWAYS_SHOW_CHANGELOG=true
```

### Override version code and version name
To avoid or override the rust based version generation, the `OVERRIDE_VERSION_CODE` and
`OVERRIDE_VERSION_NAME` properties can be set in `local.properties`. For example:
Expand Down
6 changes: 0 additions & 6 deletions android/app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,6 @@ android {
}

applicationVariants.configureEach {
val alwaysShowChangelog =
gradleLocalProperties(rootProject.projectDir, providers)
.getProperty("ALWAYS_SHOW_CHANGELOG") ?: "false"

buildConfigField("boolean", "ALWAYS_SHOW_CHANGELOG", alwaysShowChangelog)

val enableInAppVersionNotifications =
gradleLocalProperties(rootProject.projectDir, providers)
.getProperty("ENABLE_IN_APP_VERSION_NOTIFICATIONS") ?: "true"
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,6 +54,7 @@ class ConnectScreenTest {
unmockkAll()
}

@Suppress("LongParameterList")
private fun ComposeContext.initScreen(
state: ConnectUiState = ConnectUiState.INITIAL,
onDisconnectClick: () -> Unit = {},
Expand All @@ -65,6 +67,8 @@ class ConnectScreenTest {
onSettingsClick: () -> Unit = {},
onAccountClick: () -> Unit = {},
onDismissNewDeviceClick: () -> Unit = {},
onChangelogClick: () -> Unit = {},
onDismissChangelogClick: () -> Unit = {},
) {
setContentWithTheme {
ConnectScreen(
Expand All @@ -79,6 +83,8 @@ class ConnectScreenTest {
onSettingsClick = onSettingsClick,
onAccountClick = onAccountClick,
onDismissNewDeviceClick = onDismissNewDeviceClick,
onChangelogClick = onChangelogClick,
onDismissChangelogClick = onDismissChangelogClick,
)
}
}
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -80,6 +86,8 @@ fun NotificationBanner(
isPlayBuild: Boolean,
openAppListing: () -> Unit,
onClickShowAccount: () -> Unit,
onClickShowChangelog: () -> Unit,
onClickDismissChangelog: () -> Unit,
onClickDismissNewDevice: () -> Unit,
) {
// Fix for animating to invisible state
Expand All @@ -97,6 +105,8 @@ fun NotificationBanner(
isPlayBuild = isPlayBuild,
openAppListing,
onClickShowAccount,
onClickShowChangelog,
onClickDismissChangelog,
onClickDismissNewDevice,
)
)
Expand Down Expand Up @@ -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,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
) {
Expand All @@ -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(
Expand All @@ -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),
),
)
Expand Down Expand Up @@ -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,
)

Expand Down
Loading
Loading