diff --git a/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt index 51c88fdcaae4..ad43c639159d 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt @@ -85,19 +85,6 @@ class LandingTests { Intents.intended(hasAction(Intent.ACTION_OPEN_DOCUMENT)) } - @Test - fun theFabOpensTheSystemPicker() { - // the fab is hidden behind the empty state, which offers the same thing with a label - seedRecentDocument() - stubOpenDocumentCancelled() - - launch() - - onView(withId(R.id.landing_open_fab)).perform(click()) - - Intents.intended(hasAction(Intent.ACTION_OPEN_DOCUMENT)) - } - @Test fun aRecentDocumentIsListed() { seedRecentDocument() @@ -121,6 +108,19 @@ class LandingTests { ) } + @Test + fun theFabOpensTheSystemPicker() { + // the fab is hidden behind the empty state, which offers the same thing with a label + seedRecentDocument() + stubOpenDocumentCancelled() + + launch() + + onView(withId(R.id.landing_open_fab)).perform(click()) + + Intents.intended(hasAction(Intent.ACTION_OPEN_DOCUMENT)) + } + @Test fun addingAFolderAsksTheSystemForATree() { Intents.intending(hasAction(Intent.ACTION_OPEN_DOCUMENT_TREE)) diff --git a/app/src/main/java/app/opendocument/droid/background/RecentDocumentList.kt b/app/src/main/java/app/opendocument/droid/background/RecentDocumentList.kt index e62367db4d98..a430cd990a35 100644 --- a/app/src/main/java/app/opendocument/droid/background/RecentDocumentList.kt +++ b/app/src/main/java/app/opendocument/droid/background/RecentDocumentList.kt @@ -52,4 +52,19 @@ object RecentDocumentList { /** Drops every entry for [uri]. Returns [current] unchanged if there is none. */ fun remove(current: List, uri: String): List = current.filter { it.uri != uri } + + /** + * Puts [entry] back at [index], for undoing a removal. + * + * Unlike [add] this does not move it to the front - the point of an undo is that the list ends + * up looking like it did before. An index past the end lands at the end. + */ + fun insert(current: List, entry: Entry, index: Int): List { + val entries = ArrayList(current.size + 1) + current.filterTo(entries) { it.uri != entry.uri } + + entries.add(index.coerceIn(0, entries.size), entry) + + return entries + } } diff --git a/app/src/main/java/app/opendocument/droid/background/RecentDocumentsUtil.kt b/app/src/main/java/app/opendocument/droid/background/RecentDocumentsUtil.kt index 9c3c96871cf4..0f04cd4de649 100644 --- a/app/src/main/java/app/opendocument/droid/background/RecentDocumentsUtil.kt +++ b/app/src/main/java/app/opendocument/droid/background/RecentDocumentsUtil.kt @@ -65,6 +65,12 @@ object RecentDocumentsUtil { return update.evicted } + /** Puts a removed document back where it was, for undoing a swipe. */ + @Synchronized + fun restoreRecentDocument(context: Context, entry: RecentDocumentList.Entry, index: Int) { + write(context, RecentDocumentList.insert(read(context), entry, index)) + } + /** Drops [uri] from the list. Does nothing if it is not in it. */ @Synchronized fun removeRecentDocument(context: Context, uri: Uri) { diff --git a/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt b/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt index dc6bb8da84cc..aabe859971ea 100644 --- a/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt +++ b/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt @@ -25,6 +25,25 @@ object SnackbarHelper { ) } + /** Same, with a button that says something other than "OK" - "undo", typically. */ + fun show( + activity: Activity, + resId: Int, + buttonResId: Int, + callback: Runnable?, + isIndefinite: Boolean, + isError: Boolean, + ) { + show( + activity, + activity.getString(buttonResId), + activity.getString(resId), + callback, + isIndefinite, + isError, + ) + } + private fun show( activity: Activity, buttonText: String, diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/LandingFragment.kt b/app/src/main/java/app/opendocument/droid/ui/activity/LandingFragment.kt index cc8075007e0f..260f75db4dea 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/LandingFragment.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/LandingFragment.kt @@ -4,6 +4,7 @@ import android.app.Activity import android.content.Intent import android.net.Uri import android.os.Bundle +import android.text.format.DateUtils import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -11,11 +12,13 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.contract.ActivityResultContracts import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProvider +import androidx.recyclerview.widget.ItemTouchHelper import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView import app.opendocument.droid.R import app.opendocument.droid.background.DocumentTreeBrowser import app.opendocument.droid.background.PersistedUriPermissions +import app.opendocument.droid.background.RecentDocumentList import app.opendocument.droid.nonfree.AnalyticsConstants import app.opendocument.droid.ui.SnackbarHelper import app.opendocument.droid.ui.widget.LandingAdapter @@ -42,6 +45,10 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { private var landingVisible = true + // the entries behind the recent rows, kept so a swipe can restore the exact entry - the row + // itself only carries what it needs to draw + private var lastDocuments: List = emptyList() + /** * Back goes up a folder before it goes anywhere else. * @@ -117,6 +124,8 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { mainActivity.findDocument() } + attachSwipeToRemove() + requireActivity() .onBackPressedDispatcher .addCallback(viewLifecycleOwner, folderBackCallback) @@ -158,6 +167,8 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { private fun render(state: LandingViewModel.State) { folderBackCallback.isEnabled = landingVisible && state.location != null + lastDocuments = state.documents + val items = ArrayList() val location = state.location @@ -182,6 +193,75 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { adapter.submitList(items) } + /** + * Swiping a recent document away removes it, with an undo that puts it back at the same place. + * + * Only the recently opened documents can go: a document inside a folder is simply there, and + * the app has no business deleting anything. + */ + private fun attachSwipeToRemove() { + val callback = + object : + ItemTouchHelper.SimpleCallback( + 0, + ItemTouchHelper.START or ItemTouchHelper.END, + ) { + + override fun onMove( + recyclerView: RecyclerView, + viewHolder: RecyclerView.ViewHolder, + target: RecyclerView.ViewHolder, + ): Boolean = false + + override fun getSwipeDirs( + recyclerView: RecyclerView, + viewHolder: RecyclerView.ViewHolder, + ): Int = + if (adapter.isRemovableDocument(viewHolder.bindingAdapterPosition)) + super.getSwipeDirs(recyclerView, viewHolder) + else 0 + + override fun onSwiped(viewHolder: RecyclerView.ViewHolder, direction: Int) { + val position = viewHolder.bindingAdapterPosition + val document = adapter.documentAt(position) ?: return + + removeWithUndo(document, position) + } + } + + ItemTouchHelper(callback).attachToRecyclerView(list) + } + + private fun removeWithUndo(document: LandingItem.Document, position: Int) { + val entry = entryFor(document) ?: return + + // where it sits among the documents, which is what restoring it needs - the adapter + // position counts the section header too + val index = lastDocuments.indexOfFirst { it.uri == entry.uri } + + mainActivity.analyticsManager.report("recent_swiped_away") + + viewModel.removeRecentDocumentUndoably(document.uri) + + // the grant it was holding is deliberately not released here: an undo would need it back, + // and prune() reclaims it on the next launch anyway, which is the whole point of + // reconciling against the stored lists rather than bookkeeping every removal + SnackbarHelper.show( + requireActivity(), + R.string.landing_recent_removed, + R.string.landing_undo, + { viewModel.restoreRecentDocument(entry, index) }, + false, + false, + ) + } + + private fun entryFor(document: LandingItem.Document): RecentDocumentList.Entry? { + val uri = document.uri.toString() + + return lastDocuments.firstOrNull { it.uri == uri } + } + private fun renderRoot( items: ArrayList, state: LandingViewModel.State, @@ -196,7 +276,13 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { if (state.documents.isNotEmpty()) { items.add(LandingItem.Header(R.string.landing_section_recent)) for (document in state.documents) { - items.add(LandingItem.Document(document.filename, Uri.parse(document.uri))) + items.add( + LandingItem.Document( + document.filename, + Uri.parse(document.uri), + lastOpenedLabel(document.lastOpenedAt), + ) + ) } } @@ -220,7 +306,28 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { } items.add(LandingItem.Header(R.string.landing_section_settings)) - items.add(LandingItem.CatchAll(state.catchAllEnabled)) + items.add( + LandingItem.Setting( + LandingItem.SETTING_CATCH_ALL, + R.string.landing_catch_all_title, + R.string.landing_intro_open_all, + state.catchAllEnabled, + ) + ) + } + + /** "2 hours ago", or nothing at all for entries written before this was recorded. */ + private fun lastOpenedLabel(lastOpenedAt: Long): String? { + if (lastOpenedAt <= 0) { + return null + } + + return DateUtils.getRelativeTimeSpanString( + lastOpenedAt, + System.currentTimeMillis(), + DateUtils.MINUTE_IN_MILLIS, + ) + .toString() } private fun renderFolder( @@ -341,12 +448,16 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { mainActivity.findDocument() } - override fun onCatchAllChanged(enabled: Boolean) { - viewModel.setCatchAllEnabled(enabled) + override fun onSettingChanged(setting: Int, enabled: Boolean) { + when (setting) { + LandingItem.SETTING_CATCH_ALL -> { + viewModel.setCatchAllEnabled(enabled) - mainActivity.analyticsManager.report( - if (enabled) "catch_all_enabled" else "catch_all_disabled" - ) + mainActivity.analyticsManager.report( + if (enabled) "catch_all_enabled" else "catch_all_disabled" + ) + } + } } private val mainActivity: MainActivity diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/LandingViewModel.kt b/app/src/main/java/app/opendocument/droid/ui/activity/LandingViewModel.kt index b6043b320a0d..3399d2f7d4df 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/LandingViewModel.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/LandingViewModel.kt @@ -152,6 +152,34 @@ class LandingViewModel(application: Application) : AndroidViewModel(application) } } + /** + * Puts a swiped away document back where it was. + * + * The grant it needs is still held: [prune] only runs after a removal that the user has had the + * chance to undo, so nothing has been handed back yet. + */ + fun restoreRecentDocument(entry: RecentDocumentList.Entry, index: Int) { + executor.execute { + RecentDocumentsUtil.restoreRecentDocument(getApplication(), entry, index) + + refresh() + } + } + + /** Forgets a document without releasing its grant yet, so an undo can still put it back. */ + fun removeRecentDocumentUndoably(uri: Uri) { + executor.execute { + RecentDocumentsUtil.removeRecentDocument(getApplication(), uri) + + refresh() + } + } + + /** Hands back the grants of everything that is no longer referenced. */ + fun releaseUnusedGrants() { + executor.execute { PersistedUriPermissions.prune(getApplication()) } + } + override fun onCleared() { super.onCleared() diff --git a/app/src/main/java/app/opendocument/droid/ui/widget/LandingAdapter.kt b/app/src/main/java/app/opendocument/droid/ui/widget/LandingAdapter.kt index 15c7fc254ee2..1354c7753ab2 100644 --- a/app/src/main/java/app/opendocument/droid/ui/widget/LandingAdapter.kt +++ b/app/src/main/java/app/opendocument/droid/ui/widget/LandingAdapter.kt @@ -32,16 +32,32 @@ class LandingAdapter(private val listener: Listener) : fun onOpenClicked() - fun onCatchAllChanged(enabled: Boolean) + fun onSettingChanged(setting: Int, enabled: Boolean) } + /** Whether the row at [position] can be swiped away, i.e. whether it is a recent document. */ + fun isRemovableDocument(position: Int): Boolean { + if (position !in 0 until itemCount) { + return false + } + + val item = getItem(position) + + // a document inside a folder is not ours to forget - it is simply there. those carry no + // subtitle, which is what tells the two apart here. + return item is LandingItem.Document && item.subtitle != null + } + + fun documentAt(position: Int): LandingItem.Document? = + if (position in 0 until itemCount) getItem(position) as? LandingItem.Document else null + override fun getItemViewType(position: Int): Int = when (getItem(position)) { is LandingItem.Header -> TYPE_HEADER is LandingItem.Document -> TYPE_DOCUMENT is LandingItem.Folder -> TYPE_FOLDER is LandingItem.Action -> TYPE_ACTION - is LandingItem.CatchAll -> TYPE_CATCH_ALL + is LandingItem.Setting -> TYPE_SETTING is LandingItem.Message -> TYPE_MESSAGE is LandingItem.Empty -> TYPE_EMPTY } @@ -52,7 +68,7 @@ class LandingAdapter(private val listener: Listener) : val layout = when (viewType) { TYPE_HEADER -> R.layout.item_landing_header - TYPE_CATCH_ALL -> R.layout.item_landing_switch + TYPE_SETTING -> R.layout.item_landing_switch TYPE_MESSAGE -> R.layout.item_landing_message TYPE_EMPTY -> R.layout.item_landing_empty // documents, folders and actions are all an icon plus a label @@ -78,6 +94,7 @@ class LandingAdapter(private val listener: Listener) : is LandingItem.Document -> { holder.icon.setImageResource(R.drawable.ic_description) holder.title.text = item.filename + holder.bindSubtitle(item.subtitle) holder.itemView.setOnClickListener { listener.onDocumentClicked(item) } holder.itemView.setOnLongClickListener { listener.onDocumentRemoveRequested(item) @@ -89,6 +106,7 @@ class LandingAdapter(private val listener: Listener) : is LandingItem.Folder -> { holder.icon.setImageResource(R.drawable.ic_folder) holder.title.text = item.name + holder.bindSubtitle(null) holder.itemView.setOnClickListener { listener.onFolderClicked(item) } holder.itemView.setOnLongClickListener { listener.onFolderRemoveRequested(item) @@ -100,16 +118,20 @@ class LandingAdapter(private val listener: Listener) : is LandingItem.Action -> { holder.icon.setImageResource(item.icon) holder.title.setText(item.label) + holder.bindSubtitle(null) holder.itemView.setOnClickListener { listener.onActionClicked(item.action) } holder.itemView.setOnLongClickListener(null) } - is LandingItem.CatchAll -> { + is LandingItem.Setting -> { + holder.title.setText(item.title) + holder.subtitle.setText(item.body) + // set the state before the listener, so restoring it does not report a change holder.switch.setOnCheckedChangeListener(null) holder.switch.isChecked = item.checked holder.switch.setOnCheckedChangeListener { _, isChecked -> - listener.onCatchAllChanged(isChecked) + listener.onSettingChanged(item.setting, isChecked) } holder.itemView.setOnClickListener { holder.switch.toggle() } @@ -120,9 +142,15 @@ class LandingAdapter(private val listener: Listener) : class ViewHolder(view: View) : RecyclerView.ViewHolder(view) { val icon: ImageView by lazy { view.findViewById(R.id.landing_row_icon) } val title: TextView by lazy { view.findViewById(R.id.landing_row_title) } + val subtitle: TextView by lazy { view.findViewById(R.id.landing_row_subtitle) } val switch: MaterialSwitch by lazy { view.findViewById(R.id.landing_row_switch) } val open: View by lazy { view.findViewById(R.id.landing_empty_open) } val addFolder: View by lazy { view.findViewById(R.id.landing_empty_add_folder) } + + fun bindSubtitle(text: String?) { + subtitle.text = text + subtitle.visibility = if (text == null) View.GONE else View.VISIBLE + } } private companion object { @@ -130,7 +158,7 @@ class LandingAdapter(private val listener: Listener) : const val TYPE_DOCUMENT = 1 const val TYPE_FOLDER = 2 const val TYPE_ACTION = 3 - const val TYPE_CATCH_ALL = 4 + const val TYPE_SETTING = 4 const val TYPE_MESSAGE = 5 const val TYPE_EMPTY = 6 @@ -146,12 +174,13 @@ class LandingAdapter(private val listener: Listener) : ): Boolean = when { oldItem is LandingItem.Document && newItem is LandingItem.Document -> - oldItem.filename == newItem.filename + oldItem.filename == newItem.filename && + oldItem.subtitle == newItem.subtitle oldItem is LandingItem.Folder && newItem is LandingItem.Folder -> oldItem.name == newItem.name - oldItem is LandingItem.CatchAll && newItem is LandingItem.CatchAll -> + oldItem is LandingItem.Setting && newItem is LandingItem.Setting -> oldItem.checked == newItem.checked // headers, messages, actions and the empty state are fully described diff --git a/app/src/main/java/app/opendocument/droid/ui/widget/LandingItem.kt b/app/src/main/java/app/opendocument/droid/ui/widget/LandingItem.kt index 7de80b17d34c..c2ee2ac007db 100644 --- a/app/src/main/java/app/opendocument/droid/ui/widget/LandingItem.kt +++ b/app/src/main/java/app/opendocument/droid/ui/widget/LandingItem.kt @@ -13,7 +13,9 @@ sealed class LandingItem { override val id: String = "header:$title" } - class Document(val filename: String, val uri: Uri) : LandingItem() { + /** [subtitle] is the second line, when there is something worth saying - a last opened time. */ + class Document(val filename: String, val uri: Uri, val subtitle: String? = null) : + LandingItem() { override val id: String = "document:$uri" } @@ -26,8 +28,14 @@ sealed class LandingItem { override val id: String = "action:$action" } - class CatchAll(val checked: Boolean) : LandingItem() { - override val id: String = "catch_all" + /** A switch under the settings header. */ + class Setting( + val setting: Int, + @param:StringRes val title: Int, + @param:StringRes val body: Int, + val checked: Boolean, + ) : LandingItem() { + override val id: String = "setting:$setting" } /** An explanatory line, used for the empty states of a section. */ @@ -48,5 +56,7 @@ sealed class LandingItem { companion object { const val ACTION_ADD_FOLDER: Int = 1 const val ACTION_UP: Int = 2 + + const val SETTING_CATCH_ALL: Int = 1 } } diff --git a/app/src/main/res/layout/item_landing_row.xml b/app/src/main/res/layout/item_landing_row.xml index d155ed7db060..3749b1da069e 100644 --- a/app/src/main/res/layout/item_landing_row.xml +++ b/app/src/main/res/layout/item_landing_row.xml @@ -1,10 +1,11 @@ - + - + android:paddingBottom="12dp"> + + + + + diff --git a/app/src/main/res/layout/item_landing_switch.xml b/app/src/main/res/layout/item_landing_switch.xml index 10b3eb286fb7..ddaade227adb 100644 --- a/app/src/main/res/layout/item_landing_switch.xml +++ b/app/src/main/res/layout/item_landing_switch.xml @@ -18,17 +18,17 @@ android:orientation="vertical"> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 70b18724dc2a..297d0c0d5a96 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -36,22 +36,24 @@ Save Upload Edit your document below and press Save + By default this app only offers to open document files. If another app like \"Samsung My Files\" won\'t let you open a document here, turn this on to register for all file types. Recent + Folders Settings Open all file types + Removed from recent + Undo Open a file - Folders Add a folder… This folder can\'t be remembered. Try another one. Remove folder Stop showing this folder here? The folder and the documents in it are not deleted. - No documents in this folder. - Up one level Remove Remove this document from the recent list? The document itself is not deleted. + No documents in this folder. + Up one level Nothing here yet Open a document to get started. It will show up here next time. - By default this app only offers to open document files. If another app like \"Samsung My Files\" won\'t let you open a document here, turn this on to register for all file types. Reading… Initializing TTS… Ready! diff --git a/app/src/test/java/app/opendocument/droid/background/RecentDocumentListTest.kt b/app/src/test/java/app/opendocument/droid/background/RecentDocumentListTest.kt index aea84b00dedd..bb306c674c0d 100644 --- a/app/src/test/java/app/opendocument/droid/background/RecentDocumentListTest.kt +++ b/app/src/test/java/app/opendocument/droid/background/RecentDocumentListTest.kt @@ -96,5 +96,52 @@ class RecentDocumentListTest { assertTrue(RecentDocumentList.remove(emptyList(), "a").isEmpty()) } + @Test + fun insertPutsADocumentBackWhereItWas() { + val current = listOf(entry("a"), entry("c")) + + val restored = RecentDocumentList.insert(current, entry("b"), 1) + + assertEquals(listOf("a", "b", "c"), restored.map { it.uri }) + } + + @Test + fun insertAtTheFrontAndAtTheEndBothWork() { + val current = listOf(entry("a"), entry("b")) + + assertEquals( + listOf("x", "a", "b"), + RecentDocumentList.insert(current, entry("x"), 0).map { it.uri }, + ) + assertEquals( + listOf("a", "b", "x"), + RecentDocumentList.insert(current, entry("x"), 2).map { it.uri }, + ) + } + + @Test + fun insertClampsAnIndexPastTheEnd() { + val current = listOf(entry("a")) + + assertEquals( + listOf("a", "x"), + RecentDocumentList.insert(current, entry("x"), 99).map { it.uri }, + ) + assertEquals( + listOf("x", "a"), + RecentDocumentList.insert(current, entry("x"), -5).map { it.uri }, + ) + } + + @Test + fun insertDoesNotDuplicateAKnownUri() { + val current = listOf(entry("a"), entry("b")) + + assertEquals( + listOf("b", "a"), + RecentDocumentList.insert(current, entry("a"), 1).map { it.uri }, + ) + } + private fun entry(uri: String) = RecentDocumentList.Entry("name of $uri", uri, 0) }