Settings Screen: Java/XML to Kotlin/Compose migration + tests by m4pl · Pull Request #101 · GrapheneOS/Messaging

m4pl · 2026-04-03T18:59:44Z

Migrated the Settings screen from Java + XML to Kotlin + Compose. Added unit tests for Composable(s) and ViewModel. Additionally, wrote an E2E test covering the flow: user opens Conversations list screen and navigates to a conversation.

Screenshots

Before	After

Figma

https://figma.com/design/JtQuS3wQQs9YdwrtZtGw0y

RankoR · 2026-04-06T15:37:16Z

+import dagger.hilt.components.SingletonComponent
+
+@Module
+@InstallIn(SingletonComponent::class)


Should be ViewModelComponent probably

Right, will fix

inthewaves · 2026-04-07T20:03:06Z

+            is SettingsNavRoute.SubscriptionSettings -> {
+                val sub = uiState.subscriptionSettings.find { it.subId == route.subId }
+                if (sub != null) {
+                    SubscriptionSettingsScreen(
+                        subscriptionSettings = sub,
+                        title = route.title,
+                        screenModel = screenModel,
+                        onNavigateBack = navigateUp,
+                    )
+                }
+            }


On a dual-SIM setup, if looking at settings for a SIM and that SIM is removed (eSIM is disabled or physical SIM is removed), sub will be null. Since nothing is used for sub == null case, the entire screen turns white until back gesture.

We should show something (e.g. make SubscriptionSettingsScreen accept a nullable subscriptionSettings and handle it inside) or force the user to go back when SIM is removed (i.e., when sub is null) while SubscriptionSettingsScreen is currently being used (and then add a test for this case)

Good catch, this definitely needs to be handled

inthewaves · 2026-04-07T20:39:02Z

+        val displayPhoneNumber = when {
+            !savedPhoneNumber.isNullOrEmpty() -> phoneUtils.formatForDisplay(savedPhoneNumber)
+            !defaultPhoneNumber.isNullOrEmpty() -> phoneUtils.formatForDisplay(defaultPhoneNumber)
+            else -> context.getString(R.string.unknown_phone_number_pref_display_value)
+        }


Current code only uses the equivalent of PhoneUtils.get(mSubId).formatForDisplay(value)

Previous code used PhoneUtils.get(mSubId).formatForDisplay(value) then bidiFormatter.unicodeWrap(displayValue, TextDirectionHeuristicsCompat.LTR); for phone number preference summary.

Messaging/src/com/android/messaging/ui/appsettings/PhoneNumberPreference.java

Lines 90 to 105 in f1e0efc

@Nullable

@Override

public CharSequence getSummary() {

// Show the preference value if it's set, or the default number if not.

// If we don't have a default, fall back to a static string (e.g. Unknown).

String value = getText();

if (TextUtils.isEmpty(value)) {

value = mDefaultPhoneNumber;

}

final String displayValue = (!TextUtils.isEmpty(value))

? PhoneUtils.get(mSubId).formatForDisplay(value)

: getContext().getString(R.string.unknown_phone_number_pref_display_value);

final BidiFormatter bidiFormatter = BidiFormatter.getInstance();

return bidiFormatter.unicodeWrap

(displayValue, TextDirectionHeuristicsCompat.LTR);

}

It seems we should add this back. There's also the Compose-equivalent of this like TextStyle(textDirection = TextDirection.ContentOrLtr) for phone number Text or OutlinedTextFields. The BidiFormatter usage seems easier/more unconditional though + it would help if string is embedded somewhere else.

Thanks, I’ll take another look

inthewaves · 2026-04-07T20:46:09Z

+            Switch(
+                checked = checked,
+                onCheckedChange = null,
+            )


the enabled flag should be passed to the switch. Currently the switch is not dimmed when the SettingsSwitchItem's enabled is false

e.g., disable the Auto-retrieve preference and look at the switch for Roaming auto-retrieve, especially when Roaming auto-retrieve is on

Agreed, I’ll update it so it is less confusing for users

inthewaves · 2026-04-07T20:47:07Z

+                )
+            }
+
+            items(subscriptions) { subscription ->


Suggested change

items(subscriptions) { subscription ->

items(subscriptions, key = { it.subId }) { subscription ->

inthewaves · 2026-04-07T20:55:45Z

+    @Before
+    fun setUpDefaultSmsApp() {
+        val instrumentation = InstrumentationRegistry.getInstrumentation()
+        val packageName = instrumentation.targetContext.packageName
+        val command = "cmd role add-role-holder android.app.role.SMS $packageName"
+        val parcelFileDescriptor = instrumentation.uiAutomation.executeShellCommand(command)
+
+        ParcelFileDescriptor.AutoCloseInputStream(parcelFileDescriptor).use { inputStream ->
+            val result = String(inputStream.readBytes())
+            println("Role assignment result: $result")
+        }
+    }


This seems like it could be in a @BeforeClass as well (if more tests are added, it'll run for every test)

Yes, that is a good idea for future tests. I’ll update it

inthewaves · 2026-04-07T21:22:39Z

        <activity
            android:name=".ui.appsettings.SettingsActivity"
-            android:label="@string/settings_activity_title"
-            android:theme="@style/BugleTheme.SettingsActivity"
-            android:configChanges="orientation|screenSize|keyboardHidden"
-            android:screenOrientation="user"
-            android:parentActivityName="com.android.messaging.ui.conversationlist.ConversationListActivity">
-            <meta-data
-                android:name="android.support.PARENT_ACTIVITY"
-                android:value="com.android.messaging.ui.conversationlist.ConversationListActivity" />
-        </activity>
-
-        <activity
-            android:name=".ui.appsettings.PerSubscriptionSettingsActivity"
-            android:label="@string/advanced_settings_activity_title"
-            android:theme="@style/BugleTheme.SettingsActivity"
+            android:allowEmbedded="true"
            android:configChanges="orientation|screenSize|keyboardHidden"
-            android:screenOrientation="user"
-            android:parentActivityName="com.android.messaging.ui.appsettings.SettingsActivity">
-            <meta-data
-                android:name="android.support.PARENT_ACTIVITY"
-                android:value="com.android.messaging.ui.appsettings.SettingsActivity" />
-        </activity>
-
-        <activity
-            android:name=".ui.appsettings.ApplicationSettingsActivity"
            android:exported="true"


Technically, this is removing the exported Activity .ui.appsettings.ApplicationSettingsActivity and just using .ui.appsettings.SettingsActivity now.

Since .ui.appsettings.ApplicationSettingsActivity used to be exported, for completeness, maybe an activity-alias could be added here for .ui.appsettings.ApplicationSettingsActivity with android:targetActivity=".ui.appsettings.SettingsActivity">

I will take a look, thanks

stringres added 2 commits March 30, 2026 01:20

Update verification-metadata.xml

64446ef

Add settings activity

3517938

m4pl force-pushed the appsettings-compose branch 2 times, most recently from 38013d9 to 05688c9 Compare April 3, 2026 22:40

RankoR reviewed Apr 6, 2026

View reviewed changes

m4pl force-pushed the appsettings-compose branch from 50c7726 to af392d1 Compare April 6, 2026 23:31

stringres added 9 commits April 7, 2026 01:50

Add settings main screen

83d1d10

Add subscription settings screen

f615bf2

Add app settings screen

2e0cd0d

Fix navigation issues

f67d7c2

Add settings tests

c6c978d

Add conversation user flow test

046dd9e

Remove old settings implementation

10fa182

Update ViewModel and Compose for detekt

47800dd

Update ktlint config to prevent collapsing data classes

90cc047

m4pl force-pushed the appsettings-compose branch from af392d1 to 90cc047 Compare April 6, 2026 23:51

inthewaves suggested changes Apr 7, 2026

View reviewed changes

inthewaves reviewed Apr 7, 2026

View reviewed changes

	@Nullable
	@Override
	public CharSequence getSummary() {
	// Show the preference value if it's set, or the default number if not.
	// If we don't have a default, fall back to a static string (e.g. Unknown).
	String value = getText();
	if (TextUtils.isEmpty(value)) {
	value = mDefaultPhoneNumber;
	}
	final String displayValue = (!TextUtils.isEmpty(value))
	? PhoneUtils.get(mSubId).formatForDisplay(value)
	: getContext().getString(R.string.unknown_phone_number_pref_display_value);
	final BidiFormatter bidiFormatter = BidiFormatter.getInstance();
	return bidiFormatter.unicodeWrap
	(displayValue, TextDirectionHeuristicsCompat.LTR);
	}

	items(subscriptions) { subscription ->
	items(subscriptions, key = { it.subId }) { subscription ->

Conversation

m4pl commented Apr 3, 2026

Screenshots

Figma

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inthewaves Apr 7, 2026 • 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!

inthewaves Apr 7, 2026 • 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!

inthewaves Apr 7, 2026 • 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!

inthewaves Apr 7, 2026 • 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!

inthewaves Apr 7, 2026 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

inthewaves Apr 7, 2026 •

edited

Loading

inthewaves Apr 7, 2026 •

edited

Loading

inthewaves Apr 7, 2026 •

edited

Loading

inthewaves Apr 7, 2026 •

edited

Loading

inthewaves Apr 7, 2026 •

edited

Loading