-
Notifications
You must be signed in to change notification settings - Fork 380
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
Feature: Use jetpack navigation component #1088
Comments
Would like to work on this, once made available. |
Not sure if you can claim issues before they are made available. |
Can i work on this issue |
@isabelcosta since it's a possible future feature can it be assigned now or should we wait? |
@vj-codes what was your thought behind possible future feature? |
@isabelcosta what I meant was the navigation currently is alright for the MVP, we do have some features like forgot password, feedback email, resend email for verification, upload profile, improving ui, passing the tests that are necessary as compared to jetpack navigation. Another reason is the PR for this if merged will change the code structure a lot causing merge conflicts in all other PRs which were ready to merge and if we think about creating a PR and not reviewing now due to priority, but if there's some changes required weeks later I can't guarantee that the contributor assigned is available to solve them which will lead to closing it anyway. |
I agree with @vj-codes although I do want to point out that if we want to add more tests to android then jetpack navigation is easier to test. |
Hi, is this available now? I'd like to work on it if possible. |
Marking available as discussed yesterday. |
@DhaneshShetty @kartikeysaran would you like to work on this? |
if they don't respond I'll assign this to you. |
@epicadk my schedule is packed up right now, I won't get much time to fix this, feel free to assign @DhaneshShetty or @unc0ded :) |
Oops seems like I tagged you by mistake sorry. |
it's fine, no problem 🙂 |
@unc0ded assigning you |
@unc0ded , we just discussed in the weekly meeting and @epicadk brought up this issue. I think you could start working on this with regards to the bottom navigation. I still don't know what this entails, but would be good to see a PR up (in case you want to try this out), and see how complex this is, and if people contributing to this repository could learn and adjust to starting using this feature. |
Hi, I have started working on it, one of the main challenges to implementing this fully is the sheer number of activities (that could possibly be better suited as fragments). Navigation component works better in the single activity - multiple fragment architecture that google is also trying to push. Maybe converting some of the activities to fragments can be considered as a separate issue, to improve the architecture. |
Yes this is what we had discussed in the open session as well. We can create separate issues |
Is your feature request related to a problem? Please describe.
Currently we have a custom implementation for navigation.
Describe the solution you'd like
Use jetpack Navigation component.
Also initialize the viewmodels from the NavController backstack.
Additional context
It is a very well documented library and using it would also reduce bugs due to navigation in our code.
The text was updated successfully, but these errors were encountered: