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.

Sign up for GitHub

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

feat: add counts to summary cards #352

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add counts to summary cards #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd8e33a
feat: add counts to summary cards
aanorbel Dec 12, 2024
0cb840b
chore: correct tests
aanorbel Dec 13, 2024
ed18a39
fix based on review
aanorbel Dec 18, 2024
ac61165
fix based on review
aanorbel Dec 18, 2024
52f319e
fix: tests
aanorbel Dec 18, 2024
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
6 changes: 6 additions & 0 deletions composeApp/src/commonMain/composeResources/values/strings-common.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@
<string name="TestResults_NotAvailable">N/A</string>
<string name="TestResults_Summary_Performance_Hero_Upload">Upload</string>
<string name="TestResults_Summary_Performance_Hero_Download">Download</string>


<string name="TestResults_Summary_Websites_Hero_Tested_Singular">Tested</string>
<string name="TestResults_Summary_Websites_Hero_Blocked_Singular">Blocked</string>
<string name="TestResults_Summary_Websites_Hero_Reachable_Singular">Accessible</string>
Comment on lines +63 to +65
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are different strings for circunvention, websites and instant messaging

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its thesame string. I doubt we want to keep 12+ strings saying similar things.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about other languages, but at least in English, Instant Messaging uses accessible and Circumvention uses available.


<string name="Modal_ReRun_Title">Re-run test</string>
<string name="Modal_ReRun_Websites_Title">You are about to re-test %1$s websites.</string>
<string name="Modal_ReRun_Websites_Run">Run</string>
Expand Down
12 changes: 12 additions & 0 deletions composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/ResultItem.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package org.ooni.probe.data.models

import kotlin.time.Duration.Companion.seconds

data class ResultCount(val total: Long, val succeeded: Long) {
val failed get() = total - succeeded
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised that we're mixing failed and anomalies here.
Blocked = isAnomaly
Failed = isFailed

We shouldn't be counting failures inside the blocked count.

}

data class ResultItem(
val result: ResultModel,
val descriptor: Descriptor,
Expand All @@ -16,4 +20,12 @@ data class ResultItem(
val canBeRerun get() = descriptor.name == "websites" && result.isDone && urlCount > 0

val urlCount get() = measurements.count { it.url != null }

val measurementCounts
get() = ResultCount(
total = measurements.size.toLong(),
succeeded = measurements.count {
!it.measurement.isFailed && !it.measurement.isAnomaly && it.measurement.isDone
}.toLong(),
)
}
85 changes: 71 additions & 14 deletions composeApp/src/commonMain/kotlin/org/ooni/probe/ui/result/ResultScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.IntrinsicSize
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.asPaddingValues
import androidx.compose.foundation.layout.defaultMinSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.navigationBars
import androidx.compose.foundation.layout.offset
import androidx.compose.foundation.layout.padding
Expand All @@ -30,6 +32,7 @@ import androidx.compose.material3.Surface
import androidx.compose.material3.Text
import androidx.compose.material3.TextButton
import androidx.compose.material3.TopAppBarDefaults
import androidx.compose.material3.VerticalDivider
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
Expand All @@ -50,7 +53,6 @@ import ooniprobe.composeapp.generated.resources.NetworkType_Vpn
import ooniprobe.composeapp.generated.resources.Res
import ooniprobe.composeapp.generated.resources.TestResults_NotAvailable
import ooniprobe.composeapp.generated.resources.TestResults_Overview_Hero_DataUsage
import ooniprobe.composeapp.generated.resources.TestResults_Overview_Hero_Tests
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Hero_Country
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Hero_DateAndTime
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Hero_Mobile
Expand All @@ -60,6 +62,9 @@ import ooniprobe.composeapp.generated.resources.TestResults_Summary_Hero_Runtime
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Hero_WiFi
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Performance_Hero_Download
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Performance_Hero_Upload
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Websites_Hero_Blocked_Singular
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Websites_Hero_Reachable_Singular
import ooniprobe.composeapp.generated.resources.TestResults_Summary_Websites_Hero_Tested_Singular
import ooniprobe.composeapp.generated.resources.ic_download
import ooniprobe.composeapp.generated.resources.ic_replay
import ooniprobe.composeapp.generated.resources.ic_upload
Expand Down Expand Up @@ -166,7 +171,7 @@ fun ResultScreen(

@Composable
private fun Summary(item: ResultItem) {
val pagerState = rememberPagerState(pageCount = { 2 })
val pagerState = rememberPagerState(pageCount = { 3 })
Box {
HorizontalPager(
state = pagerState,
Expand All @@ -176,8 +181,9 @@ private fun Summary(item: ResultItem) {
.defaultMinSize(minHeight = 128.dp),
) { page ->
when (page) {
0 -> SummaryDetails(item)
1 -> SummaryNetwork(item)
0 -> SummaryStats(item)
1 -> SummaryDetails(item)
2 -> SummaryNetwork(item)
}
}
Row(
Expand Down Expand Up @@ -210,21 +216,72 @@ private fun Summary(item: ResultItem) {
}

@Composable
private fun SummaryDetails(item: ResultItem) {
Column(Modifier.padding(horizontal = 16.dp)) {
val labelModifier = Modifier.weight(4f)
val valueModifier = Modifier.weight(6f)
Row {
private fun SummaryStats(item: ResultItem) {
Row(
modifier = Modifier
.fillMaxWidth()
.height(IntrinsicSize.Min) // So VerticalDividers don't expand to the whole screen
.padding(16.dp),
) {
Column(
modifier = Modifier.weight(1f),
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(
stringResource(Res.string.TestResults_Overview_Hero_Tests),
fontWeight = FontWeight.Bold,
modifier = labelModifier,
item.measurementCounts.total.toString(),
style = MaterialTheme.typography.headlineMedium,
)

Text(
stringResource(Res.string.TestResults_Summary_Websites_Hero_Tested_Singular),
style = MaterialTheme.typography.labelLarge,
modifier = Modifier.padding(bottom = 8.dp),
)
}

VerticalDivider(Modifier.padding(4.dp), color = LocalContentColor.current)

Column(
modifier = Modifier.weight(1f),
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(
item.measurements.size.toString(),
modifier = valueModifier,
item.measurementCounts.failed?.toString() ?: "0",
style = MaterialTheme.typography.headlineMedium,
)

Text(
stringResource(Res.string.TestResults_Summary_Websites_Hero_Blocked_Singular),
style = MaterialTheme.typography.labelLarge,
modifier = Modifier.padding(bottom = 8.dp),
)
}

VerticalDivider(Modifier.padding(4.dp), color = LocalContentColor.current)

Column(
modifier = Modifier.weight(1f),
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(
item.measurementCounts.succeeded?.toString() ?: "0",
style = MaterialTheme.typography.headlineMedium,
)

Text(
stringResource(Res.string.TestResults_Summary_Websites_Hero_Reachable_Singular),
style = MaterialTheme.typography.labelLarge,
modifier = Modifier.padding(bottom = 8.dp),
)
}
}
}

@Composable
private fun SummaryDetails(item: ResultItem) {
Column(Modifier.padding(horizontal = 16.dp)) {
val labelModifier = Modifier.weight(4f)
val valueModifier = Modifier.weight(6f)
Row {
Text(
stringResource(Res.string.TestResults_Summary_Hero_DateAndTime),
Expand Down