davx5-ose icon indicating copy to clipboard operation
davx5-ose copied to clipboard

[Meta] Migrate to Single-Activity Compose Navigation architecture

Open ArnyminerZ opened this issue 1 year ago • 6 comments

Right now we are using the multi-activity architecture that was the standard for a long time, but it looks like everything is moving to a single-activity with navigation (at least it's how it's explained in the docs).

In the migration guide, the first thing they recommend is to "Move screen-specific UI logic out of activities", which we have already done pretty much everywhere, so this is completed. The next step would be of course, start getting rid of the activities in order to migrate into a single-activity.

I don't think this process should be done in a single movement, but maybe from less to more complex. First move activities that don't have any intent-filters, and are directly related to each other. For example, AboutActivity can be merged with AccountsActivity, and the same goes for IntroActivity.

PermissionsActivity is for example a complex one, because it's related to multiple other activities, so I'd wait to migrate it.

I'd split it into the following steps.

The idea is obviously at the end of the process, AccountsActivity stops existing and we have a proper MainActivity, that holds the main navgraph, and redirects accordingly to the request made.


First Phase (no cross-dependencies)

I'll put first the activity that will hold the other ones, and below the ones that would be merged into it.

  • AccountsActivity
    • IntroActivity
    • AboutActivity
  • AppSettingsActivity
    • TasksActivity
  • AccountActivity
    • CollectionActivity
    • CreateAddressBookActivity
    • CreateCalendarActivity
  • WebdavMountsActivity
    • AddWebdavMountActivity

Second Phase (AppSettingsActivity)

This activity is defined as parent for DebugInfoActivity, PermissionsActivity and TasksActivity.

TasksActivity

Should already be merged into the Activity by this point, so no worries here.

DebugInfoActivity

It has IntentBuilder defined inside, so this would have to be moved into AppSettingsActivity. All the extras can be moved into the companion of AppSettingsActivity, and I'd probably create a new action named something like at.bitfire.davdroid.ui.debug_info, so that if AppSettingsActivity is launched with this action, it will be automatically redirected to the destination of the "old" DebugInfoActivity passing all the extras.

Then, all the functions in DebugInfoActivity would be moved into a viewmodel (shareZipFile, shareFile, viewFile).

All usages of DebugInfoActivity and DebugInfoActivity.IntentBuilder would of course be moved to AppSettingsActivity.

PermissionsActivity

It is used in multiple places, but doesn't have any extras, so we would only have to add an action, for example at.bitfire.davdroid.ui.permissions, which if given to AppSettingsActivity sets the navgraph location to the contents of PermissionsScreen.

ArnyminerZ avatar Dec 13 '24 11:12 ArnyminerZ

I don't think this process should be done in a single movement, but maybe from less to more complex. First move activities that don't have any intent-filters, and are directly related to each other. For example, AboutActivity can be merged with AccountsActivity, and the same goes for IntroActivity.

Good plan, but I think it shouldn't be named AccountsActivity then. At the end of every phase, every activity, screen etc. should have a correct name. For instance, if we can't continue right after the first phase and then come back in a few months, there will be a a confusing naming scheme of the activity in the meanwhile which makes it hard to fix bugs etc.

So I'm for a new MainActivity and then migrate the activities one-by-one.

We must however pay extra attention to activities/intents which can be launched from outside. For instance the login Intent or the Account intent. And of course all the screens that take arguments from a PendingIntent / notification.

rfc2822 avatar Dec 16 '24 12:12 rfc2822

We could try starting this mega-issue 😅

I suggest to

  • start with a new issue + PR that converts one activity (AccountsActivity? or something more easy, perhaps without arguments) to MainActivity, and as soon as this PR is merged,
  • remove the Activities one by one (1 PR per activity/screen).

Then we can also work on multiple activities in parallel. What do you think @ArnyminerZ @sunkup?

@ArnyminerZ Would you like to start with the MainActivity PR?

rfc2822 avatar Jan 01 '25 12:01 rfc2822

I suggest to

  • start with a new issue + PR that converts one activity (AccountsActivity? or something more easy, perhaps without arguments) to MainActivity, and as soon as this PR is merged,
  • remove the Activities one by one (1 PR per activity/screen).

Then we can also work on multiple activities in parallel. What do you think @ArnyminerZ @sunkup?

Sounds good to me.

We must however pay extra attention to activities/intents which can be launched from outside. For instance the login Intent or the Account intent. And of course all the screens that take arguments from a PendingIntent / notification.

I take it that apps which start certain activities (Etar, JtxBoard, Tasks.org) might have to make changes to how they send users to certain parts of the DAVx5 app. Or would the reference, default to the new MainActivity? Otherwise we could notify the app developers about the breaking changes?

sunkup avatar Jan 02 '25 09:01 sunkup

I take it that apps which start certain activities (Etar, JtxBoard, Tasks.org) might have to make changes to how they send users to certain parts of the DAVx5 app. Or would the reference, default to the new MainActivity? Otherwise we could notify the app developers about the breaking changes?

Yes, I'd prefer to

  • provide a legacy wrapper that allows the old entry points / activites to be called and redirect to the new MainActivity somehow,
  • change the documentation to the new way and notify related apps about it
  • remove the legacy wrapper at some time in the future when everyone has switched.

rfc2822 avatar Jan 02 '25 09:01 rfc2822

List of known references, to check they don't break:

  • Nextcloud app ("sync contacts/calendars with DAVx5")
  • jtxBoard (has refernences in this SyncUtil file)
    • AccountsActivity: Drawermenu > Collections > 3-dotmenu > Add remote collection (DAVx5)
      • adb shell am start -n at.bitfire.davdroid/at.bitfire.davdroid.ui.AccountsActivity
    • AccountActivity: Drawermenu > Synchronization > Go to collections
    • ...
  • tasks.org
    • ...

sunkup avatar Jan 03 '25 09:01 sunkup

We should probably wait for Nav3.

rfc2822 avatar May 21 '25 13:05 rfc2822