Android
Android copied to clipboard
duckduckgo
ViewBinding migration
Task/Issue URL: https://github.com/duckduckgo/Android/issues/1478
Description
I have migrated most of the files from findViewById and kotlinExtenstions including Activities,Fragments,Dialog Fragments,Adapters. I have used the Kotlin-delegates in all the components to make code more readable and structured. I have also added a delegate for the "Dialog Fragments" because dialog fragment has a different lifecycle compared to "fragment" due to overriding of onCreateDialog.
Remaining Fragments and components
I would love to work on these also, but for this, I would need approval from the internal team. because the intro page is coupled with the base fragment, which is not very good practice, we can use the extension function for that, I know "browserTabFragment " is assigned to the internal team , but I would like to work on that including all other remaining components.
Hi @cmonfortep! please review this PR (ViewBinding migration) when you have time. Thanks.
@AndroidPoet will take a look thanks. Just to set expectations, I'm planning to review it beginning next week.
@AndroidPoet will take a look thanks. Just to set expectations, I'm planning to review it beginning next week. Thanks!
@AndroidPoet I was doing a quick pass and I see many unrelated changes (e.g: code formating, new lines, code commented, etc.), can you discard them? You can run as well spotless to fix any code style issues (https://github.com/duckduckgo/Android/blob/develop/STYLEGUIDE.md#code-formatting)
Also, our goal is to migrate away from kotlin synthetics since they are deprecated. Usage of findViewById or binding is allowed, up to each developer to follow their preference on each case. We don't have a strong rule to follow there. Could you discard changes on those classes that were already migrated?
Thanks!
Thanks for the review.@cmonfortep, I will discard formatting and all these unnecessary things. I agree that we don't have to follow that strong rule, we can use findViewId and view binding together and remove kotlin synthetics. But In my opinion, code always should be uniform and follow the same structure on all the screens, It creates a pattern in our mind so we can easily identify things. But will use the same pattern which is View Binding for kotlin synthetics screens and will not change if there is already findViewId used.
No worries. That will helps us have a smaller PR easier for me to review, otherwise, there are a lot more things to check here.
Yes i was also thinking that we can divide screens and will work on that,so we can fix this step by step. will upload screenshot a list of screen which still have kotlin synthetic.
@AndroidPoet I know you are busy with some interviews (I hope you do well :)).
If we are not actively working on this contribution, are you ok if we close it? You can reopen it again once it's ready for review. Thanks.
@AndroidPoet I know you are busy with some interviews (I hope you do well :)).
If we are not actively working on this contribution, are you ok if we close it? You can reopen it again once it's ready for review. Thanks.
Thanks! Yeah Sure will reopen it .
@cmonfortep assigning to you @AndroidPoet is this still active? can it be closed?
