Migrate Help Fragment to Compose by rsrsaa · Pull Request #14756 · signalapp/Signal-Android

rsrsaa · 2026-05-05T02:05:40Z

First time contributor checklist

I have read how to contribute to this project
I have signed the Contributor License Agreement

Contributor checklist

I am following the Code Style Guidelines
I have tested my contribution on these devices:

Google Pixel 7, Android 16
Google Pixel 4a, Android 13

My contribution is fully baked and ready to be merged as is

Description

Migrated HelpFragment screen to Compose & Kotlin.

Tested on playStagingDebug build variant.
Also checked compose screen in layout inspector and for overdraw.

Screenshots before/after attached.

Before	After

Before	After

greyson-signal · 2026-05-05T15:04:13Z

Hi there, thank you for your contribution, but I don't really want to take in things like full screen conversions.

rsrsaa · 2026-05-05T15:40:05Z

@greyson-signal Hi, but this screen is pretty small one, I can add more details on what was done, on testing, etc.
I also asked on forum whether this PR is good contribution and @alex-signal said go ahead.

rsrsaa · 2026-05-05T15:42:40Z

@greyson-signal Also, I’m available ASAP if any bugs occur

alex-signal

This is the right direction but we've adopted a Component(state, (event) -> Unit) pattern that we want to formalize throughout. Also concerned with some of the window adjustments, since this fragment will be shown in a split-pane in the near future.

alex-signal · 2026-05-07T13:58:32Z

    override fun onContactSupport() {
      requireActivity().finish()
-      requireActivity().startActivity(AppSettingsActivity.help(requireContext(), HelpFragment.REMOTE_BACKUPS_INDEX))
+      requireActivity().startActivity(AppSettingsActivity.help(requireContext(), REMOTE_BACKUPS_INDEX))


This can be reverted, we don't directly import statics (or at least try not to)

alex-signal · 2026-05-07T13:59:39Z

+
+  @Composable
+  override fun FragmentContent() {
+    val startCategoryIndex = arguments?.getInt(START_CATEGORY_INDEX, 0) ?: 6


6 magic number. Should this be PAYMENT_INDEX?

Nice catch, thank you, fixed

alex-signal · 2026-05-07T14:00:41Z

+      onFaqClick = {
+        val intent = Intent(Intent.ACTION_VIEW)
+        intent.setData(Uri.parse(getString(R.string.HelpFragment__link__faq)))
+        startActivity(intent)


CommunicationActions.openBrowserLink

alex-signal · 2026-05-07T14:00:45Z

+      onNavigationClick = { requireActivity().onBackPressedDispatcher.onBackPressed() },
+      onWhatIsDebugLogClick = {
+        val intent = Intent(Intent.ACTION_VIEW)
+        intent.setData(Uri.parse(getString(R.string.HelpFragment__link__debug_info)))


CommunicationActions.openBrowserLink

alex-signal · 2026-05-07T14:01:32Z

+
+class HelpViewModel(application: Application) : AndroidViewModel(application) {
+
+  private val _state = MutableStateFlow(HelpScreenState())


internalState

alex-signal · 2026-05-07T14:09:11Z

+
+  DisposableEffect(Unit) {
+    activity?.window?.let {
+      WindowCompat.setDecorFitsSystemWindows(it, false)


This doesn't feel right at all. You should be using the insets modifiers if you need to space things correctly. Scaffolds.Settings should already be doing that for you.

Totally agree with you, we don't need this, removed.

alex-signal · 2026-05-07T14:10:04Z

+        .fillMaxSize()
+        .padding(paddingValues),
+    ) {
+      Column(


LazyColumn is likely a better and more performant choice.

Maybe it is OK to leave Column here?
Pros:

content is fixed and small

it has .verticalScroll()

Cons:

don't see for now :)

alex-signal · 2026-05-07T14:11:05Z

+  val snackbarHostState = remember { SnackbarHostState() }
+
+  LaunchedEffect(startCategoryIndex) {
+    viewModel.onCategorySelected(startCategoryIndex)


This should just be passed to the ViewModel at construction time?

alex-signal · 2026-05-07T14:11:53Z

+
+package org.thoughtcrime.securesms.help
+
+sealed interface HelpScreenEvents {


*Events is reserved for user-initiated actions like clicks and toggles.

alex-signal · 2026-05-07T14:13:51Z

+    }
+
+    val subject = context.getString(R.string.HelpFragment__signal_android_support_request)
+    val body    = SupportEmailUtil.generateSupportEmailBody(


You'll want to run the formatter with ./gradlew format

…ents class for user interaction

rsrsaa · 2026-05-13T01:10:53Z

@alex-signal thank you for your time and comments!

I’ve resolved all comments and I have a few to discuss.
I’m open to discuss it and if needed make new changes/refactoring to comply with your architecture and project vision :)

We can’t have HelpScreenRepository with sendEmail() fun to call from VM because we need activity context to open email app.

So I made SideEffects flow which VM can use to send event and Compose screen can address it.

Also I left ShowSnackbar in the same SideEffects flow because

it more belongs to UI/Compose than VM
anyway we have this sideEffect flow
it is more natural approach with snackbar handling in Compose I think.

Events was refactored to user interaction events

So what do you think on having SideEffect flow in VM to communicate from VM to Compose screen?

Can we leave Column as root compose component in Scaffold.Settings because screen is pretty small and content is fixed?

I’ve added onEvent() param in Compose function, but left navigation back and open browsers clicks.

@Composable
fun HelpScreenContent(
  state: HelpScreenState,
  onEvent: (HelpScreenEvents) -> Unit,
  sideEffect: Flow<HelpScreenSideEffects>,
  onNavigationClick: () -> Unit,
  onWhatIsDebugLogClick: () -> Unit,
  onFaqClick: () -> Unit,
) {

rsrsaa added 2 commits May 5, 2026 03:45

Migrate HelpFragment to Compose

7c3acf7

Clean up code, remove old HelpFragment

477ed4d

greyson-signal closed this May 5, 2026

alex-signal reopened this May 7, 2026

alex-signal requested changes May 7, 2026

View reviewed changes

rsrsaa added 8 commits May 7, 2026 17:19

Resolve PR comments

1684cf4

Fix internal state & events variable names

d21eb8c

Fix code formatting

f419d2c

Rename HelpScreenEvents class to HelpScreenSideEffects class, keep Ev…

2819339

…ents class for user interaction

Pass startCategoryIndex argument directly to VM constructor

8ca3052

Refactor help screen events

72189f7

Fix HelpScreenSideEffects name

a7bb041

Clean up code

f7b81e9

rsrsaa requested a review from alex-signal May 13, 2026 01:11

Format code

2d8f89a


		class HelpViewModel(application: Application) : AndroidViewModel(application) {

		private val _state = MutableStateFlow(HelpScreenState())


		package org.thoughtcrime.securesms.help

		sealed interface HelpScreenEvents {

Uh oh!

Conversation

rsrsaa commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

First time contributor checklist

Contributor checklist

Description

Uh oh!

greyson-signal commented May 5, 2026

Uh oh!

rsrsaa commented May 5, 2026

Uh oh!

rsrsaa commented May 5, 2026

Uh oh!

alex-signal 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!

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!

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!

rsrsaa commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rsrsaa commented May 5, 2026 •

edited

Loading

rsrsaa commented May 13, 2026 •

edited

Loading