Skip to content

Commit

Permalink
Fix remarks
Browse files Browse the repository at this point in the history
  • Loading branch information
Rawa committed Sep 27, 2023
1 parent 5cccca3 commit 1ac53ac
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 76 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ Line wrap the file at 100 chars. Th
- Move out of time evaluation to connect view model.
- Migrate out of time view to compose.
- Migrate login view to compose.
- Migrate Report Problem view to compose.
- Migrate View Logs view to compose.

#### Linux
- Don't block forwarding of traffic when the split tunnel mark (ct mark) is set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,31 @@ package net.mullvad.mullvadvpn.compose.dialog
import androidx.compose.foundation.Image
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.layout.size
import androidx.compose.material3.AlertDialog
import androidx.compose.material3.ButtonDefaults
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.Color
import androidx.compose.ui.res.colorResource
import androidx.compose.ui.res.dimensionResource
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.ActionButton
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.lib.theme.Dimens

@Preview
@Composable
private fun PreviewReportProblemNoEmailDialog() {
ReportProblemNoEmailDialog(
onDismiss = {},
onConfirm = {},
)
AppTheme {
ReportProblemNoEmailDialog(
onDismiss = {},
onConfirm = {},
)
}
}

@Composable
Expand All @@ -42,26 +41,25 @@ fun ReportProblemNoEmailDialog(onDismiss: () -> Unit, onConfirm: () -> Unit) {
) {
Image(
painter = painterResource(id = R.drawable.icon_alert),
contentDescription = "Remove",
modifier = Modifier.width(50.dp).height(50.dp)
contentDescription = null,
modifier = Modifier.size(Dimens.dialogIconSize)
)
}
},
text = {
Text(
text = stringResource(id = R.string.confirm_no_email),
color = colorResource(id = R.color.white),
fontSize = dimensionResource(id = R.dimen.text_small).value.sp,
modifier = Modifier.fillMaxWidth()
modifier = Modifier.fillMaxWidth(),
style = MaterialTheme.typography.bodySmall
)
},
dismissButton = {
ActionButton(
modifier = Modifier.fillMaxWidth(),
colors =
ButtonDefaults.buttonColors(
containerColor = colorResource(id = R.color.red),
contentColor = Color.White
containerColor = MaterialTheme.colorScheme.error,
contentColor = MaterialTheme.colorScheme.onError,
),
onClick = onConfirm,
text = stringResource(id = R.string.send_anyway)
Expand All @@ -72,13 +70,13 @@ fun ReportProblemNoEmailDialog(onDismiss: () -> Unit, onConfirm: () -> Unit) {
modifier = Modifier.fillMaxWidth(),
colors =
ButtonDefaults.buttonColors(
containerColor = colorResource(id = R.color.blue),
contentColor = Color.White
containerColor = MaterialTheme.colorScheme.primary,
contentColor = MaterialTheme.colorScheme.onPrimary,
),
onClick = { onDismiss() },
text = stringResource(id = R.string.back)
)
},
containerColor = colorResource(id = R.color.darkBlue)
containerColor = MaterialTheme.colorScheme.background
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
import androidx.compose.material3.AlertDialog
import androidx.compose.material3.ButtonDefaults
import androidx.compose.material3.CircularProgressIndicator
Expand All @@ -18,22 +17,17 @@ import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.res.colorResource
import androidx.compose.ui.res.dimensionResource
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.SpanStyle
import androidx.compose.ui.text.buildAnnotatedString
import androidx.compose.ui.text.font.FontStyle
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.text.withStyle
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import androidx.compose.ui.window.DialogProperties
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.ActionButton
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.lib.theme.Dimens
import net.mullvad.mullvadvpn.viewmodel.SendingReportUiState

Expand Down Expand Up @@ -62,7 +56,7 @@ fun ShowReportProblemStateDialog(
@Preview
@Composable
private fun PreviewReportProblemSendingDialog() {
ReportProblemSendingDialog()
AppTheme { ReportProblemSendingDialog() }
}

@Composable
Expand All @@ -83,10 +77,7 @@ private fun ReportProblemSendingDialog() {
) {
Text(
text = stringResource(id = R.string.sending),
color = colorResource(id = R.color.white),
fontSize = dimensionResource(id = R.dimen.text_small).value.sp,
fontStyle = FontStyle.Normal,
textAlign = TextAlign.Start,
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.fillMaxWidth()
)
}
Expand All @@ -97,17 +88,19 @@ private fun ReportProblemSendingDialog() {
dismissOnClickOutside = false,
dismissOnBackPress = false,
),
containerColor = colorResource(id = R.color.darkBlue)
containerColor = MaterialTheme.colorScheme.background
)
}

@Preview
@Composable
private fun PreviewReportProblemSuccessDialog() {
ReportProblemSuccessDialog(
"Email@em.com",
onConfirm = {},
)
AppTheme {
ReportProblemSuccessDialog(
"Email@em.com",
onConfirm = {},
)
}
}

@Composable
Expand All @@ -121,8 +114,8 @@ fun ReportProblemSuccessDialog(email: String?, onConfirm: () -> Unit) {
) {
Image(
painter = painterResource(id = R.drawable.icon_success),
contentDescription = "Remove",
modifier = Modifier.width(50.dp).height(50.dp)
contentDescription = null,
modifier = Modifier.size(Dimens.dialogIconSize)
)
}
},
Expand All @@ -131,16 +124,15 @@ fun ReportProblemSuccessDialog(email: String?, onConfirm: () -> Unit) {
Text(
text =
buildAnnotatedString {
withStyle(SpanStyle(color = colorResource(id = R.color.green))) {
withStyle(SpanStyle(color = MaterialTheme.colorScheme.surface)) {
append(stringResource(id = R.string.sent_thanks))
}
append(" ")

withStyle(SpanStyle(color = colorResource(id = R.color.white))) {
withStyle(SpanStyle(color = MaterialTheme.colorScheme.onPrimary)) {
append(stringResource(id = R.string.we_will_look_into_this))
}
},
fontSize = dimensionResource(id = R.dimen.text_small).value.sp,
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.fillMaxWidth()
)

Expand All @@ -159,8 +151,8 @@ fun ReportProblemSuccessDialog(email: String?, onConfirm: () -> Unit) {

Text(
text = annotatedEmailString,
fontSize = dimensionResource(id = R.dimen.text_small).value.sp,
color = Color.White,
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.fillMaxWidth()
)
}
Expand All @@ -171,24 +163,26 @@ fun ReportProblemSuccessDialog(email: String?, onConfirm: () -> Unit) {
modifier = Modifier.fillMaxWidth(),
colors =
ButtonDefaults.buttonColors(
containerColor = colorResource(id = R.color.blue),
contentColor = Color.White
containerColor = MaterialTheme.colorScheme.primary,
contentColor = MaterialTheme.colorScheme.onPrimary,
),
onClick = { onConfirm() },
text = stringResource(id = R.string.dismiss)
)
},
containerColor = colorResource(id = R.color.darkBlue)
containerColor = MaterialTheme.colorScheme.background
)
}

@Preview
@Composable
private fun PreviewReportProblemErrorDialog() {
ReportProblemErrorDialog(
onDismiss = {},
retry = {},
)
AppTheme {
ReportProblemErrorDialog(
onDismiss = {},
retry = {},
)
}
}

@Composable
Expand All @@ -203,15 +197,14 @@ fun ReportProblemErrorDialog(onDismiss: () -> Unit, retry: () -> Unit) {
Image(
painter = painterResource(id = R.drawable.icon_fail),
contentDescription = null,
modifier = Modifier.width(50.dp).height(50.dp)
modifier = Modifier.size(Dimens.dialogIconSize)
)
}
},
text = {
Text(
text = stringResource(id = R.string.failed_to_send_details),
color = colorResource(id = R.color.white),
fontSize = dimensionResource(id = R.dimen.text_small).value.sp,
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.fillMaxWidth()
)
},
Expand All @@ -220,8 +213,8 @@ fun ReportProblemErrorDialog(onDismiss: () -> Unit, retry: () -> Unit) {
modifier = Modifier.fillMaxWidth(),
colors =
ButtonDefaults.buttonColors(
containerColor = colorResource(id = R.color.blue),
contentColor = Color.White
containerColor = MaterialTheme.colorScheme.primary,
contentColor = MaterialTheme.colorScheme.onPrimary,
),
onClick = onDismiss,
text = stringResource(id = R.string.edit_message)
Expand All @@ -232,13 +225,13 @@ fun ReportProblemErrorDialog(onDismiss: () -> Unit, retry: () -> Unit) {
modifier = Modifier.fillMaxWidth(),
colors =
ButtonDefaults.buttonColors(
containerColor = colorResource(id = R.color.green),
contentColor = Color.White
containerColor = MaterialTheme.colorScheme.surface,
contentColor = MaterialTheme.colorScheme.onPrimary,
),
onClick = retry,
text = stringResource(id = R.string.try_again)
)
},
containerColor = colorResource(id = R.color.darkBlue)
containerColor = MaterialTheme.colorScheme.background
)
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package net.mullvad.mullvadvpn.constant

const val MINIMUM_LOADING_TIME_MILLIS = 500L
const val NAVIGATION_DELAY_MILLIS = 500L
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,12 @@ class MullvadProblemReport(context: Context, val dispatcher: CoroutineDispatcher

val sentSuccessfully =
withContext(dispatcher) {
true
// sendProblemReport(
// userReport.email ?: "",
// userReport.message,
// logsPath.absolutePath,
// cacheDirectory.absolutePath
// )
sendProblemReport(
userReport.email ?: "",
userReport.message,
logsPath.absolutePath,
cacheDirectory.absolutePath
)
}

return if (sentSuccessfully) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import net.mullvad.mullvadvpn.compose.state.LoginError
import net.mullvad.mullvadvpn.compose.state.LoginState
import net.mullvad.mullvadvpn.compose.state.LoginState.*
import net.mullvad.mullvadvpn.compose.state.LoginUiState
import net.mullvad.mullvadvpn.constant.MINIMUM_LOADING_SPINNER_TIME_MILLIS
import net.mullvad.mullvadvpn.constant.MINIMUM_LOADING_TIME_MILLIS
import net.mullvad.mullvadvpn.model.AccountCreationResult
import net.mullvad.mullvadvpn.model.AccountToken
import net.mullvad.mullvadvpn.model.LoginResult
Expand Down Expand Up @@ -75,7 +75,7 @@ class LoginViewModel(
// Ensure we always take at least MINIMUM_LOADING_SPINNER_TIME_MILLIS to show the
// loading indicator
val loginDeferred = async { accountRepository.login(accountToken) }
delay(MINIMUM_LOADING_SPINNER_TIME_MILLIS)
delay(MINIMUM_LOADING_TIME_MILLIS)

val uiState =
when (val result = loginDeferred.await()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import net.mullvad.mullvadvpn.constant.MINIMUM_LOADING_SPINNER_TIME_MILLIS
import net.mullvad.mullvadvpn.constant.MINIMUM_LOADING_TIME_MILLIS
import net.mullvad.mullvadvpn.dataproxy.MullvadProblemReport
import net.mullvad.mullvadvpn.dataproxy.SendProblemReportResult
import net.mullvad.mullvadvpn.dataproxy.UserReport
Expand Down Expand Up @@ -50,7 +50,7 @@ class ReportProblemViewModel(private val mullvadProblemReporter: MullvadProblemR
val deferredResult = async {
mullvadProblemReporter.sendReport(UserReport(nullableEmail, description))
}
delay(MINIMUM_LOADING_SPINNER_TIME_MILLIS)
delay(MINIMUM_LOADING_TIME_MILLIS)

_uiState.update {
it.copy(sendingState = deferredResult.await().toUiResult(nullableEmail))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import net.mullvad.mullvadvpn.constant.NAVIGATION_DELAY_MILLIS
import net.mullvad.mullvadvpn.dataproxy.MullvadProblemReport

data class ViewLogsUiState(val allLines: String = "", val isLoading: Boolean = true)
Expand All @@ -22,7 +23,7 @@ class ViewLogsViewModel(private val mullvadProblemReporter: MullvadProblemReport
// fragment transitions is done. I'd very much prefer to use LazyColumn in the view
// which would make the loading way faster but then the SelectionContainer is broken and
// text would not be copyable.
delay(500)
delay(NAVIGATION_DELAY_MILLIS)
_uiState.update {
it.copy(
allLines = mullvadProblemReporter.readLogs().joinToString("\n"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ data class Dimensions(
val titleIconSize: Dp = 24.dp,
val topBarHeight: Dp = 64.dp,
val verticalSpace: Dp = 20.dp,
val verticalSpacer: Dp = 1.dp
val verticalSpacer: Dp = 1.dp,
val dialogIconSize: Dp = 48.dp
)

val defaultDimensions = Dimensions()
Expand Down

0 comments on commit 1ac53ac

Please sign in to comment.