Anki-Android icon indicating copy to clipboard operation
Anki-Android copied to clipboard
verified publisher icon ankidroid

Add View Binding

Open Jatin887 opened this issue 3 years ago • 45 comments

2025 update: Task List

  • [ ] Audit adapter/ViewHolder usage: https://github.com/ankidroid/Anki-Android/pull/19565#discussion_r2553888502
  • [ ] Remove the merge tag in toolbar.xml
    • it's not needed(we already have a single view as root and we don't append this included layout to a Toolbar view) and it also breaks ViewBinding(Missing required view with ID: com.ichi2.anki.debug:id/toolbar).
  • [x] architecture review: vendor 'vbpd' David Allison 18/09/2025, 14:50
  • [ ] refactor: use BrowserColumnCellBinding David Allison 12/09/2025, 04:53
  • [ ] refactor: remove 'layout' from MultimediaFragment David Allison 12/09/2025, 02:51
  • [ ] chore: convert MultimediaImageFragment to ViewBinding David Allison 12/09/2025, 02:49
  • [ ] chore: convert DeckSelectionDialog to ViewBinding David Allison 12/09/2025, 02:33
  • [ ] chore: convert BrowserColumnSelectionFragment to ViewBinding David Allison 12/09/2025, 02:26
  • [ ] chore: convert FlagAdapter to ViewBinding David Allison 12/09/2025, 02:24
  • [ ] chore: convert RepositionCardFragment to ViewBinding David Allison 12/09/2025, 01:55
  • [ ] chore: convert FindAndReplaceDialogFragment to ViewBinding David Allison 12/09/2025, 01:47
  • [ ] chore: convert ColumnSelectionDialogFragment to ViewBinding David Allison 12/09/2025, 01:42
  • [ ] chore: convert ResizingDivider to ViewBinding David Allison 12/09/2025, 01:35
  • [ ] chore: convert AboutFragment to ViewBinding David Allison 12/09/2025, 01:27
  • [ ] chore: convert AudioRecordView to ViewBinding David Allison 12/09/2025, 01:20
  • [ ] chore: convert MultiColumnViewHolder to ViewBinding David Allison 12/09/2025, 01:16
  • [ ] chore: convert ControlPreferenceDialogFragment to ViewBinding David Allison 12/09/2025, 01:09
  • [ ] chore: convert GesturePicker to ViewBinding David Allison 12/09/2025, 01:06
  • [ ] chore: convert AlertDialogFacade to ViewBinding David Allison 12/09/2025, 01:02
  • [ ] chore: convert TemplatePreviewerPage to ViewBinding David Allison 12/09/2025, 00:29
  • [ ] ! chore: convert SettingsFragment to ViewBinding David Allison 12/09/2025, 00:24
  • [ ] chore: convert KeyPicker to ViewBinding David Allison 12/09/2025, 00:18
  • [x] #19620
  • [ ] chore: convert AudioVideoFragment to ViewBinding David Allison 11/09/2025, 23:57
  • [ ] chore: convert AudioRecordingFragment to ViewBinding David Allison 11/09/2025, 23:48
  • [ ] chore: convert HelpDialog to ViewBinding David Allison 11/09/2025, 23:45
  • [x] #19564
  • [ ] ! chore: convert DeckPickerFloatingActionMenu to ViewBinding David Allison 11/09/2025, 22:55
  • [ ] chore: convert ScheduleRemindersAdapter to ViewBinding David Allison 11/09/2025, 22:39
  • [ ] chore: convert SharedDecksDownloadFragment to ViewBinding David Allison 11/09/2025, 21:00
  • [ ] chore: convert NotetypeAdapter to ViewBinding David Allison 11/09/2025, 20:52
  • [ ] chore: convert TagLimitFragment to ViewBinding David Allison 11/09/2025, 20:47
  • [ ] chore: convert IncludedExcludedTagsAdapter to ViewBinding David Allison 11/09/2025, 20:43
  • [ ] refactor(custom-study): use userInputValue David Allison 11/09/2025, 20:43
  • [ ] chore: convert CustomStudyDialog to ViewBinding David Allison 11/09/2025, 20:37
  • [ ] chore: convert SchedulerReminders to ViewBinding David Allison 11/09/2025, 20:31
  • [ ] chore: convert WhiteboardFragment to ViewBinding David Allison 11/09/2025, 20:20
  • [ ] chore: convert CheckPronunciationFragment to ViewBinding David Allison 11/09/2025, 19:44
  • [ ] chore: convert PreviewerFragment to ViewBinding David Allison 11/09/2025, 19:39
  • [ ] chore: convert ReviewerMenuView to ViewBinding David Allison 11/09/2025, 19:29
  • [ ] chore: convert ReviewerMenuSettingsFragment to ViewBinding David Allison 11/09/2025, 19:26
  • [x] #19621
  • [ ] chore: convert AudioPlayView to ViewBinding David Allison 11/09/2025, 19:18
  • [ ] chore: convert DeckDropDownAdapter to ViewBinding David Allison 11/09/2025, 05:00
  • [ ] chore: convert TagsDialog to ViewBinding David Allison 11/09/2025, 04:46
  • [x] #19566
  • [x] #19637
  • [ ] chore: convert FlagRenameDialog to ViewBinding David Allison 11/09/2025, 04:33
  • [ ] chore: convert SavedBrowserSearchesDialogFragment to ViewBinding David Allison 11/09/2025, 04:29
  • [ ] chore: convert LocaleSelectionDialog to ViewBinding David Allison 11/09/2025, 04:26
  • [x] #19482
  • [x] #19565
  • [x] #19591
  • [ ] chore: convert ExportDialogFragment to ViewBinding David Allison 11/09/2025, 03:51
  • [x] #19608
  • [ ] chore: convert NoteFieldAdapter to ViewBinding David Allison 11/09/2025, 03:19
  • [x] #19708
  • [x] #19593
  • [x] #19707
  • [x] #19706
  • [x] #19705
  • [x] #19568
  • [x] #19704
  • [x] https://github.com/ankidroid/Anki-Android/pull/19496
  • [x] #19567
  • [x] #19483
  • [ ] refactor: remove unused layout from PermissionsFragment David Allison 11/09/2025, 01:08
  • [ ] chore: convert AllPermissionsExplanationFragment to ViewBinding David Allison 12/09/2025, 02:40
  • [ ] chore: convert PermissionsUntil29Fragment to ViewBinding David Allison 11/09/2025, 01:07
  • [ ] chore: convert PermissionsStartingAt30Fragment to ViewBinding David Allison 11/09/2025, 01:06
  • [x] #19525
  • [x] #19524
  • [x] ImageOcclusionBottomSheetFragment
    • https://github.com/ankidroid/Anki-Android/pull/19396
  • [x] #19482
  • [ ] chore: convert InstantNoteEditorActivity to ViewBinding David Allison 11/09/2025, 00:33
  • [x] #19523
  • [x] #19522
  • [x] #19526
  • [x] #19521
  • [x] #19511
  • [x] #19520
  • [x] #19502
  • [x] #19501
  • [ ] chore: convert AnkiDroidCrashReportDialog to ViewBinding David Allison 10/09/2025, 22:41
  • [x] #19471
  • [x] ~~chore: convert Preferences to ViewBinding David Allison 10/09/2025, 22:27~~
  • [x] #19442
  • [x] #19440
  • [ ] chore: convert PermissionsActivity to ViewBinding David Allison 10/09/2025, 21:47
  • [x] #19439
  • [x] #19429
  • [x] chore: convert StudyOptionsFragment to ViewBinding David Allison 10/09/2025, 21:12
  • [x] #19428
  • [x] #19420
  • [x] chore: convert DrawingActivity to ViewBinding David Allison 10/09/2025, 18:57

Implement View Binding Advantages of using it over findViewById Null safety – Properties in the generated class are non-null. In the case of multiple layout versions, e.g. layout file for different screen orientation or size, if some configuration contains an id that is not present in others, the generated property will be nullable. Type safety – Binding properties will be correctly typed, even with custom views.

Describe the solution you'd like I will try to implement ViewiBinding for all activities and fragments.

Describe alternatives you've considered This issue can be marked as good first issue so it will be helpful for new contributors. I will make a readme for this issue which have all the information about how to implement viewBinding for activities and fragments .

Additional context Advantages Interoperability – Generated classes are in Java and are optimized for Kotlin-Java interoperability. Injection capability – Generated class can be injected in activity or fragment. Speed – There are no impacts on build speed, as it doesn’t use an annotation processor. After the first build with View Binding is enabled, it will dynamically generate new properties. And if you add new view elements to your XML, there is no need to rebuild every time

Jatin887 avatar Apr 27 '22 05:04 Jatin887

Hello! 👋 Thanks for logging this issue. Please remember we are all volunteers here, so some patience may be required before we can get to the issue. Also remember that the fastest way to get resolution on an issue is to propose a change directly, https://github.com/ankidroid/Anki-Android/wiki/Contributing

welcome[bot] avatar Apr 27 '22 05:04 welcome[bot]

Why not go for data binding in case we are migrating to binding from findViewById, we can achieve several other objectives as well (if not currently then in near future for sure)

https://medium.com/geekculture/android-view-binding-v-s-data-binding-5862a27524e9

UvrajSB avatar Apr 27 '22 08:04 UvrajSB

Why not go for data binding in case we are migrating to binding from findViewById, we can achieve several other objectives as well (if not currently then in near future for sure)

https://medium.com/geekculture/android-view-binding-v-s-data-binding-5862a27524e9

Yes, you are right. We can implement both depending on the project's needs. Because databinding isn't necessary in many activities, we can achieve better speed and fatter compilation by utilising viewbinding. However, databinding may undoubtedly be employed in more complex scenarios.

Jatin887 avatar Apr 27 '22 08:04 Jatin887

View Binding: I'm concerned we've got a lot of work in the pipeline already, but I'd support this if someone were willing to estimate the time it will take and commit to seeing it through to completion. We've got 299 cases of findViewById, which is a fair amount of work, but it's feasible to switch easily for most activities. I don't think there are associated performance or stability costs.

Data Binding: likely not right now, we've already got a lot of architectural work in the pipeline, and it's a long commitment with downsides.

david-allison avatar Apr 27 '22 12:04 david-allison

View Binding: I'm concerned we've got a lot of work in the pipeline already, but I'd support this if someone were willing to estimate the time it will take and commit to seeing it through to completion. We've got 299 cases of findViewById, which is a fair amount of work, but it's feasible to switch easily for most activities. I don't think there are associated performance or stability costs.

Data Binding: likely not right now, we've already got a lot of architectural work in the pipeline, and it's a long commitment with downsides.

We can start with shifting to view binding activity by activity/ fragment by fragment. This would allow multiple contributors to work on it and will split the work. If we have to begin with this, I'll like to start with it.

puranjayK avatar Apr 27 '22 12:04 puranjayK

View Binding: I'm concerned we've got a lot of work in the pipeline already, but I'd support this if someone were willing to estimate the time it will take and commit to seeing it through to completion. We've got 299 cases of findViewById, which is a fair amount of work, but it's feasible to switch easily for most activities. I don't think there are associated performance or stability costs.

Data Binding: likely not right now, we've already got a lot of architectural work in the pipeline, and it's a long commitment with downsides.

I agree with all of this - really want to see things completed not started, if that makes sense. Partially completed transitions are terrible as it means you've now got 2 things to maintain not one, it's worse then doing nothing.

Please if you start this, alter the description above to have a list (with checkboxes) for each of the files that need converting so that there is a real idea of how much work there is and we know when it is "done". Perhaps make a lint check that looks for the old usage and generates warning, and make a PR for that - we can periodically rebase the PR and see the current state. Then we know we can actually complete it

Thanks!

mikehardy avatar Apr 27 '22 12:04 mikehardy

View Binding: I'm concerned we've got a lot of work in the pipeline already, but I'd support this if someone were willing to estimate the time it will take and commit to seeing it through to completion. We've got 299 cases of findViewById, which is a fair amount of work, but it's feasible to switch easily for most activities. I don't think there are associated performance or stability costs.

Data Binding: likely not right now, we've already got a lot of architectural work in the pipeline, and it's a long commitment with downsides.

Please assign this issue to me because I initiated it. I am happy to give as much of my time as possible.

Jatin887 avatar Apr 27 '22 12:04 Jatin887

View Binding: I'm concerned we've got a lot of work in the pipeline already, but I'd support this if someone were willing to estimate the time it will take and commit to seeing it through to completion. We've got 299 cases of findViewById, which is a fair amount of work, but it's feasible to switch easily for most activities. I don't think there are associated performance or stability costs. Data Binding: likely not right now, we've already got a lot of architectural work in the pipeline, and it's a long commitment with downsides.

I agree with all of this - really want to see things completed not started, if that makes sense. Partially completed transitions are terrible as it means you've now got 2 things to maintain not one, it's worse then doing nothing.

Please if you start this, alter the description above to have a list (with checkboxes) for each of the files that need converting so that there is a real idea of how much work there is and we know when it is "done". Perhaps make a lint check that looks for the old usage and generates warning, and make a PR for that - we can periodically rebase the PR and see the current state. Then we know we can actually complete it

Thanks!

should i start working on this?

Jatin887 avatar Apr 27 '22 18:04 Jatin887

I think we'd like to understand the amount of effort that this will take before we take this on.

Is this something that you can get a reasonable estimate for?

david-allison avatar Apr 28 '22 00:04 david-allison

I think we'd like to understand the amount of effort that this will take before we take this on.

Is this something that you can get a reasonable estimate for?

1st approach I think we can make this issue same as kotlin-cleanup where anyone can choose onefile and implement viewBinding for that. 2nd approach I alone can work on this issue and i assure you that i will complete this approximately in a month We can make a readme for the same which will help new contributers to understand viewBinding in the project. We can make a lint check also.

Jatin887 avatar Apr 28 '22 07:04 Jatin887

Could you elaborate on what's needed for completion in (2) and how long you expect each of these tasks will take. Ideally in "hours required". How confident are you in these estimates?

I expect most screens will be fairly easy, but some will require a lot more work (maybe testing, maybe not initializing in one place) and it would be useful to see a breakdown of what needs to occur

david-allison avatar Apr 28 '22 09:04 david-allison

Could you elaborate on what's needed for completion in (2) and how long you expect each of these tasks will take. Ideally in "hours required". How confident are you in these estimates?

I expect most screens will be fairly easy, but some will require a lot more work (maybe testing, maybe not initializing in one place) and it would be useful to see a breakdown of what needs to occur

I will be start working on this issue and it will take approx 80-90 hours to complete it.

Jatin887 avatar Apr 29 '22 08:04 Jatin887

What is involved in the 80-90 hours of work? Where will most of the effort go? What are the risky parts? Have you had experience with this before?

david-allison avatar Apr 29 '22 08:04 david-allison

I'll start by going through each activity and fragment to see where the layout files are utilised, and then I'll move on to the activities that just have one layout file because they're easier to implement and test. Then I'll go on to more difficult activities that require many layout files to complete. Because I have to go through each and every layout file to find the specific use of the id, I believe this will be the most challenging and time consuming component. I'll keep the maintainers informed of my progress and ask questions if I get stuck. I've worked on a lot of projects that use viewBinding and have a lot of experience with it.

Jatin887 avatar Apr 29 '22 09:04 Jatin887

Can I work on this ?

ihrishix avatar May 02 '22 12:05 ihrishix

@david-allison can i start working on this. These are the files which i have to convert to achive the goal. I will add view binding in all the files and will have one pull request which will contain all the files of one package(need suggestion)? image image

Jatin887 avatar May 09 '22 16:05 Jatin887

If it's one PR, with the commits appropriately split out, then I don't see an issue.

Ensure you're familiar with rebasing before you begin.

david-allison avatar May 09 '22 16:05 david-allison

If it's one PR, with the commits appropriately split out, then I don't see an issue.

Ensure you're familiar with rebasing before you begin.

Yes i am familiar with it.

Jatin887 avatar May 09 '22 17:05 Jatin887

@rushikeshsuryawanshi sorry for the missing answer. As you have probably understood if you still follow, @Jatin887 was working on it and already did a lot of work actually, so I don't think so.

@Jatin887 Thanks very much for this work. I had not heard about View Binding. Thanks so much for helping us understand by giving a short explanation. Can you help us furthermore by letting us know: do you have any idea how you expect us to review this? It's a huge change. A great change, I totally love the idea. However, I don't see how we can check the work is done correctly (no offense intended here. Anybody doing so many change could get one path wrong by accident), without checking line by line that we agree with you on the path you took.

I'm not sure I follow properly. Are we expecting more PR, or are those four ones doing the whole work?

@mikehardy Since you fear about having two technologies in the code base simultaneously, and having incomplete work, what do you think about, for once, having an ankidroid working branch. So we'll merge all of those changes in the working branch, and when everything is done, we merge this whole working branch into main. The main downside would be the number of conflict it would took. But that's already a risk with some PRs open for 27 days. And given the work that @Jatin887 did, I kind of hope it can be merged relatively quickly.

Arthur-Milchior avatar Jun 06 '22 01:06 Arthur-Milchior

After having reviewed a few of those commits, I've got a main question we must decide. Do we want to keep values for many of the views, or do we just use references?

For example in https://github.com/ankidroid/Anki-Android/pull/11324/commits/ac8254ca5431248ae1a88b02536f5b372afd60c6 , I'd consider replacing mMainLayout = findViewById(R.id.layoutInLoadPronActivity) by mMainLayout = binding.layoutInLoadPronActivity instead of deleting it.

Or the one hand, I understand why you would not want too much values, and it's nice to limit them. On the other hand:

  • it would ensure changes are smaller to review, and thus really easier to review (we could always remove values later)
  • values have a nice understandable name. I really don't know whether they are more or less clear than the path

Arthur-Milchior avatar Jun 06 '22 02:06 Arthur-Milchior

@Jatin887 You mentioned this was done, but I can't see classes such as Reviewer, DeckPicker, AbstractFlashcardViewer etc..., have I missed a PR?

david-allison avatar Jun 08 '22 23:06 david-allison

@Jatin887 You mentioned this was done, but I can't see classes such as Reviewer, DeckPicker, AbstractFlashcardViewer etc..., have I missed a PR?

I didn't change that because i feel these activities doesn't require changes. Implementing View Binding is not worth in these Because they are not inflating any xml file. but if you want me to implement i will do that :)

Jatin887 avatar Jun 11 '22 05:06 Jatin887

My understanding was that this change was getting rid of findViewById and associated lateinit variables. setContentView leads to a lot of findViewById calls

What are the benefits/downsides of leaving them in place?

david-allison avatar Jun 14 '22 13:06 david-allison

My understanding was that this change was getting rid of findViewById and associated lateinit variables. setContentView leads to a lot of findViewById calls

What are the benefits/downsides of leaving them in place?

I am facing difficulty in some of the files as i am not able to find setContentView for most of the files. For eg Reviwer.kt don't have setContentView. So should i go ahead and add ViewBinding in these files also

Jatin887 avatar Jun 27 '22 16:06 Jatin887

Handled in https://github.com/ankidroid/Anki-Android/blob/22950d9fb0cf75f9b9b938ff4f4b3726e645aaca/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt#L325-L331

david-allison avatar Jun 27 '22 17:06 david-allison

Handled in

https://github.com/ankidroid/Anki-Android/blob/22950d9fb0cf75f9b9b938ff4f4b3726e645aaca/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt#L325-L331

Thanks will create a pr for it soon

JatinApk avatar Jun 27 '22 21:06 JatinApk

Sorry for the long silence, been busy in other areas, I was pinged on this though:

Since you fear about having two technologies in the code base simultaneously, and having incomplete work, what do you think about, for once, having an ankidroid working branch. So we'll merge all of those changes in the working branch, and when everything is done, we merge this whole working branch into main. The main downside would be the number of conflict it would took. But that's already a risk with some PRs open for 27 days. And given the work that @Jatin887 did, I kind of hope it can be merged relatively quickly.

I think I fear long-lived branches more than a series of PRs and straddling two technologies temporarily - always tradeoffs, but at least with a stream of PRs there is incremental progress. A long-lived branch sounds like a nightmare with as much activity going on as we have

mikehardy avatar Jun 29 '22 00:06 mikehardy

hope

I will take care of that i will merge the main branch code in each pr and make it update. One more thing all the changes which i have added it's related to views only so it will not be a conflict for sure:)

Jatin887 avatar Jun 29 '22 05:06 Jatin887

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

github-actions[bot] avatar Aug 28 '22 06:08 github-actions[bot]

@Jatin887 @JatinApk What's the status of this?

david-allison avatar Aug 28 '22 13:08 david-allison