Anki-Android
Anki-Android copied to clipboard
Removed the Material Dialog Box and Added Alert Box from CardBrowserMySearchesDialog
Fixes
- Related #13315
Approach
Replaces usages of Deprecated MaterialDialog and adds the Native AlertDialog For CardBrowserMySearchesDialog.
Video
https://github.com/ankidroid/Anki-Android/assets/60827173/e94a9360-9d12-462e-9b32-d75a9dcbeed5
Few questions :
~~1. Are we okay with removing the divider lines in My searches which were there in Material Dialog? As in our extension method of Alert Dialog we don't have it.
2. Padding seems uneven for (-)
in my searches compared to Material dialog not sure its it good to go or we need to set padding for (-)
explicitly .~~
- On closer look when we open save search the keyboard doesn't seems to work it needs a tap at the input and the input opens agains and works fine. I am not able to fix this issue. Need help in this.
- Can we add prefill for save search? As the user who search a particular thing is probably gonna save that new thing. Correct me if I am wrong. I didn't added it yet as it was not there previously.
input(hint = getString(R.string.card_browser_list_my_searches_new_name), **prefill = currentSearchTerms**, allowEmpty = false, displayKeyboard = true, waitForPositiveButton = true)
Checklist
Please, go through these checks before submitting the PR.
- [x] You have a descriptive commit message with a short title (first line, max 50 chars).
- [x] You have commented your code, particularly in hard-to-understand areas
- [x] You have performed a self-review of your own code
- [X] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
- [X] UI Changes: You have tested your change using the Google Accessibility Scanner
Split out the 'easy' dialog into another PR
Fixing the second dialog will take longer. Padding definitely needs to be fixed
Also what about the divider decoration?
Edit : I fixed it. Edit: Updated the PR description and added the commit. @david-allison ~~I have not tested it yet but I am thinking what if I create an extension function and apply it so we would be having same ui as before (will also have decoration divider, padding).~~ There is no need for padding. I just made the layout width match parent in card_browser_item_my_searches_dialog.xml and it works fine and gives us the desired result.
This is what I came up with it for extension method in AlertDialogFacade for creating the decoration divider which would make the code follow DRY approach for later.
/**
* Adds a RecyclerView with a custom adapter and decoration to the AlertDialog.
* @param adapter The adapter for the RecyclerView.
* @param context The context used to access resources and LayoutInflater.
*/
fun AlertDialog.Builder.customListAdapterWithDecoration(adapter: RecyclerView.Adapter<*>, context: Context) {
val recyclerView = LayoutInflater.from(context).inflate(R.layout.dialog_generic_recycler_view, null, false) as RecyclerView
recyclerView.adapter = adapter
recyclerView.layoutManager = LinearLayoutManager(context)
val dividerItemDecoration = DividerItemDecoration(recyclerView.context, LinearLayoutManager.VERTICAL)
recyclerView.addItemDecoration(dividerItemDecoration)
this.setView(recyclerView)
}
The vertical padding on that screenshot looks off, and the touch targets look too small
Try adding 8-12dp top & bottom and see if it looks good
The vertical padding on that screenshot looks off, and the touch targets look too small
Try adding 8-12dp top & bottom and see if it looks good
I simplified the layout instead of nested layout(relative and linear, increasing size of imageview was not making the textview vertical center aligned properly) I made it simplified and clean.
few things can be again added as per accessibility suggestion by Android Studio
This item may not have a label readable by screen readers.
Adding a content description would fix this. I am not sure which string to add for it and not sure from localization. I am not adding it also its of low priority. Just adding it for reference of issue.
@neeldoshii Check the patch below for specific changes. Note that I didn't verify the dialog on the emulator.
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt
index b068b3d804..78bd03204c 100644
--- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt
@@ -5,20 +5,17 @@ import android.annotation.SuppressLint
import android.app.Dialog
import android.os.Bundle
import androidx.appcompat.app.AlertDialog
-import androidx.recyclerview.widget.DividerItemDecoration
-import androidx.recyclerview.widget.LinearLayoutManager
-import com.afollestad.materialdialogs.MaterialDialog
-import com.afollestad.materialdialogs.input.input
-import com.afollestad.materialdialogs.list.customListAdapter
-import com.afollestad.materialdialogs.list.getRecyclerView
import com.ichi2.anki.R
import com.ichi2.anki.analytics.AnalyticsDialogFragment
import com.ichi2.compat.CompatHelper.Companion.getSerializableCompat
import com.ichi2.ui.ButtonItemAdapter
+import com.ichi2.utils.createAndApply
+import com.ichi2.utils.input
import com.ichi2.utils.message
import com.ichi2.utils.negativeButton
import com.ichi2.utils.positiveButton
import com.ichi2.utils.show
+import com.ichi2.utils.title
import timber.log.Timber
// TODO: Add different classes for the two different dialogs
@@ -37,7 +34,7 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() {
@SuppressLint("CheckResult")
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
super.onCreate(savedInstanceState)
- val dialog = MaterialDialog(requireActivity())
+ val dialog = AlertDialog.Builder(requireActivity())
val type = requireArguments().getInt("type")
if (type == CARD_BROWSER_MY_SEARCHES_TYPE_LIST) {
savedFilters = requireArguments().getSerializableCompat("savedFilters")
@@ -51,7 +48,7 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() {
itemCallback = { searchName ->
Timber.d("item clicked: %s", searchName)
mySearchesDialogListener!!.onSelection(searchName)
- dialog.dismiss()
+ dismiss()
},
buttonCallback = { searchName ->
Timber.d("button clicked: %s", searchName)
@@ -60,27 +57,24 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() {
).apply {
notifyAdapterDataSetChanged() // so the values are sorted.
dialog.title(text = resources.getString(R.string.card_browser_list_my_searches_title))
- .customListAdapter(this, null)
+ .customListAdapterWithDecoration(this, requireActivity())
}
} else if (type == CARD_BROWSER_MY_SEARCHES_TYPE_SAVE) {
val currentSearchTerms = requireArguments().getString("currentSearchTerms")
- dialog.title(text = getString(R.string.card_browser_list_my_searches_save))
- .positiveButton(android.R.string.ok)
- .negativeButton(R.string.dialog_cancel)
- .input(hintRes = R.string.card_browser_list_my_searches_new_name) { _: MaterialDialog?, text: CharSequence ->
- Timber.d("Saving search with title/terms: %s/%s", text, currentSearchTerms)
- mySearchesDialogListener!!.onSaveSearch(text.toString(), currentSearchTerms)
+ return dialog.show {
+ title(text = getString(R.string.card_browser_list_my_searches_save))
+ positiveButton(android.R.string.ok)
+ negativeButton(R.string.dialog_cancel)
+ setView(R.layout.dialog_generic_text_input)
+ }.apply {
+ input(hint = getString(R.string.card_browser_list_my_searches_new_name)) { dialog, text ->
+ Timber.d("Saving search with title/terms: %s/%s", text, currentSearchTerms)
+ mySearchesDialogListener!!.onSaveSearch(text.toString(), currentSearchTerms)
+ dialog.dismiss()
+ }
}
}
- runCatching { dialog.getRecyclerView() }.onSuccess { recyclerView ->
- val layoutManager = recyclerView.layoutManager as LinearLayoutManager
- val dividerItemDecoration = DividerItemDecoration(recyclerView.context, layoutManager.orientation)
- val scale = resources.displayMetrics.density
- val dpAsPixels = (5 * scale + 0.5f).toInt()
- dialog.view.setPadding(dpAsPixels, 0, dpAsPixels, dpAsPixels)
- recyclerView.addItemDecoration(dividerItemDecoration)
- }
- return dialog
+ return dialog.create()
}
private fun removeSearch(searchName: String) {
@neeldoshii Check the patch below for specific changes. Note that I didn't verify the dialog on the emulator.
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt index b068b3d804..78bd03204c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/CardBrowserMySearchesDialog.kt @@ -5,20 +5,17 @@ import android.annotation.SuppressLint import android.app.Dialog import android.os.Bundle import androidx.appcompat.app.AlertDialog -import androidx.recyclerview.widget.DividerItemDecoration -import androidx.recyclerview.widget.LinearLayoutManager -import com.afollestad.materialdialogs.MaterialDialog -import com.afollestad.materialdialogs.input.input -import com.afollestad.materialdialogs.list.customListAdapter -import com.afollestad.materialdialogs.list.getRecyclerView import com.ichi2.anki.R import com.ichi2.anki.analytics.AnalyticsDialogFragment import com.ichi2.compat.CompatHelper.Companion.getSerializableCompat import com.ichi2.ui.ButtonItemAdapter +import com.ichi2.utils.createAndApply +import com.ichi2.utils.input import com.ichi2.utils.message import com.ichi2.utils.negativeButton import com.ichi2.utils.positiveButton import com.ichi2.utils.show +import com.ichi2.utils.title import timber.log.Timber // TODO: Add different classes for the two different dialogs @@ -37,7 +34,7 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() { @SuppressLint("CheckResult") override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { super.onCreate(savedInstanceState) - val dialog = MaterialDialog(requireActivity()) + val dialog = AlertDialog.Builder(requireActivity()) val type = requireArguments().getInt("type") if (type == CARD_BROWSER_MY_SEARCHES_TYPE_LIST) { savedFilters = requireArguments().getSerializableCompat("savedFilters") @@ -51,7 +48,7 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() { itemCallback = { searchName -> Timber.d("item clicked: %s", searchName) mySearchesDialogListener!!.onSelection(searchName) - dialog.dismiss() + dismiss() }, buttonCallback = { searchName -> Timber.d("button clicked: %s", searchName) @@ -60,27 +57,24 @@ class CardBrowserMySearchesDialog : AnalyticsDialogFragment() { ).apply { notifyAdapterDataSetChanged() // so the values are sorted. dialog.title(text = resources.getString(R.string.card_browser_list_my_searches_title)) - .customListAdapter(this, null) + .customListAdapterWithDecoration(this, requireActivity()) } } else if (type == CARD_BROWSER_MY_SEARCHES_TYPE_SAVE) { val currentSearchTerms = requireArguments().getString("currentSearchTerms") - dialog.title(text = getString(R.string.card_browser_list_my_searches_save)) - .positiveButton(android.R.string.ok) - .negativeButton(R.string.dialog_cancel) - .input(hintRes = R.string.card_browser_list_my_searches_new_name) { _: MaterialDialog?, text: CharSequence -> - Timber.d("Saving search with title/terms: %s/%s", text, currentSearchTerms) - mySearchesDialogListener!!.onSaveSearch(text.toString(), currentSearchTerms) + return dialog.show { + title(text = getString(R.string.card_browser_list_my_searches_save)) + positiveButton(android.R.string.ok) + negativeButton(R.string.dialog_cancel) + setView(R.layout.dialog_generic_text_input) + }.apply { + input(hint = getString(R.string.card_browser_list_my_searches_new_name)) { dialog, text -> + Timber.d("Saving search with title/terms: %s/%s", text, currentSearchTerms) + mySearchesDialogListener!!.onSaveSearch(text.toString(), currentSearchTerms) + dialog.dismiss() + } } } - runCatching { dialog.getRecyclerView() }.onSuccess { recyclerView -> - val layoutManager = recyclerView.layoutManager as LinearLayoutManager - val dividerItemDecoration = DividerItemDecoration(recyclerView.context, layoutManager.orientation) - val scale = resources.displayMetrics.density - val dpAsPixels = (5 * scale + 0.5f).toInt() - dialog.view.setPadding(dpAsPixels, 0, dpAsPixels, dpAsPixels) - recyclerView.addItemDecoration(dividerItemDecoration) - } - return dialog + return dialog.create() } private fun removeSearch(searchName: String) {
tested and fixed the issue in the current commit. Thank you @lukstbit
It still feels like the padding is too large on the right hand side
Implementer's choice, it looks better than the previous screen at this point
Thanks!!!
Should I update the code to match the padding like this for reducing padding right?
- Can we add prefill for save search? As the user who search a particular thing is probably gonna save that new thing. Correct me if I am wrong. I didn't added it yet as it was not there previously.
input(hint = getString(R.string.card_browser_list_my_searches_new_name), **prefill = currentSearchTerms**, allowEmpty = false, displayKeyboard = true, waitForPositiveButton = true)
I have one more question from the PR description which could be helpful. Should we add this?
Related to your questions, leave the code/UI as it is for now.
squashed + force pushed