Skip to content

Conversation

wb9688
Copy link
Contributor

@wb9688 wb9688 commented May 31, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Use Android's view binding instead of findViewById().

I'd appreciate anyone's help. Btw I still need to figure out how to port things using a ViewPager over, so don't work on those yet.

Why?

This results in cleaner code imho. This also gives us the opportunity to easily fix all memory leaks of the first category in #3517.

How could I port a fragment to use view binding?

  1. Replace the inflater in onCreateView() with binding = Binding.inflate(inflater, container, false);, where Binding should be e.g. FragmentChannelBinding when the layout is called fragment_channel.xml. Of course also add binding as a class member, Then return binding.getRoot(); in onCreateView().
  2. If there are includes in the layout's XML, make sure to give the include the same ID as the layout it includes, otherwise it won't be possible to use the included layout.
  3. Remove all the views that we have as class members and usually assign in onViewCreated(). Change all accesses to that class member to binding.whatever, where whatever is the camel case version of the ID in the layout.
  4. Make sure to assign binding to null in onDestroyView() (not onDestroy(), you'll probably see a lot of cases in our app where things are done in onDestroy() while they should be in onDestroyView()), otherwise you'll cause a memory leak.
  5. Do the same thing for eventual other inflaters.

How could I port an activity to use view binding?

  1. Replace the inflater in onCreate() with binding = Binding.inflate(inflater);, where Binding should be e.g. FragmentChannelBinding when the layout is called fragment_channel.xml. Of course also add binding as a class member, Then call setContentView(binding.getRoot()); in onCreate().
  2. If there are includes in the layout's XML, make sure to give the include the same ID as the layout it includes, otherwise it won't be possible to use the included layout.
  3. Remove all the views that we have as class members. Change all accesses to that class member to binding.whatever, where whatever is the camel case version of the ID in the layout.
  4. Do the same thing for eventual other inflaters.

How could I verify that it works correctly?

In Android's settings, enable Developer settings -> Don't keep activities. Then open the fragment/activity in NewPipe, switch to another app, switch back to NewPipe and verify that it doesn't crash or cause weird behavior. Don't forget to turn off that setting once you're finished with working on this PR, as it's pretty annoying to have that enabled in everyday use.

In case of a fragment, you should also verify that going to another fragment in NewPipe from there and then going back works correctly.

To do

Here should be a list of all fragments/activities that we need to port to use view binding. When you start working on one, please add your name behind the fragment/activity. When you've finished, please check the checkmark.

  • ChannelFragment @wb9688
  • SearchFragment @Stypox
  • MainActivity @wb9688
  • AboutActivity.java
  • LicenseFragment.java
  • DownloadActivity.java
  • DownloadDialog.java
  • VideoDetailFragment.java
  • PlaylistFragment.java
  • SuggestionListAdapter.java
  • RelatedVideosFragment.java
  • BaseListFragment.java
  • BaseStateFragment.java
  • MainFragment.java
  • ChannelMiniInfoItemHolder.java
  • CommentsInfoItemHolder.java
  • PlaylistMiniInfoItemHolder.java
  • CommentsMiniInfoItemHolder.java
  • StreamInfoItemHolder.java
  • StreamMiniInfoItemHolder.java
  • ChannelInfoItemHolder.java
  • InfoItemDialog.java
  • BookmarkFragment.java
  • PlaylistAppendDialog.java
  • PlaylistCreationDialog.java
  • StatisticsPlaylistFragment.java
  • PlaylistItemHolder.java
  • LocalPlaylistStreamItemHolder.java
  • LocalStatisticStreamItemHolder.java
  • LocalPlaylistFragment.java
  • SubscriptionsImportFragment.java
  • FeedImportExportItem.kt
  • BaseLocalListFragment.java
  • CustomBottomSheetBehavior.java
  • PlaybackParameterDialog.java
  • PlayQueueItemHolder.java
  • ServicePlayerActivity.java
  • VideoPlayer.java
  • VideoPlayerImpl.java
  • ErrorActivity.java
  • AddTabDialog.java
  • ChooseTabsFragment.java
  • SelectKioskFragment.java
  • SettingsActivity.java
  • NotificationSettingsFragment.java
  • SelectChannelFragment.java
  • SelectPlaylistFragment.java
  • PeertubeInstanceListFragment.java
  • PermissionHelper.java
  • FilePickerActivityHelper.java
  • StreamItemAdapter.java
  • FocusAwareCoordinator.java
  • ReCaptchaActivity.java
  • RouterActivity.java

Agreement

@wb9688 wb9688 closed this Oct 24, 2020
@Stypox Stypox deleted the view-binding branch June 24, 2021 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants