Settings Screen: Java/XML to Kotlin/Compose migration by m4pl · Pull Request #56 · GrapheneOS/platform_packages_apps_Contacts

m4pl · 2026-08-08T13:08:12Z

Closes #41

Screenshots

Before	After

Video

Screen_recording_20260808_150325.mp4

sdsantos

UI things I found:

I never see the snackbar messages after an Import finishes. Is it only broadcasted in special circumstances?
I feel the Settings cells should be lighter and not darker than the background. It helps having a greater contrast with the cell description (onSurfaceVariant). Here's an example from the latest Google Messaging app:

Messaging	New Contacts

Their space between cells is also slightly lower (1 vs 2 dp), which I find pleasant. And also the top padding is slightly lower (8 vs 16 dp). But I'm not 100% sure those are improvements to be honest.

sdsantos · 2026-08-11T11:22:01Z

@@ -0,0 +1,95 @@
+package com.android.contacts.ui.settings.about.ui


Not sure this needs the extra ui package.

sdsantos · 2026-08-11T11:24:32Z

+                ),
+        ) {
+            SettingsCell(
+                title = stringResource(R.string.about_build_version),


It's a new requirement, but I always feel build / version numbers should always be at least selectable, if not copy-able.

sdsantos · 2026-08-11T11:40:46Z

+    sealed interface Host : SettingsEffect
+    sealed interface Message : SettingsEffect


Nice approach, hadn't seen it before 👍

sdsantos · 2026-08-11T11:44:18Z

+    ) { currentRoute ->
+        when (currentRoute) {
+            SettingsNavRoute.Main -> {
+                SettingsMainScreen(


Naming is definitely hard.

SettingsScreen > SettingsContent > SettingsMainScreen feels a bit weird to me, but do we have an alternative name for this parent Screens than hold navigation inside for other screens? Or maybe we call the inner screens Panes like for adaptive layouts?

sdsantos · 2026-08-11T11:50:12Z

+            displaySettingsRepository.setSortOrder(sortOrder)
+            refreshState()


This we're writing the Repository ourselves here, we could also make them reactive instead: return a flow, and the flow automatically updates on a set.

M00NJ · 2026-08-11T13:01:03Z

Sorry for barging in here. I'd just like to comment on this, since I noticed similar things while making mockups for #37.

I feel the Settings cells should be lighter and not darker than the background. It helps having a greater contrast with the cell description (onSurfaceVariant). Here's an example from the latest Google Messaging app:

The way it is now is in accordance to the MD3E guidelines. Google themself don't always stick to them however and their implementation is inconsistent across their apps. I'd suggest going with the Material Design guidelines over Google's reference when in question. Apart from that I would argue, that there's more separation and emphasis on each settings item here, giving the screen more structure, compared to Google's implementation. Contrast is still sufficient if you ask me.

Their space between cells is also slightly lower (1 vs 2 dp), which I find pleasant.

I think this might be an optical illusion as a result of the color roles being reversed here.

sdsantos · 2026-08-11T13:32:22Z

Sorry for barging in here. I'd just like to comment on this, since I noticed similar things while making mockups for #37.

I feel the Settings cells should be lighter and not darker than the background. It helps having a greater contrast with the cell description (onSurfaceVariant). Here's an example from the latest Google Messaging app:

The way it is now is in accordance to the MD3E guidelines. Google themself don't always stick to them however and their implementation is inconsistent across their apps. I'd suggest going with the Material Design guidelines over Google's reference when in question. Apart from that I would argue, that there's more separation and emphasis on each settings item here, giving the screen more structure, compared to Google's implementation. Contrast is still sufficient if you ask me.

I feel even the MD3E guidelines has examples with both approaches, that's why I started looking at concrete apps. But you are right that in general the Color guideline is Background (outside) > SurfaceContainer (inside). Maybe I should switch here as well then (#33), since I was matching the Google Contacts app.

But maybe we can tone it down a bit? We're defaulting to surfaceContainerHigh on the cells, when a surfaceContainer would do just fine. I feel we just need the Lows and Highs when we have more hierarchies?

I tested the text color contrast with the accessibility scanner and onSurfaceVariant passes, so I guess although there's less contrast, it's enough as you said.

Their space between cells is also slightly lower (1 vs 2 dp), which I find pleasant.

I think this might be an optical illusion as a result of the color roles being reversed here.

Could be yeah. In the guidelines I only find a mention to a List segment gap of 2dp.

sdsantos · 2026-08-11T14:41:43Z

I also noticed the scroll position it's not kept when returning from AboutScreen to the MainSettingsScreen. Not a big deal, but I sort of expected it.

M00NJ · 2026-08-12T11:20:06Z

I feel even the MD3E guidelines has examples with both approaches, that's why I started looking at concrete apps.

Yes, it seems not completely clear cut. I also looked around for more examples. The new Gallery app for example matches the current state. Interestingly the systems settings match the current behavior in dark mode, but in light mode it's reversed and matches Google Messagings implementation.

But maybe we can tone it down a bit? We're defaulting to surfaceContainerHigh on the cells, when a surfaceContainer would do just fine. I feel we just need the Lows and Highs when we have more hierarchies?

I wouldn't recommend that. There's quite a bit of thought in those color roles and it might not matter for this screen, but if you want to apply this evenly across all screens and apps, you might end up in a situation where these color roles where reserved for other components and on screen and now you'd have to change those components as well to retain a hierarchy. So I think it's better to pick between the default behavior, how it is now and Google Messagings implementation. Neither is a bad choice, though I would personally lean towards the current one.

RankoR

Also:

AppCompat dialogs crash under Theme.Compose, worth adding <item name="alertDialogTheme">@style/ContactsAlertDialogThemeAppCompat</item> to theme
accompanist-drawablepainter and kotlin-parcelize seem to be dead dependencies now
lint-baseline.xml references deleted files

RankoR · 2026-08-19T15:31:23Z

+    private val profile: StateFlow<ProfileData?> = profileRepository.observeProfile()
+        .stateIn(
+            scope = viewModelScope,
+            started = SharingStarted.Eagerly,


Maybe WhileSubscribed(STATE_STOP_TIMEOUT_MILLIS)?

RankoR · 2026-08-19T15:33:29Z

+                        buildVersion = uiState.buildVersion,
+                        onBuildVersionLongClick = { onAction(Action.BuildVersionLongClicked) },
+                        onLicensesClick = { onAction(Action.LicensesClicked) },
+                        onNavigateBack = { route = SettingsNavRoute.Main },


No snackbarHostState here

RankoR · 2026-08-19T15:34:39Z

+    effectHandler: SettingsEffectHandler,
+    onNavigateBack: () -> Unit,
+    modifier: Modifier = Modifier,
+    screenModel: SettingsScreenModel = viewModel<SettingsViewModel>(),


Should be hiltViewModel<SettingsViewModel>() I guess

m4pl requested review from RankoR, inthewaves and sdsantos August 8, 2026 13:08

sdsantos approved these changes Aug 11, 2026

View reviewed changes

m4pl added 15 commits August 14, 2026 23:08

Sync build configuration

9f57664

Port shared Compose and DI foundation

f46823c

Port accounts domain layer

af651dc

Add display settings repository

47b01c7

Add settings availability repository

817bf92

Add profile repository

4c0faa8

Add contacts filter and SIM import result repositories

cf5e37a

Add settings UI models

68ba7bb

Remove ported code the settings screen does not use

bb372cf

Add settings ViewModel

6deb00f

Add reusable settings components

c49a7d0

Add settings main screen

48896fa

Add about screen

7963056

Replace preference settings screen with Compose

713a211

Polish the Compose settings screen

8ba0c2d

m4pl force-pushed the task/41-settings-compose branch from 04c125c to 8ba0c2d Compare August 14, 2026 21:08

m4pl added 3 commits August 15, 2026 01:01

Rework settings navigation

8717de8

Copy build version on tap

21ff69d

Observe display settings

5b4ba07

RankoR suggested changes Aug 19, 2026

View reviewed changes

		@@ -0,0 +1,95 @@
		package com.android.contacts.ui.settings.about.ui

		sealed interface Host : SettingsEffect
		sealed interface Message : SettingsEffect

		displaySettingsRepository.setSortOrder(sortOrder)
		refreshState()

Uh oh!

Uh oh!

Conversation

m4pl commented Aug 8, 2026

Screenshots

Video

Uh oh!

sdsantos left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

M00NJ commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdsantos commented Aug 11, 2026

Uh oh!

sdsantos commented Aug 11, 2026

Uh oh!

M00NJ commented Aug 12, 2026

Uh oh!

RankoR left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sdsantos left a comment •

edited

Loading

M00NJ commented Aug 11, 2026 •

edited

Loading