Android icon indicating copy to clipboard operation
Android copied to clipboard

ViewBinding migration (replacing kotlin synthetics)

Open cmonfortep opened this issue 4 years ago • 16 comments

Description

We are currently migrating to ViewBinding since Kotlin synthetics have been deprecated.

This is a great opportunity for contributors to help our Android team.

We are accepting contributions to remove kotlin synthetics usages and replace them by using ViewBinding.

Inside our codebase, we have already migrated a bunch of classes that you can use as a reference:

  • Fragments: InitialFeedbackFragment, ShareOpenEndedFeedbackFragment, etc.
  • Dialogs: DaxDialog
  • RecyclerView.ViewHolder: AppIconsAdapter,SearchSuggestionViewHolder, BookmarkSuggestionViewHolder,
  • CustomView: TabSwitcherButton

How to work on this issue

But there are still some pending migrations.

  • Fragments: EmailAutofillTooltipFragment, SavedSiteDialogFragment, DefaultBrowserPage, WelcomePage, BrowserTabFragment, DownloadConfirmationFragment
  • Dialogs: FireDialog
  • Adapaters: BookmarksAdapter, FavoritesAdapter, BookmarkFolersAdapter, FavoritesQuickAccessAdapter, etc...
  • CustomView: TrackerNetworkLeaderboardPillView

Just leave us a comment that you would like to work on the issue and what classes you will migrate. That will avoid conflicts with other contributions migrating the same classes.

Thanks!

cmonfortep avatar Oct 11 '21 14:10 cmonfortep

I would like to help with all the fragment classes. Let me know!

droid-it avatar Oct 12 '21 03:10 droid-it

@cmonfortep I can take up Dialogs & Adapters

mslalith avatar Oct 12 '21 03:10 mslalith

CAN I WORK ON THIS ?

Rishavgupta12345 avatar Oct 12 '21 06:10 Rishavgupta12345

@droid-it sounds great

cmonfortep avatar Oct 12 '21 07:10 cmonfortep

@cmonfortep I'll start with it then, thank you! Please assign it to me as well..

droid-it avatar Oct 12 '21 08:10 droid-it

@droid-it done. Please use as a reference some of our migrate fragments. https://github.com/duckduckgo/Android/pull/1329 we introduced a fragment viewbinding delegate to make viewbinding in fragments easier.

cmonfortep avatar Oct 12 '21 09:10 cmonfortep

@mslalith I've just noticed we still have some adapters and dialogs using kotlin synthetics: FireproofWebsitesAdapter, LocationPermissionsAdapter, and FireDialog. Could you submit a PR to migrate those pending classes?

cmonfortep avatar Oct 12 '21 16:10 cmonfortep

@cmonfortep okay

mslalith avatar Oct 13 '21 01:10 mslalith

Is it safe to say that this issue has been resolved ?

kolanse avatar Nov 19 '21 11:11 kolanse

Do you still need help here?

simonesummo avatar Nov 29 '21 10:11 simonesummo

Hi, I would like to work on this, there are a lot of findviewbyid files+kotlin extenstions. I would like to replace those with viewbinding.

AndroidPoet avatar Jun 03 '22 02:06 AndroidPoet

I would like to migrate all the remaining classes.

AndroidPoet avatar Jun 03 '22 06:06 AndroidPoet

i have started working on classes if someone is already working on something please let me know

AndroidPoet avatar Jun 03 '22 06:06 AndroidPoet

You can work on any of the ones we haven't yet migrated (except browserTabFragment which we will refactor internally since it's too complex)

cmonfortep avatar Jun 03 '22 11:06 cmonfortep

Thanks! i was bit confused about the two approaches used in the app. Getter property 171803609-14689af6-f990-4800-9ea8-ba09b6802798 kotlin delegate 171803416-78ab227c-87c5-480b-baa9-01da4c9acdcc which one is recommended?

AndroidPoet avatar Jun 03 '22 11:06 AndroidPoet

I would also also like to work on this issue , please let me know which classes i can work with i can take up EmailAutofillTooltipFragment and SavedSiteDialogFragment fragments

GauravGupta17 avatar Jun 19 '22 13:06 GauravGupta17

We will continue the migration internally. Closing the issue.

cmonfortep avatar Dec 16 '22 12:12 cmonfortep