From 5c048311b274dd840872b70f4236300ec82d47b6 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Wed, 20 Nov 2024 14:51:24 +0100 Subject: [PATCH] refactor: Ensure copying text experience is consistent (#1115) Previous code was inconsistent about whether or not a notification toast was shown after copying text (contrary to platform guidelines), and there was some code duplication. Fix this with a new `ClipboardUseCase` with a `copyTextTo` method that handles copying text to the clipboard and showing a message afterwards (depending on platform level). --- .../main/java/app/pachli/ViewMediaActivity.kt | 10 +-- .../components/account/AccountActivity.kt | 12 ++-- .../fragments/SearchStatusesFragment.kt | 10 +-- .../TrendingLinksAccessibilityDelegate.kt | 24 ++----- .../java/app/pachli/fragment/SFragment.kt | 10 +-- .../app/pachli/core/ui/ClipboardUseCase.kt | 67 +++++++++++++++++++ .../ArrayAdapterWithCopyButton.kt | 26 ++----- .../pachli/core/ui/di/UseCaseEntryPoint.kt | 33 +++++++++ .../app/pachli/feature/about/AboutFragment.kt | 20 ++---- 9 files changed, 138 insertions(+), 74 deletions(-) create mode 100644 core/ui/src/main/kotlin/app/pachli/core/ui/ClipboardUseCase.kt create mode 100644 core/ui/src/main/kotlin/app/pachli/core/ui/di/UseCaseEntryPoint.kt diff --git a/app/src/main/java/app/pachli/ViewMediaActivity.kt b/app/src/main/java/app/pachli/ViewMediaActivity.kt index 70714c4dd..516931fd8 100644 --- a/app/src/main/java/app/pachli/ViewMediaActivity.kt +++ b/app/src/main/java/app/pachli/ViewMediaActivity.kt @@ -21,9 +21,6 @@ import android.Manifest import android.animation.Animator import android.animation.AnimatorListenerAdapter import android.app.DownloadManager -import android.content.ClipData -import android.content.ClipboardManager -import android.content.Context import android.content.pm.PackageManager import android.graphics.Color import android.os.Build @@ -53,6 +50,7 @@ import app.pachli.core.navigation.AttachmentViewData import app.pachli.core.navigation.ViewMediaActivityIntent import app.pachli.core.navigation.ViewThreadActivityIntent import app.pachli.core.navigation.pachliAccountId +import app.pachli.core.ui.ClipboardUseCase import app.pachli.databinding.ActivityViewMediaBinding import app.pachli.fragment.MediaActionsListener import app.pachli.pager.ImagePagerAdapter @@ -83,6 +81,9 @@ class ViewMediaActivity : BaseActivity(), MediaActionsListener { @Inject lateinit var downloadUrlUseCase: DownloadUrlUseCase + @Inject + lateinit var clipboard: ClipboardUseCase + private val viewModel: ViewMediaViewModel by viewModels() private val binding by viewBinding(ActivityViewMediaBinding::inflate) @@ -260,8 +261,7 @@ class ViewMediaActivity : BaseActivity(), MediaActionsListener { private fun copyLink() { val url = imageUrl ?: attachmentViewData!![binding.viewPager.currentItem].attachment.url - val clipboard = getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager - clipboard.setPrimaryClip(ClipData.newPlainText(null, url)) + clipboard.copyTextTo(url) } private fun shareMedia() { diff --git a/app/src/main/java/app/pachli/components/account/AccountActivity.kt b/app/src/main/java/app/pachli/components/account/AccountActivity.kt index 777f9d9b5..5923ce24b 100644 --- a/app/src/main/java/app/pachli/components/account/AccountActivity.kt +++ b/app/src/main/java/app/pachli/components/account/AccountActivity.kt @@ -17,9 +17,6 @@ package app.pachli.components.account import android.animation.ArgbEvaluator -import android.content.ClipData -import android.content.ClipboardManager -import android.content.Context import android.content.Intent import android.content.res.ColorStateList import android.content.res.Configuration @@ -83,6 +80,7 @@ import app.pachli.core.network.model.Relationship import app.pachli.core.network.parseAsMastodonHtml import app.pachli.core.preferences.AppTheme import app.pachli.core.preferences.PrefKeys +import app.pachli.core.ui.ClipboardUseCase import app.pachli.core.ui.LinkListener import app.pachli.core.ui.extensions.reduceSwipeSensitivity import app.pachli.core.ui.getDomain @@ -129,6 +127,9 @@ class AccountActivity : @Inject lateinit var draftsAlert: DraftsAlert + @Inject + lateinit var clipboard: ClipboardUseCase + private val viewModel: AccountViewModel by viewModels() private val binding: ActivityAccountBinding by viewBinding(ActivityAccountBinding::inflate) @@ -498,10 +499,7 @@ class AccountActivity : view.setOnLongClickListener { loadedAccount?.let { loadedAccount -> val fullUsername = getFullUsername(loadedAccount) - val clipboard = getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager - clipboard.setPrimaryClip(ClipData.newPlainText(null, fullUsername)) - Snackbar.make(binding.root, getString(R.string.account_username_copied), Snackbar.LENGTH_SHORT) - .show() + clipboard.copyTextTo(fullUsername, R.string.account_username_copied) } true } diff --git a/app/src/main/java/app/pachli/components/search/fragments/SearchStatusesFragment.kt b/app/src/main/java/app/pachli/components/search/fragments/SearchStatusesFragment.kt index 50c0e8dc6..530468f10 100644 --- a/app/src/main/java/app/pachli/components/search/fragments/SearchStatusesFragment.kt +++ b/app/src/main/java/app/pachli/components/search/fragments/SearchStatusesFragment.kt @@ -17,9 +17,6 @@ package app.pachli.components.search.fragments import android.Manifest -import android.content.ClipData -import android.content.ClipboardManager -import android.content.Context import android.content.Intent import android.content.pm.PackageManager import android.net.Uri @@ -55,6 +52,7 @@ import app.pachli.core.network.model.Attachment import app.pachli.core.network.model.Poll import app.pachli.core.network.model.Status import app.pachli.core.network.model.Status.Mention +import app.pachli.core.ui.ClipboardUseCase import app.pachli.interfaces.StatusActionListener import app.pachli.view.showMuteAccountDialog import app.pachli.viewdata.StatusViewData @@ -75,6 +73,9 @@ class SearchStatusesFragment : SearchFragment(), StatusActionLis @Inject lateinit var downloadUrlUseCase: DownloadUrlUseCase + @Inject + lateinit var clipboard: ClipboardUseCase + override val data: Flow> get() = viewModel.statusesFlow @@ -281,8 +282,7 @@ class SearchStatusesFragment : SearchFragment(), StatusActionLis return@setOnMenuItemClickListener true } R.id.status_copy_link -> { - val clipboard = requireActivity().getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager - clipboard.setPrimaryClip(ClipData.newPlainText(null, statusUrl)) + statusUrl?.let { clipboard.copyTextTo(it) } return@setOnMenuItemClickListener true } R.id.status_open_as -> { diff --git a/app/src/main/java/app/pachli/components/trending/TrendingLinksAccessibilityDelegate.kt b/app/src/main/java/app/pachli/components/trending/TrendingLinksAccessibilityDelegate.kt index f480f75bb..737321460 100644 --- a/app/src/main/java/app/pachli/components/trending/TrendingLinksAccessibilityDelegate.kt +++ b/app/src/main/java/app/pachli/components/trending/TrendingLinksAccessibilityDelegate.kt @@ -17,21 +17,18 @@ package app.pachli.components.trending -import android.content.ClipData -import android.content.ClipboardManager -import android.os.Build import android.os.Bundle import android.view.View -import android.widget.Toast -import androidx.core.content.ContextCompat import androidx.core.view.AccessibilityDelegateCompat import androidx.core.view.accessibility.AccessibilityNodeInfoCompat import androidx.core.view.accessibility.AccessibilityNodeInfoCompat.AccessibilityActionCompat import androidx.recyclerview.widget.RecyclerView import app.pachli.R import app.pachli.core.ui.accessibility.PachliRecyclerViewAccessibilityDelegate +import app.pachli.core.ui.di.UseCaseEntryPoint import app.pachli.view.PreviewCardView import app.pachli.view.PreviewCardView.Target +import dagger.hilt.android.EntryPointAccessors /** * Accessbility delete for [TrendingLinkViewHolder]. @@ -44,6 +41,9 @@ internal class TrendingLinksAccessibilityDelegate( private val recyclerView: RecyclerView, val listener: PreviewCardView.OnClickListener, ) : PachliRecyclerViewAccessibilityDelegate(recyclerView) { + private val useCaseEntryPoint = EntryPointAccessors.fromApplication(context.applicationContext) + val clipboard = useCaseEntryPoint.clipboardUseCase + private val openLinkAction = AccessibilityActionCompat( app.pachli.core.ui.R.id.action_open_link, context.getString(R.string.action_open_link), @@ -85,19 +85,7 @@ internal class TrendingLinksAccessibilityDelegate( true } app.pachli.core.ui.R.id.action_copy_item -> { - val clipboard = ContextCompat.getSystemService( - context, - ClipboardManager::class.java, - ) as ClipboardManager - val clip = ClipData.newPlainText("", viewHolder.link.url) - clipboard.setPrimaryClip(clip) - if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.S_V2) { - Toast.makeText( - context, - context.getString(app.pachli.core.ui.R.string.item_copied), - Toast.LENGTH_SHORT, - ).show() - } + clipboard.copyTextTo(viewHolder.link.url) true } app.pachli.core.ui.R.id.action_open_byline_account -> { diff --git a/app/src/main/java/app/pachli/fragment/SFragment.kt b/app/src/main/java/app/pachli/fragment/SFragment.kt index 7cce461d7..9c0210dcc 100644 --- a/app/src/main/java/app/pachli/fragment/SFragment.kt +++ b/app/src/main/java/app/pachli/fragment/SFragment.kt @@ -16,8 +16,6 @@ package app.pachli.fragment import android.Manifest -import android.content.ClipData -import android.content.ClipboardManager import android.content.Context import android.content.DialogInterface import android.content.Intent @@ -60,6 +58,7 @@ import app.pachli.core.network.model.Attachment import app.pachli.core.network.model.Status import app.pachli.core.network.parseAsMastodonHtml import app.pachli.core.network.retrofit.MastodonApi +import app.pachli.core.ui.ClipboardUseCase import app.pachli.core.ui.extensions.getErrorString import app.pachli.interfaces.StatusActionListener import app.pachli.usecase.TimelineCases @@ -95,6 +94,9 @@ abstract class SFragment : Fragment(), StatusActionListener @Inject lateinit var downloadUrlUseCase: DownloadUrlUseCase + @Inject + lateinit var clipboard: ClipboardUseCase + private var serverCanTranslate = false protected abstract var pachliAccountId: Long @@ -296,9 +298,7 @@ abstract class SFragment : Fragment(), StatusActionListener return@setOnMenuItemClickListener true } R.id.status_copy_link -> { - (requireActivity().getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager).apply { - setPrimaryClip(ClipData.newPlainText(null, statusUrl)) - } + statusUrl?.let { clipboard.copyTextTo(it) } return@setOnMenuItemClickListener true } R.id.status_open_as -> { diff --git a/core/ui/src/main/kotlin/app/pachli/core/ui/ClipboardUseCase.kt b/core/ui/src/main/kotlin/app/pachli/core/ui/ClipboardUseCase.kt new file mode 100644 index 000000000..f1f8ba058 --- /dev/null +++ b/core/ui/src/main/kotlin/app/pachli/core/ui/ClipboardUseCase.kt @@ -0,0 +1,67 @@ +/* + * Copyright 2024 Pachli Association + * + * This file is a part of Pachli. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Pachli; if not, + * see . + */ + +package app.pachli.core.ui + +import android.content.ClipData +import android.content.ClipboardManager +import android.content.Context +import android.os.Build +import android.widget.Toast +import androidx.annotation.StringRes +import androidx.core.content.ContextCompat +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject + +/** + * Copies plain text to the clipboard as the primary clip. + */ +class ClipboardUseCase @Inject constructor( + @ApplicationContext val context: Context, +) { + private val clipboard: ClipboardManager by lazy { + ContextCompat.getSystemService( + context, + ClipboardManager::class.java, + ) as ClipboardManager + } + + /** + * Copies [text] to the clipboard as the primary clip, with optional + * [label]. If necessary displays a toast showing [message] to confirm + * copy is complete. + * + * @param text Text to copy. + * @param message Optional message to show after completion. + * @param label Optional user-visible label to associate with the copied + * text, see [ClipData.newPlainText]. + */ + fun copyTextTo( + text: CharSequence, + @StringRes message: Int = R.string.item_copied, + label: CharSequence = "", + ) { + clipboard.setPrimaryClip(ClipData.newPlainText(label, text)) + notify(message) + } + + private fun notify(@StringRes message: Int) { + if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.S_V2) { + Toast.makeText(context, context.getString(message), Toast.LENGTH_SHORT).show() + } + } +} diff --git a/core/ui/src/main/kotlin/app/pachli/core/ui/accessibility/ArrayAdapterWithCopyButton.kt b/core/ui/src/main/kotlin/app/pachli/core/ui/accessibility/ArrayAdapterWithCopyButton.kt index d88e94fa0..ca3529e3c 100644 --- a/core/ui/src/main/kotlin/app/pachli/core/ui/accessibility/ArrayAdapterWithCopyButton.kt +++ b/core/ui/src/main/kotlin/app/pachli/core/ui/accessibility/ArrayAdapterWithCopyButton.kt @@ -17,18 +17,15 @@ package app.pachli.core.ui.accessibility -import android.content.ClipData -import android.content.ClipboardManager import android.content.Context -import android.os.Build import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.ArrayAdapter -import android.widget.Toast -import androidx.core.content.ContextCompat import app.pachli.core.ui.R import app.pachli.core.ui.databinding.SimpleListItem1CopyButtonBinding +import app.pachli.core.ui.di.UseCaseEntryPoint +import dagger.hilt.android.EntryPointAccessors /** * An [ArrayAdapter] that shows a "copy" button next to each item. @@ -49,27 +46,16 @@ class ArrayAdapterWithCopyButton( fun onClick(position: Int) } + private val useCaseEntryPoint = EntryPointAccessors.fromApplication(context.applicationContext) + private val clipboard = useCaseEntryPoint.clipboardUseCase + override fun getView(position: Int, convertView: View?, parent: ViewGroup): View { val binding = if (convertView == null) { SimpleListItem1CopyButtonBinding.inflate(LayoutInflater.from(context), parent, false).apply { text1.setOnClickListener { listener.onClick(position) } copy.setOnClickListener { - getItem(position)?.let { text -> - val clipboard = ContextCompat.getSystemService( - context, - ClipboardManager::class.java, - ) as ClipboardManager - val clip = ClipData.newPlainText("", text) - clipboard.setPrimaryClip(clip) - if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.S_V2) { - Toast.makeText( - context, - context.getString(R.string.item_copied), - Toast.LENGTH_SHORT, - ).show() - } - } + getItem(position)?.let { clipboard.copyTextTo(it) } } } } else { diff --git a/core/ui/src/main/kotlin/app/pachli/core/ui/di/UseCaseEntryPoint.kt b/core/ui/src/main/kotlin/app/pachli/core/ui/di/UseCaseEntryPoint.kt new file mode 100644 index 000000000..8a3585f4d --- /dev/null +++ b/core/ui/src/main/kotlin/app/pachli/core/ui/di/UseCaseEntryPoint.kt @@ -0,0 +1,33 @@ +/* + * Copyright 2024 Pachli Association + * + * This file is a part of Pachli. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Pachli; if not, + * see . + */ + +package app.pachli.core.ui.di + +import app.pachli.core.ui.ClipboardUseCase +import dagger.hilt.EntryPoint +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent + +/** + * Entry point for use cases that need to be field-injected in to classes + * Hilt does not manage. + */ +@EntryPoint +@InstallIn(SingletonComponent::class) +interface UseCaseEntryPoint { + val clipboardUseCase: ClipboardUseCase +} diff --git a/feature/about/src/main/kotlin/app/pachli/feature/about/AboutFragment.kt b/feature/about/src/main/kotlin/app/pachli/feature/about/AboutFragment.kt index a6a7462af..d5f729eb2 100644 --- a/feature/about/src/main/kotlin/app/pachli/feature/about/AboutFragment.kt +++ b/feature/about/src/main/kotlin/app/pachli/feature/about/AboutFragment.kt @@ -17,8 +17,6 @@ package app.pachli.feature.about -import android.content.ClipData -import android.content.ClipboardManager import android.os.Build import android.os.Bundle import android.text.SpannableString @@ -28,9 +26,7 @@ import android.text.style.URLSpan import android.text.util.Linkify import android.view.View import android.widget.TextView -import android.widget.Toast import androidx.annotation.StringRes -import androidx.core.content.ContextCompat.getSystemService import androidx.fragment.app.Fragment import androidx.fragment.app.viewModels import androidx.lifecycle.lifecycleScope @@ -39,13 +35,18 @@ import app.pachli.core.common.extensions.hide import app.pachli.core.common.extensions.show import app.pachli.core.common.extensions.viewBinding import app.pachli.core.common.util.versionName +import app.pachli.core.ui.ClipboardUseCase import app.pachli.core.ui.NoUnderlineURLSpan import app.pachli.feature.about.databinding.FragmentAboutBinding import dagger.hilt.android.AndroidEntryPoint +import javax.inject.Inject import kotlinx.coroutines.launch @AndroidEntryPoint class AboutFragment : Fragment(R.layout.fragment_about) { + @Inject + lateinit var clipboard: ClipboardUseCase + private val viewModel: AboutFragmentViewModel by viewModels() private val binding by viewBinding(FragmentAboutBinding::bind) @@ -100,16 +101,7 @@ class AboutFragment : Fragment(R.layout.fragment_about) { binding.copyDeviceInfo.setOnClickListener { val text = "$version\n\nDevice:\n\n$deviceInfo\n\nAccount:\n\n${binding.accountInfo.text}" - val clipboard = getSystemService(requireContext(), ClipboardManager::class.java) as ClipboardManager - val clip = ClipData.newPlainText("Pachli version information", text) - clipboard.setPrimaryClip(clip) - if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.S_V2) { - Toast.makeText( - requireContext(), - getString(R.string.about_copied), - Toast.LENGTH_SHORT, - ).show() - } + clipboard.copyTextTo(text, R.string.about_copied, "Pachli version information") } }