Skip to content

Commit 49de8d4

Browse files
breslee1707breslee1707
committed
Fix the launch crash I shipped, and add the test that would have caught it
The build handed over would not start. HeaderView asked Compose's painterResource for R.mipmap.ic_launcher, which on API 26+ resolves to the <adaptive-icon> XML; painterResource reads vectors and bitmaps only and throws IllegalArgumentException on the first composition. Reproduced on an emulator from the exact APK: java.lang.IllegalArgumentException: Only VectorDrawables and rasterized asset types are supported ex. PNG, JPG, WEBP at androidx.compose.ui.res.PainterResources_androidKt.loadVectorResource at com.vitranslate.pdf.ui.components.HeaderViewKt.HeaderView(HeaderView.kt:44) The header now draws ic_app_mark, a plain PNG generated per density from the same logo, full bleed rather than inset for a launcher mask it is not going through. Compiling, unit tests and lint all passed on the broken build, and none of them could have failed: the fault only exists once an activity composes. LaunchSmokeTest starts MainActivity for real. Putting the bug back makes it fail with the exception above, which is the only reason to trust it. Two things the emulator then showed: - The notification permission was requested in onCreate, so a system dialog covered the app before the user had done anything. Android's guidance is to ask in context, and the notification only matters once a translation is running, so it is asked for at "Bắt đầu dịch" instead. This was also why the smoke test could not find the composition. - The About dialog's title row had no weight, so the title clipped and pushed the close button off the edge. It also used a 32dp box holding a ✕ character.
1 parent e44167e commit 49de8d4

12 files changed

Lines changed: 108 additions & 34 deletions

File tree

‎android/app/build.gradle.kts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,5 +143,7 @@ dependencies {
143143
androidTestImplementation(libs.androidx.junit)
144144
androidTestImplementation(libs.androidx.espresso.core)
145145
androidTestImplementation(platform(libs.androidx.compose.bom))
146+
androidTestImplementation(libs.androidx.ui.test.junit4)
146147
debugImplementation(libs.androidx.ui.tooling)
148+
debugImplementation(libs.androidx.ui.test.manifest)
147149
}

‎android/app/src/androidTest/java/com/vitranslate/pdf/LaunchSmokeTest.kt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package com.vitranslate.pdf
2+
3+
import androidx.compose.ui.test.assertIsDisplayed
4+
import androidx.compose.ui.test.junit4.createAndroidComposeRule
5+
import androidx.compose.ui.test.onNodeWithText
6+
import androidx.test.ext.junit.runners.AndroidJUnit4
7+
import org.junit.Rule
8+
import org.junit.Test
9+
import org.junit.runner.RunWith
10+
11+
/**
12+
* Does the app open?
13+
*
14+
* Unit tests and lint both passed on a build that crashed the moment it was
15+
* launched: the header asked Compose's painterResource for R.mipmap.ic_launcher,
16+
* which on API 26+ resolves to the <adaptive-icon> XML that painterResource
17+
* cannot read. Nothing that runs on the JVM or reads source can catch that —
18+
* only starting the activity can.
19+
*
20+
* This test composes the real screen, so a resource that fails to load, a theme
21+
* that fails to resolve or a crash in the first composition all fail here.
22+
*/
23+
@RunWith(AndroidJUnit4::class)
24+
class LaunchSmokeTest {
25+
26+
@get:Rule
27+
val composeRule = createAndroidComposeRule<MainActivity>()
28+
29+
@Test
30+
fun theMainScreenComposes() {
31+
composeRule.onNodeWithText("PDF Translate").assertIsDisplayed()
32+
}
33+
34+
@Test
35+
fun theQueueAndActionAreOnScreen() {
36+
composeRule.onNodeWithText("Hàng đợi").assertIsDisplayed()
37+
composeRule.onNodeWithText("Bắt đầu dịch").assertIsDisplayed()
38+
}
39+
}

‎android/app/src/main/java/com/vitranslate/pdf/MainActivity.kt‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,6 @@ class MainActivity : ComponentActivity() {
7979
enableEdgeToEdge()
8080
super.onCreate(savedInstanceState)
8181

82-
askForNotificationPermission()
83-
8482
// Handle incoming PDF shared intent
8583
intent?.let { handleIntent(it) }
8684

@@ -94,24 +92,33 @@ class MainActivity : ComponentActivity() {
9492
viewModel = viewModel,
9593
onPickFiles = { pickFilesLauncher.launch(arrayOf("application/pdf")) },
9694
onPickDirectory = { pickDirectoryLauncher.launch(null) },
97-
onPickSaveDirectory = { pickSaveDirectoryLauncher.launch(null) }
95+
onPickSaveDirectory = { pickSaveDirectoryLauncher.launch(null) },
96+
onStartTranslation = { startTranslationAskingToNotify() }
9897
)
9998
}
10099
}
101100
}
102101
}
103102

104103
/**
105-
* The foreground service runs either way; without this the progress bar and
106-
* the Huỷ action simply never appear.
104+
* Asked when a translation starts, not at launch.
105+
*
106+
* Requesting it in onCreate put a system permission dialog over the app
107+
* before the user had done anything, which is what Android's own guidance
108+
* tells you not to do. The notification only matters once there is progress
109+
* to report, so that is when it is worth interrupting for.
110+
*
111+
* The foreground service runs either way; without the permission the
112+
* progress bar and the Huỷ action simply never appear.
107113
*/
108-
private fun askForNotificationPermission() {
109-
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) return
110-
val granted = checkSelfPermission(Manifest.permission.POST_NOTIFICATIONS) ==
114+
private fun startTranslationAskingToNotify() {
115+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU &&
116+
checkSelfPermission(Manifest.permission.POST_NOTIFICATIONS) !=
111117
PackageManager.PERMISSION_GRANTED
112-
if (!granted) {
118+
) {
113119
requestNotificationPermission.launch(Manifest.permission.POST_NOTIFICATIONS)
114120
}
121+
viewModel.startTranslation()
115122
}
116123

117124
override fun onNewIntent(intent: Intent) {
@@ -139,7 +146,8 @@ fun MainScreen(
139146
viewModel: MainViewModel,
140147
onPickFiles: () -> Unit,
141148
onPickDirectory: () -> Unit,
142-
onPickSaveDirectory: () -> Unit
149+
onPickSaveDirectory: () -> Unit,
150+
onStartTranslation: () -> Unit
143151
) {
144152
val queueItems by viewModel.queueItems.collectAsState()
145153
val selectedLanguage by viewModel.selectedLanguage.collectAsState()
@@ -203,7 +211,7 @@ fun MainScreen(
203211
customSaveDirectory = customSaveDirectory,
204212
onPickSaveDirectory = onPickSaveDirectory,
205213
isTranslating = isTranslating,
206-
onStartTranslation = { viewModel.startTranslation() },
214+
onStartTranslation = onStartTranslation,
207215
onCancelTranslation = { viewModel.cancelTranslation() }
208216
)
209217

‎android/app/src/main/java/com/vitranslate/pdf/ui/components/AboutDialog.kt‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,22 @@ import androidx.compose.foundation.shape.RoundedCornerShape
99
import androidx.compose.foundation.verticalScroll
1010
import androidx.compose.material.icons.Icons
1111
import androidx.compose.material.icons.filled.OpenInNew
12-
import androidx.compose.material.icons.filled.PictureAsPdf
12+
import androidx.compose.material.icons.filled.Close
1313
import androidx.compose.material3.*
1414
import androidx.compose.runtime.Composable
1515
import androidx.compose.ui.Alignment
1616
import androidx.compose.ui.Modifier
17+
import androidx.compose.foundation.Image
18+
import androidx.compose.ui.draw.clip
1719
import androidx.compose.ui.platform.LocalContext
20+
import androidx.compose.ui.res.painterResource
1821
import androidx.compose.ui.text.font.FontWeight
22+
import androidx.compose.ui.text.style.TextOverflow
1923
import androidx.compose.ui.text.style.TextDecoration
2024
import androidx.compose.ui.unit.dp
2125
import androidx.compose.ui.unit.sp
2226
import androidx.compose.ui.window.Dialog
27+
import com.vitranslate.pdf.R
2328

2429
@Composable
2530
fun AboutDialog(
@@ -54,32 +59,36 @@ fun AboutDialog(
5459
.fillMaxSize()
5560
.padding(18.dp)
5661
) {
62+
// The title row had no weight, so a title this long pushed the
63+
// close button past the dialog edge and clipped itself.
5764
Row(
5865
modifier = Modifier.fillMaxWidth(),
59-
verticalAlignment = Alignment.CenterVertically,
60-
horizontalArrangement = Arrangement.SpaceBetween
66+
verticalAlignment = Alignment.CenterVertically
6167
) {
62-
Row(verticalAlignment = Alignment.CenterVertically) {
68+
Image(
69+
painter = painterResource(R.drawable.ic_app_mark),
70+
contentDescription = null,
71+
modifier = Modifier
72+
.size(28.dp)
73+
.clip(RoundedCornerShape(6.dp))
74+
)
75+
Spacer(modifier = Modifier.width(8.dp))
76+
Text(
77+
text = "Ghi nhận & Mã nguồn",
78+
style = MaterialTheme.typography.titleMedium,
79+
fontWeight = FontWeight.Bold,
80+
maxLines = 1,
81+
overflow = TextOverflow.Ellipsis,
82+
modifier = Modifier.weight(1f)
83+
)
84+
IconButton(onClick = onDismiss) {
6385
Icon(
64-
imageVector = Icons.Default.PictureAsPdf,
65-
contentDescription = "App Icon",
66-
tint = MaterialTheme.colorScheme.primary,
67-
modifier = Modifier.size(28.dp)
68-
)
69-
Spacer(modifier = Modifier.width(8.dp))
70-
Text(
71-
text = "Ghi nhận & Mã nguồn (Credits)",
72-
style = MaterialTheme.typography.titleMedium,
73-
fontWeight = FontWeight.Bold
86+
imageVector = Icons.Default.Close,
87+
contentDescription = "Đóng",
88+
tint = MaterialTheme.colorScheme.onSurfaceVariant,
89+
modifier = Modifier.size(20.dp)
7490
)
7591
}
76-
77-
IconButton(
78-
onClick = onDismiss,
79-
modifier = Modifier.size(32.dp)
80-
) {
81-
Text("✕", style = MaterialTheme.typography.titleMedium)
82-
}
8392
}
8493

8594
Spacer(modifier = Modifier.height(10.dp))

‎android/app/src/main/java/com/vitranslate/pdf/ui/components/HeaderView.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ fun HeaderView(
4141
// The app's own mark rather than a generic PDF glyph, so the header and
4242
// the launcher icon are recognisably the same product.
4343
Image(
44-
painter = painterResource(R.mipmap.ic_launcher),
44+
painter = painterResource(R.drawable.ic_app_mark),
4545
contentDescription = null,
4646
modifier = Modifier
4747
.size(36.dp)

‎android/app/src/main/res/drawable-hdpi/ic_app_mark.png‎

4.94 KB
Loading

‎android/app/src/main/res/drawable-mdpi/ic_app_mark.png‎

3.08 KB
Loading

‎android/app/src/main/res/drawable-xhdpi/ic_app_mark.png‎

7.07 KB
Loading

‎android/app/src/main/res/drawable-xxhdpi/ic_app_mark.png‎

11.7 KB
Loading

‎android/app/src/main/res/drawable-xxxhdpi/ic_app_mark.png‎

17.3 KB
Loading

0 commit comments

Comments
 (0)