Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: app navigation - compose destinations [RC] [WPB-360] #2005

Conversation

saleniuk
Copy link
Contributor


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

Our navigation is complex, not easy to understand how things work and to make any additional changes, and it’s very difficult to find possible errors. This is also because of the limitations of the standard compose navigation, which works using routes that are intended to be url-like, so there is no type-safety and passing arguments backwards is complicated.

Solutions

We refactored the navigation to use the Compose Destinations library, which is an overlay on the Official Jetpack Compose Navigation, which allows us to hide the complex, boilerplate code and use type-safe parameters and make the whole navigation graph and process easier and more readable.

Testing

How to Test

The whole navigation in the app should work as before (or even better).


PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

saleniuk and others added 30 commits June 5, 2023 17:41
…n_refactor_define_destinations

# Conflicts:
#	app/src/main/kotlin/com/wire/android/ui/home/conversations/details/editselfdeletingmessages/EditSelfDeletingMessagesViewModel.kt
#	app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt
…n_refactor_define_destinations

# Conflicts:
#	app/src/main/kotlin/com/wire/android/ui/debug/dev/DevDebugViewModel.kt
#	app/src/main/kotlin/com/wire/android/ui/home/settings/SettingsItem.kt
#	app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt
… [WPB-360] (#1856)

* refactor: current screen observers to use Routes instead of NavigationItems

* remove unused function

* fixes after merge

* fixed detekt issues

* fixed detekt issues

* fixed detekt issues

* fix tests

* fix detekt issues
* refactor: navigate back with args

* chore: kalium reference

* chore: kalium reference

* chore: address comments

* chore: cleanup

* chore: detekt

* chore: detekt

* chore: fix broken unit test

* chore: address comment

* chore: address comments
* refactor: navigate back with args

* chore: kalium reference

* chore: kalium reference

* chore: address comments

* chore: cleanup

* chore: detekt

* chore: detekt

* chore: fix broken unit test

* chore: address comment

* chore: fix broken unit test

* chore: cleanup

* chore: cleanup

* chore: cleanup

* chore: fix wrong navigation

* chore: fix broken unit test

* chore: detekt

* chore: fix broken unit test

* fix: exclude build sub folders

* chore: detekt

* chore: detekt

* chore: detekt

* chore(detekt): exclude build folder

* chore: address comments

* chore: remove unused imports

* chore: detekt config
…#1863)

* refactor: replace NavigationManager with Navigator in AccountSwitchUseCase, WireActivity and WelcomeScreen

* remove unused imports

* detekt issues

* changes after review
…#1866)

* refactor: replace NavigationManager with Navigator in AccountSwitchUseCase, WireActivity and WelcomeScreen

* remove unused imports

* detekt issues

* changes after review

* refactor: replace NavigationManager with Navigator

* removed unused code

* removed unused code

* refactor: replace NavigationManager with Navigator

* refactor: replace NavigationManager with Navigator

* detekt issues

* fix tests

* clean up code

* fix detekt
* refactor: navigate back with args

* chore: kalium reference

* chore: kalium reference

* chore: address comments

* chore: cleanup

* chore: detekt

* chore: detekt

* chore: fix broken unit test

* chore: address comment

* chore: fix broken unit test

* chore: cleanup

* chore: cleanup

* chore: cleanup

* chore: fix wrong navigation

* chore: fix broken unit test

* chore: detekt

* chore: fix broken unit test

* fix: exclude build sub folders

* chore: detekt

* chore: detekt

* chore: detekt

* chore(detekt): exclude build folder

* chore: address comments

* chore: remove unused imports

* chore: detekt config

* refactor: create nestedGraph for account creation

* refactor: cleanup

* chore: cleanup

* chore: clean up

* chore: resolve conflicts
…#1883)

* refactor: replace NavigationManager with Navigator in AccountSwitchUseCase, WireActivity and WelcomeScreen

* remove unused imports

* detekt issues

* changes after review

* refactor: replace NavigationManager with Navigator

* removed unused code

* removed unused code

* refactor: replace NavigationManager with Navigator

* refactor: replace NavigationManager with Navigator

* detekt issues

* fix tests

* clean up code

* fix detekt

* refactor: replace NavigationManager with Navigator

* removed unused imports

* change lambdas in inits into states

* detekt and tests fixes

* fix detekt

* change after review

* trigger build
…#1887)

* refactor: replace NavigationManager with Navigator in AccountSwitchUseCase, WireActivity and WelcomeScreen

* remove unused imports

* detekt issues

* changes after review

* refactor: replace NavigationManager with Navigator

* removed unused code

* removed unused code

* refactor: replace NavigationManager with Navigator

* refactor: replace NavigationManager with Navigator

* detekt issues

* fix tests

* clean up code

* fix detekt

* refactor: replace NavigationManager with Navigator

* removed unused imports

* change lambdas in inits into states

* detekt and tests fixes

* fix detekt

* refactor: replace NavigationManager with Navigator

* remove unused imports

* change after review

* trigger build

* fix detekt issues

* fix tests
…PB-384] (#1889)

* refactor: navigate back with args

* chore: kalium reference

* chore: kalium reference

* chore: address comments

* chore: cleanup

* chore: detekt

* chore: detekt

* chore: fix broken unit test

* chore: address comment

* chore: fix broken unit test

* chore: cleanup

* chore: cleanup

* chore: cleanup

* chore: fix wrong navigation

* chore: fix broken unit test

* chore: detekt

* chore: fix broken unit test

* fix: exclude build sub folders

* chore: detekt

* chore: detekt

* chore: detekt

* chore(detekt): exclude build folder

* chore: address comments

* chore: remove unused imports

* chore: detekt config

* refactor: replace NavigationManager with Navigator in AccountSwitchUseCase, WireActivity and WelcomeScreen

* remove unused imports

* detekt issues

* refactor: create nestedGraph for account creation

* refactor: cleanup

* chore: cleanup

* changes after review

* refactor: replace NavigationManager with Navigator

* removed unused code

* removed unused code

* refactor: replace NavigationManager with Navigator

* refactor: replace NavigationManager with Navigator

* detekt issues

* fix tests

* clean up code

* fix detekt

* refactor: replace NavigationManager with Navigator

* removed unused imports

* chore: clean up

* chore: resolve conflicts

* change lambdas in inits into states

* detekt and tests fixes

* fix detekt

* refactor: replace NavigationManager with Navigator

* remove unused imports

* change opening incoming call screen to rely on full screen intent, fix screen modes

* fixes after code analysis

* replace NavigationManager with Navigator on account creation screens

* remove unused imports

* fix detekt issues

---------

Co-authored-by: Oussama <[email protected]>
* refactor nested navhosts on login and new conversation flows

* remove unused imports

* uncomment code, add description

* fix detekt
* refactor: nested navhost on home screen

* remove unused imports

* fix detekt

* change ExternalDirection to be more direct
…n_refactor_define_destinations

# Conflicts:
#	app/build.gradle.kts
#	app/src/main/kotlin/com/wire/android/ui/home/settings/account/MyAccountScreen.kt
#	app/src/main/kotlin/com/wire/android/ui/home/settings/account/MyAccountViewModel.kt
…n_refactor_define_destinations

# Conflicts:
#	app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallViewModel.kt
#	app/src/main/kotlin/com/wire/android/ui/home/settings/account/MyAccountViewModel.kt
#	app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt
#	app/src/test/kotlin/com/wire/android/ui/home/settings/account/MyAccountViewModelTest.kt
#	app/src/test/kotlin/com/wire/android/ui/home/sync/FeatureFlagNotificationViewModelTest.kt
@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2023

Test Results

500 tests   499 ✔️  5m 10s ⏱️
  75 suites      1 💤
  75 files        0

Results for commit 30fee10.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 662 succeeded.

The build produced the following APK's:

@github-actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 715 succeeded.

The build produced the following APK's:

@AndroidBob
Copy link
Collaborator

Build 760 succeeded.

The build produced the following APK's:

@github-actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 785 succeeded.

The build produced the following APK's:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 808 succeeded.

The build produced the following APK's:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 836 succeeded.

The build produced the following APK's:

@saleniuk
Copy link
Contributor Author

Closing it as we decided not to merge it to RC 4.3.x, only to develop 4.4.x

@saleniuk saleniuk closed this Aug 10, 2023
@saleniuk saleniuk deleted the refactor/navigation_refactor_define_destinations_RC branch January 23, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants