nextcloud / talk-android

📱😀 Video & audio calls through Nextcloud on Android
Other
530 stars 238 forks source link

get rid of Conductor #1551

Closed mahibi closed 10 months ago

mahibi commented 3 years ago

We want to switch to native Android solutions instead to use Conductor.

TODOS:

nickvergessen commented 2 years ago

Moved to "To do" to perform the "Research how to avoid a big bang switch" Please let us know if it can be avoided somehow

mahibi commented 2 years ago

well the picture in picture feature actually made a start to replace a controller from conductor with a native activity. This was possible because there was a controller that was on the top of the stack, which could be replaced easily. However other controllers might be more difficult to replace because they are not always on top of the stack or there is more interaction with other controllers. So some research here still totally makes sense..

AndyScherzinger commented 2 years ago

Given @mahibi's remarks I think it is also totally fine if there is no way to easily migrate all conductors but if there is a way to split the work like with the PIP then this would be a valid and helpful approach, so there can be valid, creative approaches. The goal would simply be to find a way to not have to do big bang even when it is a exotic approach :)

tobiasKaminsky commented 2 years ago

If you need any help, ping me :)

mahibi commented 2 years ago

The goal would simply be to find a way to not have to do big bang even when it is a exotic approach :)

an example for one "exotic" approach: In https://github.com/nextcloud/talk-android/pull/1285 i created a BottomSheetDialog that defined another conductor router: https://github.com/nextcloud/talk-android/blob/master/app/src/main/java/com/nextcloud/talk/ui/dialog/ConversationsListBottomDialog.kt#L274

This was necessary because the BottomSheetDialog just worked differently than the formerly used BottomSheetMenu ( https://github.com/Kennyc1012/BottomSheetMenu )

I introduced the additional router to reuse the existing Controllers for now. So i didn't actually replace Controllers yet, but the router stack was "cut" into pieces with that. Whenever some button in the Dialog should trigger an action in the app, i throw an event like here:

https://github.com/nextcloud/talk-android/blob/master/app/src/main/java/com/nextcloud/talk/controllers/bottomsheet/OperationsMenuController.java#L570

which is catched inside the ConversationsListController...

https://github.com/nextcloud/talk-android/blob/f4cb9578136367f8b85d99834cb3872508a75aa0/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.java#L1072

... which then closes the Dialog. So i guess a next step could be to replace the controllers that are used for the Dialogs, as they are not on top of the main router of the app.

tobiasKaminsky commented 2 years ago

Feels a bit strange that eventbus is needed. If you want, we can have a look together, once you want to use this approach as a blueprint for other migrations.

Same for other migration of entire controller. I had a quick look and I think it should be possible like this

It would then be a migration step by step (controller by controller). If you want to tackle it, just call me.