Skip to content

Commit 4479ab7

Browse files
committed
Address code review feedback
- Use View Binding for dialog - Switch to FixedTextView in layout - Use android.R.string for standard buttons - Use Context.copyToClipboard() utility - Apply sentence case to dialog title - Simplify dialog message wording - Use AlertDialogFacade.create extension - Move fragment result listener to onCreate() - Remove clearFragmentResultListener from callback - Add listener cleanup in onDestroy()
1 parent a896781 commit 4479ab7

File tree

4 files changed

+39
-54
lines changed

4 files changed

+39
-54
lines changed

AnkiDroid/src/main/java/com/ichi2/anki/account/AccountRemovalExplanationDialog.kt

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,62 +15,43 @@
1515
* this program. If not, see <http://www.gnu.org/licenses/>.
1616
*/
1717
package com.ichi2.anki.account
18-
import android.annotation.SuppressLint
1918
import android.app.Dialog
20-
import android.content.ClipData
21-
import android.content.ClipboardManager
22-
import android.content.Context
2319
import android.os.Bundle
24-
import android.view.LayoutInflater
25-
import android.widget.Button
26-
import android.widget.TextView
2720
import androidx.core.os.bundleOf
2821
import androidx.fragment.app.DialogFragment
2922
import androidx.fragment.app.setFragmentResult
3023
import androidx.preference.PreferenceManager
3124
import com.google.android.material.dialog.MaterialAlertDialogBuilder
3225
import com.ichi2.anki.R
33-
import com.ichi2.anki.snackbar.showSnackbar
26+
import com.ichi2.anki.databinding.DialogAccountRemovalExplanationBinding
27+
import com.ichi2.utils.copyToClipboard
28+
import com.ichi2.utils.create
3429

3530
class AccountRemovalExplanationDialog : DialogFragment() {
3631
companion object {
3732
const val REQUEST_KEY = "account_removal_explanation"
3833
const val RESULT_PROCEED = "proceed"
3934

40-
fun newInstance(): AccountRemovalExplanationDialog = AccountRemovalExplanationDialog()
35+
fun newInstance() = AccountRemovalExplanationDialog()
4136
}
4237

43-
@SuppressLint("UseGetLayoutInflater")
4438
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
4539
val preferences = PreferenceManager.getDefaultSharedPreferences(requireContext())
4640
val email = preferences.getString("username", "") ?: ""
4741

48-
val view =
49-
LayoutInflater
50-
.from(requireContext())
51-
.inflate(R.layout.dialog_account_removal_explanation, null)
42+
val binding = DialogAccountRemovalExplanationBinding.inflate(layoutInflater)
5243

53-
val emailTextView: TextView = view.findViewById(R.id.email_text)
54-
val copyEmailButton: Button = view.findViewById(R.id.copy_email_button)
55-
56-
emailTextView.text = email
57-
58-
copyEmailButton.setOnClickListener {
59-
copyEmailToClipboard(email)
44+
binding.emailText.text = email
45+
binding.copyEmailButton.setOnClickListener {
46+
requireContext().copyToClipboard(email)
6047
}
6148

62-
return MaterialAlertDialogBuilder(requireContext())
63-
.setView(view)
64-
.setPositiveButton(R.string.proceed) { _, _ ->
49+
return MaterialAlertDialogBuilder(requireContext()).create {
50+
setView(binding.root)
51+
setPositiveButton(R.string.dialog_ok) { _, _ ->
6552
setFragmentResult(REQUEST_KEY, bundleOf(RESULT_PROCEED to true))
66-
}.setNegativeButton(R.string.cancel_btn, null)
67-
.create()
68-
}
69-
70-
private fun copyEmailToClipboard(email: String) {
71-
val clipboard = requireContext().getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager
72-
val clip = ClipData.newPlainText("AnkiWeb Email", email)
73-
clipboard.setPrimaryClip(clip)
74-
showSnackbar(R.string.email_copied)
53+
}
54+
setNegativeButton(R.string.dialog_cancel) { _, _ -> }
55+
}
7556
}
7657
}

AnkiDroid/src/main/java/com/ichi2/anki/account/LoggedInFragment.kt

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ class LoggedInFragment : Fragment(R.layout.my_account_logged_in) {
5656

5757
private lateinit var loggedInLogo: ImageView
5858

59+
override fun onCreate(savedInstanceState: Bundle?) {
60+
super.onCreate(savedInstanceState)
61+
62+
// Register the listener BEFORE the dialog is shown
63+
parentFragmentManager.setFragmentResultListener(
64+
AccountRemovalExplanationDialog.REQUEST_KEY,
65+
this,
66+
) { _, bundle ->
67+
if (bundle.getBoolean(AccountRemovalExplanationDialog.RESULT_PROCEED, false)) {
68+
openRemoveAccountScreen()
69+
}
70+
}
71+
}
72+
5973
override fun onViewCreated(
6074
view: View,
6175
savedInstanceState: Bundle?,
@@ -106,17 +120,6 @@ class LoggedInFragment : Fragment(R.layout.my_account_logged_in) {
106120
*/
107121
private fun showAccountRemovalExplanation() {
108122
val dialog = AccountRemovalExplanationDialog.newInstance()
109-
110-
parentFragmentManager.setFragmentResultListener(
111-
AccountRemovalExplanationDialog.REQUEST_KEY,
112-
viewLifecycleOwner,
113-
) { _, bundle ->
114-
if (bundle.getBoolean(AccountRemovalExplanationDialog.RESULT_PROCEED, false)) {
115-
openRemoveAccountScreen()
116-
}
117-
parentFragmentManager.clearFragmentResultListener(AccountRemovalExplanationDialog.REQUEST_KEY)
118-
}
119-
120123
dialog.show(parentFragmentManager, "AccountRemovalExplanationDialog")
121124
}
122125

@@ -144,4 +147,9 @@ class LoggedInFragment : Fragment(R.layout.my_account_logged_in) {
144147
super.onConfigurationChanged(newConfig)
145148
loggedInLogo.isVisible = !(isCompactWidth && newConfig.orientation == Configuration.ORIENTATION_LANDSCAPE)
146149
}
150+
151+
override fun onDestroy() {
152+
super.onDestroy()
153+
parentFragmentManager.clearFragmentResultListener(AccountRemovalExplanationDialog.REQUEST_KEY)
154+
}
147155
}

AnkiDroid/src/main/res/layout/dialog_account_removal_explanation.xml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
android:orientation="vertical"
66
android:padding="24dp">
77

8-
<TextView
8+
<com.ichi2.ui.FixedTextView
99
android:id="@+id/dialog_title"
1010
android:layout_width="match_parent"
1111
android:layout_height="wrap_content"
@@ -14,7 +14,7 @@
1414
android:textColor="?attr/colorOnSurface"
1515
android:layout_marginBottom="16dp" />
1616

17-
<TextView
17+
<com.ichi2.ui.FixedTextView
1818
android:id="@+id/dialog_message"
1919
android:layout_width="match_parent"
2020
android:layout_height="wrap_content"
@@ -31,7 +31,7 @@
3131
android:padding="12dp"
3232
android:gravity="center_vertical">
3333

34-
<TextView
34+
<com.ichi2.ui.FixedTextView
3535
android:id="@+id/email_text"
3636
android:layout_width="0dp"
3737
android:layout_height="wrap_content"
@@ -46,7 +46,7 @@
4646
style="@style/Widget.Material3.Button.TextButton"
4747
android:layout_width="wrap_content"
4848
android:layout_height="wrap_content"
49-
android:text="@string/copy"
49+
android:text="@android:string/copy"
5050
android:minWidth="0dp"
5151
android:paddingStart="8dp"
5252
android:paddingEnd="8dp" />

AnkiDroid/src/main/res/values/02-strings.xml

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -438,11 +438,7 @@ opening the system text to speech settings fails">Failed to open text to speech
438438
<!-- Scheduler Upgrade Required Dialog title-->
439439
<string name="scheduler_upgrade_required_dialog_title" comment="Shown in a dialog when user has imported a collection using a very old version of the Anki spaced repetition scheduler">Scheduler upgrade required</string>
440440

441-
<string name="remove_account_explanation_title">Account Removal Requires Authentication</string>
442-
<string name="remove_account_explanation_message">To remove your AnkiWeb account, you need to re-authenticate first. You will be redirected to the AnkiWeb login page.</string>
443-
<string name="copy">Copy</string>
444-
<string name="email_copied">Email copied to clipboard</string>
445-
<string name="proceed">Proceed</string>
446-
<string name="cancel_btn">Dismiss</string>
441+
<string name="remove_account_explanation_title">Re-authentication required</string>
442+
<string name="remove_account_explanation_message">You need to re-authenticate before you remove your AnkiWeb account. You will be redirected to the AnkiWeb login page.</string>
447443
</resources>
448444

0 commit comments

Comments
 (0)