Refactor UI to strict Material Design 3#12
Conversation
- Add standard Material 3 Typography and Shape definitions. - Hook up Typography and Shapes to the root MaterialTheme. - Migrate TrackingScreen to standard M3 Cards and remove legacy text colors. - Migrate HistoryScreen to M3 Cards with surfaceVariant coloration for tonal elevation. - Migrate SettingsScreen grouped sections to M3 Cards. - Migrate RoutineAnalysisScreen to M3 Cards. - Migrate PermissionScreen elements to M3 Cards. - Clean up unused string/variables triggered during cleanup. Co-authored-by: Max97k <14903047+Max97k@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors several screens (History, Permission, Routine Analysis, Settings, and Tracking) to wrap UI components in Cards with a surfaceVariant background and apply Material 3 compliant colors. It also integrates custom shapes and typography into the application theme. The review feedback suggests improving performance by replacing an inefficient Surface with a lightweight Box for drawing simple shapes, and recommends moving several hardcoded user-facing strings, labels, and units into resource files to support proper localization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
| Row(verticalAlignment = Alignment.CenterVertically) { | ||
| // Color block | ||
| Surface(color = color, modifier = Modifier.size(16.dp), shape = MaterialTheme.shapes.small) {} |
There was a problem hiding this comment.
Using Surface just to draw a simple colored shape is inefficient because Surface is a relatively heavy composable that handles elevation, borders, and input interception. Instead, use a lightweight Box with background and clip modifiers.
| Surface(color = color, modifier = Modifier.size(16.dp), shape = MaterialTheme.shapes.small) {} | |
| Box(modifier = Modifier.size(16.dp).background(color, MaterialTheme.shapes.small)) |
| Surface(color = color, modifier = Modifier.size(16.dp), shape = MaterialTheme.shapes.small) {} | ||
| Spacer(modifier = Modifier.width(8.dp)) | ||
| Text( | ||
| text = "${state.name}: $hours h $minutes min", |
There was a problem hiding this comment.
Avoid hardcoding user-facing strings and time unit labels like 'h' and 'min'. These should be defined as a plural or formatted string resource in strings.xml to support proper localization and internationalization (i18n).
| text = "${state.name}: $hours h $minutes min", | |
| text = stringResource(R.string.routine_state_duration, state.name, hours, minutes), |
| ) | ||
| } else { | ||
| Box(modifier = Modifier.fillMaxWidth().height(250.dp), contentAlignment = Alignment.Center) { | ||
| Text("No tracking time accumulated", style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) |
There was a problem hiding this comment.
Avoid hardcoding user-facing text. Use stringResource to load the string from strings.xml to ensure the application can be localized.
| Text("No tracking time accumulated", style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) | |
| Text(stringResource(R.string.routine_no_tracking_time), style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) |
| } | ||
| } else { | ||
| Box(modifier = Modifier.fillMaxWidth().height(250.dp), contentAlignment = Alignment.Center) { | ||
| Text("No data for this day", style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) |
There was a problem hiding this comment.
Avoid hardcoding user-facing text. Use stringResource to load the string from strings.xml to ensure the application can be localized.
| Text("No data for this day", style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) | |
| Text(stringResource(R.string.routine_no_data), style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) |
| Text(text = "Lat: ${currentLocation?.latitude}, Lon: ${currentLocation?.longitude}", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) | ||
| Text(text = "Alt: ${currentLocation?.altitude}m, Speed: ${currentLocation?.speed}m/s", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) |
There was a problem hiding this comment.
Avoid hardcoding labels ('Lat:', 'Lon:', 'Alt:', 'Speed:') and units ('m', 'm/s') directly in the code. These should be defined as formatted string resources in strings.xml to support localization.
| Text(text = "Lat: ${currentLocation?.latitude}, Lon: ${currentLocation?.longitude}", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) | |
| Text(text = "Alt: ${currentLocation?.altitude}m, Speed: ${currentLocation?.speed}m/s", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) | |
| Text(text = stringResource(R.string.track_lat_lon, currentLocation?.latitude ?: 0.0, currentLocation?.longitude ?: 0.0), style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant)\n Text(text = stringResource(R.string.track_alt_speed, currentLocation?.altitude ?: 0.0, currentLocation?.speed ?: 0f), style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) |
1 participant
Refactors the entire application UI to strictly comply with Jetpack Compose Material Design 3 guidelines. Replaces raw column groupings with standard
Cardcomponents implementingtonalElevation(viasurfaceVariantbackground color). Replaces all hardcoded colors withMaterialTheme.colorSchemevariants, effectively supporting dynamic color correctly everywhere. Fixes emptyTypographyandShapesimplementations with standard M3 values.PR created automatically by Jules for task 1238639963804352306 started by @Max97k