fix: use default constructor / factory + Arguments init style

[mikehardy]

Purpose / Description

this allows Fragment lifecycle save/restore to work seemlessly and that fixes crashes on Activity restarts

NOTE THAT IF THIS STYLE IS OKAY - A FEW OTHER DIALOGS NEED FIXES IN SAME STYLE

[edit: turns out this was the only easy one like this - so just one commit]

Fixes

• Fixes Unable to start activity ComponentInfo{com.ichi2.anki/com.ichi2.anki.CardBrowser}: com.ichi2.anki.exception.UserSubmittedException: Unable to instantiate fragment X3.h: could not find Fragment constructor #17381

Approach

Used the approach mentioned here https://stackoverflow.com/a/51831137/9910298

• Stop providing params to constructor, so there is a default constructor as required by default Fragment factory
• Start providing params as a Bundle set in to Fragment.arguments via a static newInstance method
• Access all params via Fragment.arguments
• Handle optionally missing arguments with warnings+defaults

How Has This Been Tested?

• Enable developer option Don't Keep Activities
• Go to Card Browser, open Actions Menu "Options"
• Background AnkiDroid, resume AnkiDroid --> crash without patch
• With patch check all toggles + background/foreground permutations

Everything seems to work

browser-options-crashfix.webm

Learning (optional, can help others)

Now we don't just have Activity lifecycle problems, we have Fragment lifecycle problems. Great

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[BrayanDSO]

Fully agreed with empty constructors for Fragments and passing stuff with arguments. Following the Android way of things is normally the most sane alternative.

[mikehardy]

Okay, going to take that preliminary note of approval for the style, Kotlin-ify it, and try another Dialog Fragment or two

[lukstbit]

Yes, dialogs with constructor parameters that don't use a fragment factory should be refactored.

[mikehardy]

Turns out there was only one other DialogFragment with a problem like this (no default constructor) and it has a Factory but it must just not get installed correctly every time. I'm not going to add that commit here, I'll work it in a separate PR

So this is ready to go

I've adopted all the feedback to make this more Kotlin-y (and extract string constants) - thank you!

Three people approved but there were enough little changes from the feedback I think at least one other person should take a final look before merge so I am repinging the reviewers but hopefully (since it is a simple change) it's easy and done

[david-allison]

Minor improvement with constants I believe, code is good

Although I stated that the styles of getter was inconsistent, I prefer them the way they are now.

The following looks ugly to me

    private val isTruncated by lazy {
        when (arguments?.getBoolean("isTruncated")) {
            true -> true
            false -> false
            null -> {
                Timber.w("BrowserOptionsDialog instantiated without configuration.")
                false
            }
        }
   

[david-allison]

Note: this is now just a spacing change