From 0b6bf616a334701f88c6abfc0e379a5587e3336b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 19:32:10 +0200 Subject: [PATCH 01/24] Move to Material 3 with real night resources (#524) MainTheme was Theme.AppCompat.Light with five hardcoded hex values, copied in full into values-v29 and values-v35 so that the qualified variants had already drifted apart. Dark mode was the OS force-dark hack turned on with android:forceDarkAllowed, not a resource set of our own. The theme is now Theme.Material3.DayNight over named colour roles, with a values-night counterpart. Base.MainTheme holds everything and the qualified variants only add what is specific to them, so there is no copy to drift. The inverse and tertiary roles are set explicitly: leave them out and snackbars fall back to material's purple baseline. uiMode comes out of configChanges. Swallowing it meant the activity was never recreated on a dark mode toggle, so already-inflated views kept their day colours until the next launch - which would have made the whole migration look half-applied. Documents stay on the background they were authored against. Algorithmic darkening never fired under the old light theme, and at targetSdk 33 and up it starts the moment the app theme reports itself as dark; inverting an odt full of tables and coloured text is not a change to make silently. ProgressDialogFragment moves off the framework ProgressDialog, which resolves android:alertDialogTheme and would have stayed light. Co-authored-by: Claude Opus 5 (1M context) --- app/src/main/AndroidManifest.xml | 8 ++- .../opendocument/droid/ui/SnackbarHelper.kt | 13 ++++- .../opendocument/droid/ui/widget/PageView.kt | 24 +++++--- .../droid/ui/widget/ProgressDialogFragment.kt | 23 ++++---- app/src/main/res/drawable/ic_edit.xml | 2 +- app/src/main/res/drawable/ic_folder_open.xml | 2 +- app/src/main/res/drawable/ic_fullscreen.xml | 2 +- .../res/drawable/ic_keyboard_arrow_down.xml | 2 +- .../res/drawable/ic_keyboard_arrow_up.xml | 2 +- app/src/main/res/drawable/ic_pause.xml | 2 +- app/src/main/res/drawable/ic_play_arrow.xml | 2 +- app/src/main/res/drawable/ic_save.xml | 2 +- app/src/main/res/drawable/ic_search.xml | 2 +- app/src/main/res/drawable/ic_skip_next.xml | 2 +- .../main/res/drawable/ic_skip_previous.xml | 2 +- app/src/main/res/layout/dialog_progress.xml | 24 ++++++++ app/src/main/res/layout/main.xml | 3 +- app/src/main/res/values-night/bools.xml | 6 ++ app/src/main/res/values-night/colors.xml | 38 +++++++++++++ app/src/main/res/values-v29/themes.xml | 12 ---- app/src/main/res/values-v35/themes.xml | 21 +++---- app/src/main/res/values/bools.xml | 10 ++++ app/src/main/res/values/colors.xml | 51 +++++++++++++++++ app/src/main/res/values/themes.xml | 56 ++++++++++++++++--- 24 files changed, 246 insertions(+), 65 deletions(-) create mode 100644 app/src/main/res/layout/dialog_progress.xml create mode 100644 app/src/main/res/values-night/bools.xml create mode 100644 app/src/main/res/values-night/colors.xml delete mode 100644 app/src/main/res/values-v29/themes.xml create mode 100644 app/src/main/res/values/bools.xml create mode 100644 app/src/main/res/values/colors.xml diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index f85dfdccc660..24cedbd452fd 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -72,9 +72,15 @@ activity-alias name does not have to correspond to a real class, so the old names live on as aliases pointing at the relocated activity. --> + + + + #86CFFF + #00344C + #004C6D + #C7E7FF + + #B6C9D8 + #21323E + #384955 + #D2E5F5 + + #191C1E + #E2E2E5 + #41484D + #C1C7CE + #1D2022 + + #CDC0E9 + #342B4B + #4B4163 + #E9DDFF + + #E2E2E5 + #2E3133 + #00658F + + #8B9198 + #41484D + + #FFB4AB + #690005 + #93000A + #FFDAD6 + + diff --git a/app/src/main/res/values-v29/themes.xml b/app/src/main/res/values-v29/themes.xml deleted file mode 100644 index 53dbc731bec3..000000000000 --- a/app/src/main/res/values-v29/themes.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - \ No newline at end of file diff --git a/app/src/main/res/values-v35/themes.xml b/app/src/main/res/values-v35/themes.xml index f4136c3da446..0b1a0c48bff3 100644 --- a/app/src/main/res/values-v35/themes.xml +++ b/app/src/main/res/values-v35/themes.xml @@ -1,17 +1,14 @@ - + diff --git a/app/src/main/res/values/bools.xml b/app/src/main/res/values/bools.xml new file mode 100644 index 000000000000..4c1d84b4af9e --- /dev/null +++ b/app/src/main/res/values/bools.xml @@ -0,0 +1,10 @@ + + + + + true + + diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml new file mode 100644 index 000000000000..082052a015f3 --- /dev/null +++ b/app/src/main/res/values/colors.xml @@ -0,0 +1,51 @@ + + + + + #00658F + #FFFFFF + #C7E7FF + #001E2F + + #4F616E + #FFFFFF + #D2E5F5 + #0B1D29 + + #FCFCFF + #191C1E + #DDE3EA + #41484D + #ECEFF3 + + #63597C + #FFFFFF + #E9DDFF + #1F1635 + + + #2E3133 + #F0F1F3 + #86CFFF + + #71787E + #C1C7CE + + #BA1A1A + #FFFFFF + #FFDAD6 + #410002 + + + #FFFFFF + + diff --git a/app/src/main/res/values/themes.xml b/app/src/main/res/values/themes.xml index aefcfee01400..25eb44bca7b9 100644 --- a/app/src/main/res/values/themes.xml +++ b/app/src/main/res/values/themes.xml @@ -1,12 +1,54 @@ - - \ No newline at end of file + - - - - - \ No newline at end of file diff --git a/app/src/main/res/values-normal/styles.xml b/app/src/main/res/values-normal/styles.xml deleted file mode 100644 index 9c89871557b2..000000000000 --- a/app/src/main/res/values-normal/styles.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/app/src/main/res/values-small/styles.xml b/app/src/main/res/values-small/styles.xml deleted file mode 100644 index 89896162bb10..000000000000 --- a/app/src/main/res/values-small/styles.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/app/src/main/res/values-xlarge/styles.xml b/app/src/main/res/values-xlarge/styles.xml deleted file mode 100644 index c2a843b39726..000000000000 --- a/app/src/main/res/values-xlarge/styles.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index e31b696c94be..769d8f0577d0 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -40,10 +40,14 @@ Save Upload Edit your document below and press Save - Welcome to \nOpenDocument Reader - OpenDocument Reader allows you to view documents that are stored in OpenDocument format (.odt, .ods, .odp and .odg) wherever you are. - With OpenDocument Reader you can read and search through your documents in a breeze. - Found one last typo left to fix ahead of your big presentation? It now supports modifications too! + Recent + Settings + Open all file types + Open a file + Remove + Remove this document from the recent list? The document itself is not deleted. + 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… diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml deleted file mode 100644 index d8f7dca1de56..000000000000 --- a/app/src/main/res/values/styles.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 3d9b640d9d24..63aa2d876f3a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -13,6 +13,8 @@ androidxAnnotation = "1.10.0" androidxAppcompat = "1.7.1" androidxCore = "1.19.0" androidxWebkit = "1.16.0" +androidxRecyclerview = "1.4.0" +androidxLifecycle = "2.9.4" material = "1.14.0" playServicesAds = "25.4.0" @@ -30,6 +32,12 @@ androidx-annotation ={ module = "androidx.annotation:annotation", version.ref = androidx-appcompat = { module = "androidx.appcompat:appcompat", version.ref = "androidxAppcompat" } androidx-core = { module = "androidx.core:core", version.ref = "androidxCore" } androidx-webkit = { module = "androidx.webkit:webkit", version.ref = "androidxWebkit" } +# recyclerview and lifecycle already arrive transitively through material, but the landing +# screen and DocumentFragment's ViewModel compile against them - declaring them keeps a +# material bump from silently taking a compile dependency away +androidx-recyclerview = { module = "androidx.recyclerview:recyclerview", version.ref = "androidxRecyclerview" } +androidx-lifecycle-viewmodel = { module = "androidx.lifecycle:lifecycle-viewmodel", version.ref = "androidxLifecycle" } +androidx-lifecycle-livedata = { module = "androidx.lifecycle:lifecycle-livedata", version.ref = "androidxLifecycle" } material = { module = "com.google.android.material:material", version.ref = "material" } play-services-ads = { module = "com.google.android.gms:play-services-ads", version.ref = "playServicesAds" } From 29ebda2992e7e5507c5a291c23bcdd5cf2cc470c Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 19:48:04 +0200 Subject: [PATCH 03/24] Open the system picker directly (#526) findDocument queried every activity answering ACTION_OPEN_DOCUMENT and raised an "Open document via:" dialog to choose between them, before the picker it was going to launch anyway. That is a tap that buys nothing: the system picker already browses Drive, Downloads, a usb stick and every installed file manager, and it is the ui people know. Its last row used to be the recently opened documents, which is now the landing screen itself, so the dialog has nothing left that is its own. The entries stay. They were there for queryIntentActivities, but removing package visibility is exactly the kind of change that breaks on one OEM and nowhere else. MainActivityTests drove the picker through the dialog, so it loses that step; the intent stubbing is untouched, because Espresso-Intents intercepts at execStartActivity, below ActivityResultLauncher. Co-authored-by: Claude Opus 5 (1M context) --- .../opendocument/droid/test/LandingTests.kt | 36 +++++++++++ .../droid/test/MainActivityTests.kt | 13 +--- .../droid/ui/activity/MainActivity.kt | 64 +++++++------------ app/src/main/res/values/strings.xml | 4 -- 4 files changed, 60 insertions(+), 57 deletions(-) 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 e526011dd7da..2de747335a94 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt @@ -3,6 +3,8 @@ package app.opendocument.droid.test +import android.app.Activity +import android.app.Instrumentation import android.content.Context import android.content.Intent import android.net.Uri @@ -12,6 +14,7 @@ import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.intent.Intents +import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction import androidx.test.espresso.matcher.ViewMatchers.isDisplayed import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText @@ -70,6 +73,30 @@ class LandingTests { onView(withId(R.id.landing_empty)).check(matches(isDisplayed())) } + @Test + fun emptyStateOpensTheSystemPicker() { + stubOpenDocumentCancelled() + + launch() + + onView(withId(R.id.landing_empty_open)).perform(click()) + + 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() @@ -115,6 +142,15 @@ class LandingTests { return activity } + /** + * The picker is stubbed as cancelled: these tests are about what the landing screen asks for, + * not about loading a document. + */ + private fun stubOpenDocumentCancelled() { + Intents.intending(hasAction(Intent.ACTION_OPEN_DOCUMENT)) + .respondWith(Instrumentation.ActivityResult(Activity.RESULT_CANCELED, null)) + } + private fun seedRecentDocument() { RecentDocumentsUtil.addRecentDocument(context(), TEST_DOCUMENT, uriOf(requireTestFile())) } diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt index 67dd71ca98fa..8f200797694f 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt @@ -48,7 +48,6 @@ import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicReference import org.hamcrest.Matchers.allOf -import org.hamcrest.Matchers.anyOf import org.hamcrest.Matchers.equalTo import org.junit.After import org.junit.AfterClass @@ -254,6 +253,8 @@ class MainActivityTests { ) } + // the toolbar action now launches ACTION_OPEN_DOCUMENT directly, which the test stubs out. + // it used to raise a chooser of our own that a file manager had to be picked from first. private fun openDocumentThroughPicker() { onView( allOf( @@ -263,16 +264,6 @@ class MainActivityTests { ) ) .perform(click()) - - // The menu item could be either Documents or Files. - onView( - allOf( - withId(android.R.id.text1), - anyOf(withText("Documents"), withText("Files")), - isDisplayed(), - ) - ) - .perform(click()) } private fun clickEditWithOverflowFallback() { diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt index 0b3ec7a8f3bf..7656b7466ac1 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt @@ -5,7 +5,6 @@ import android.content.ActivityNotFoundException import android.content.ComponentName import android.content.Intent import android.content.ServiceConnection -import android.content.pm.ResolveInfo import android.content.res.Configuration import android.net.Uri import android.os.Bundle @@ -20,7 +19,6 @@ import android.view.View import android.widget.LinearLayout import androidx.activity.OnBackPressedCallback import androidx.activity.result.contract.ActivityResultContracts -import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AppCompatActivity import androidx.core.view.MenuProvider import androidx.core.view.ViewCompat @@ -641,51 +639,33 @@ class MainActivity : AppCompatActivity(), MenuProvider { intent.addFlags(Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION) intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) - val packageManager = packageManager - val targetList: List = - packageManager.queryIntentActivities(intent, 0).filter { target -> - target.activityInfo.packageName == packageName || target.activityInfo.exported - } - - // the recently opened documents used to be the last row here; they are the landing - // screen itself now - val targetNames = targetList.map { it.loadLabel(packageManager).toString() }.toTypedArray() - - val builder = AlertDialog.Builder(this) - builder.setTitle(R.string.dialog_choose_filemanager) - builder.setItems(targetNames) { dialog, which -> - val target = targetList[which] - - intent.component = - ComponentName(target.activityInfo.packageName, target.activityInfo.name) - - try { - OpenFileIdling.increment() - - openDocumentLauncher.launch(intent) - } catch (e: Exception) { - OpenFileIdling.decrement() + // straight to the system picker. this used to put an "Open document via:" dialog of our + // own in front of it, listing every app that answers ACTION_OPEN_DOCUMENT - an extra tap + // that duplicated what the picker itself already offers, since it can browse Drive, + // Downloads, a usb stick and every installed file manager on its own. + try { + OpenFileIdling.increment() - crashManager.log(e) + openDocumentLauncher.launch(intent) + } catch (e: ActivityNotFoundException) { + OpenFileIdling.decrement() - SnackbarHelper.show( - this, - R.string.crouton_error_open_app, - { findDocument() }, - true, - true, - ) - } + crashManager.log(e) - analyticsManager.report( - AnalyticsConstants.EVENT_SELECT_CONTENT, - AnalyticsConstants.PARAM_CONTENT_TYPE, - target.activityInfo.packageName, + SnackbarHelper.show( + this, + R.string.crouton_error_open_app, + { findDocument() }, + true, + true, ) - - dialog.dismiss() } - builder.show() + + analyticsManager.report( + AnalyticsConstants.EVENT_SELECT_CONTENT, + AnalyticsConstants.PARAM_CONTENT_TYPE, + "picker", + ) } override fun onPause() { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 769d8f0577d0..e9faa9d8e9a8 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -11,15 +11,11 @@ File could not be saved. Please contact support@opendocument.app Device out of memory! Too many pictures, or file too big. This document is password-protected - Recent documents - Open document via: Loading… Uploading… Please wait, this could take a few minutes. Uploaded files are private and automatically deleted after 24 hours. We aren\'t able to open this document, because we don\'t support its format. Do you want to upload it to our server temporarily, so we can display it for you anyway? Uploaded files are private and automatically deleted after 24 hours. - No documents found - Recently opened documents Search in document Open document Open with... From ad75abd5f8e020c9aeb45c49b65afd6aa9c5e203 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:00:50 +0200 Subject: [PATCH 04/24] Browse the folders you have granted access to (#527) "See the files on my phone" has no permission behind it any more. READ_EXTERNAL_STORAGE has not reached documents since scoped storage, READ_MEDIA_* covers only images, video and audio, and Play restricts MANAGE_EXTERNAL_STORAGE to file managers and backup apps - a document viewer asking for it gets the listing rejected. The manifest still declares nothing but INTERNET, and it stays that way. ACTION_OPEN_DOCUMENT_TREE does the same job with the user's consent instead: they pick a folder once, we persist a read only grant, and the landing screen lists and navigates it from then on. Read only - no write flag is ever requested. Listing goes through DocumentsContract rather than androidx's DocumentFile, which issues a fresh query per child the moment a name or type is read. One cursor covers a whole folder. SupportedDocumentTypes decides what is worth showing. It has to guess: MetadataLoader only knows what is supported after libmagic has run over the cached file, and a folder listing has nothing but a name and whatever mime type the provider volunteered - which is regularly application/octet-stream, hence the extension fallback. It mirrors the STRICT_CATCH intent filter and nothing keeps the two in step, so both sides now say so, as does CLAUDE.md. prune() learns that a document below a granted tree is covered by that tree's grant, which it can tell from the uri alone - so nothing has to record where a document was opened from. The tree grant is taken through PersistedUriPermissions rather than the resolver directly, so that it joins the in flight set the recent list's grants already use: addFolderTree writes the tree down on the view model's executor, and a prune in that window would find a grant nothing points at yet and hand it straight back. Same race, same fix, one folder instead of one document. That decision is now isReferenced(), which takes the strings rather than a context and so can be covered by a plain jvm test, with the release half lifted out beside it to keep prune() down to the loop it is. Co-authored-by: Claude Opus 5 (1M context) --- CLAUDE.md | 27 +++ .../opendocument/droid/test/LandingTests.kt | 32 +++ .../droid/background/DocumentTreeBrowser.kt | 125 ++++++++++ .../droid/background/FolderTreesUtil.kt | 125 ++++++++++ .../background/PersistedUriPermissions.kt | 78 ++++--- .../background/SupportedDocumentTypes.kt | 67 ++++++ .../droid/ui/activity/LandingFragment.kt | 216 +++++++++++++++++- .../droid/ui/activity/LandingViewModel.kt | 89 +++++++- .../droid/ui/widget/LandingAdapter.kt | 43 +++- .../droid/ui/widget/LandingItem.kt | 14 ++ app/src/main/res/drawable/ic_arrow_upward.xml | 10 + app/src/main/res/layout/fragment_landing.xml | 8 + app/src/main/res/values/strings.xml | 7 + .../background/PersistedUriPermissionsTest.kt | 30 ++- .../background/SupportedDocumentTypesTest.kt | 71 ++++++ 15 files changed, 891 insertions(+), 51 deletions(-) create mode 100644 app/src/main/java/app/opendocument/droid/background/DocumentTreeBrowser.kt create mode 100644 app/src/main/java/app/opendocument/droid/background/FolderTreesUtil.kt create mode 100644 app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt create mode 100644 app/src/main/res/drawable/ic_arrow_upward.xml create mode 100644 app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt diff --git a/CLAUDE.md b/CLAUDE.md index 897ac0ac0f12..95859fd9d9f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -145,6 +145,33 @@ the `.pro` suffix) differ on purpose - do not "fix" the mismatch: `AndroidFileCache`, the SharedPreferences name in `MainActivity`) follows `applicationId` and must stay that way, or existing users lose their saved prefs. +### Supported file types are declared twice + +`SupportedDocumentTypes` (mime prefixes plus an extension fallback) and the `STRICT_CATCH` +`activity-alias` in `AndroidManifest.xml` describe the same set from two directions - the manifest +is what the system offers the app for, `SupportedDocumentTypes` is what the folder browser on the +landing screen offers itself for. **Nothing keeps them in step, so change both.** + +The duplication is deliberate: `MetadataLoader` decides what is really supported by running +libmagic over the file *after* copying it into the cache, and `CoreLoader.isSupported()` / +`RawLoader.isSupported()` both take a filled-in `FileLoader.Options`. A folder listing only has a +name and whatever mime type the provider volunteered, so it cannot ask them and has to guess. + +### Storage access + +The app declares **no storage permission at all** - only `INTERNET` - and it has to stay that way. +`READ_EXTERNAL_STORAGE` has not reached documents since scoped storage, `READ_MEDIA_*` only covers +images/video/audio, and Play restricts `MANAGE_EXTERNAL_STORAGE` to file managers and backup apps, +so a document viewer asking for it gets the listing rejected. + +Everything therefore goes through SAF: `ACTION_OPEN_DOCUMENT` for a single file and +`ACTION_OPEN_DOCUMENT_TREE` (read only, never `FLAG_GRANT_WRITE_URI_PERMISSION`) for the folders +the landing screen browses. Those grants have to be persisted to survive a restart - +`PersistedUriPermissions` takes them and reclaims them by reconciling against the recent list and +the granted trees, rather than releasing on close. Do not add a release next to a +`documentFragment.loadUri()`: that call only queues the load onto `LoaderService`, so the stream is +opened long after it returns. + ### Language Kotlin; support is built into AGP 9, no kotlin plugin is applied. The only java left is 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 2de747335a94..64167ba9f6b7 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt @@ -29,6 +29,7 @@ import app.opendocument.droid.ui.activity.MainActivity import java.io.File import java.io.FileOutputStream import java.io.InputStream +import org.hamcrest.Matchers.allOf import org.junit.After import org.junit.Assert import org.junit.Before @@ -120,6 +121,32 @@ class LandingTests { ) } + @Test + fun addingAFolderAsksTheSystemForATree() { + Intents.intending(hasAction(Intent.ACTION_OPEN_DOCUMENT_TREE)) + .respondWith(Instrumentation.ActivityResult(Activity.RESULT_CANCELED, null)) + + launch() + + onView(withId(R.id.landing_empty_add_folder)).perform(click()) + + Intents.intended(hasAction(Intent.ACTION_OPEN_DOCUMENT_TREE)) + } + + @Test + fun theFoldersSectionOffersToAddOne() { + seedRecentDocument() + + launch() + + onView(withText(R.string.landing_section_folders)).check(matches(isDisplayed())) + + // the empty state carries a button with the same label, so match the one on screen - + // the empty state is gone once there is a recent document to show + onView(allOf(withText(R.string.landing_add_folder), isDisplayed())) + .check(matches(isDisplayed())) + } + @Test fun theCatchAllSettingIsOffered() { seedRecentDocument() @@ -155,8 +182,13 @@ class LandingTests { RecentDocumentsUtil.addRecentDocument(context(), TEST_DOCUMENT, uriOf(requireTestFile())) } + /** + * Both stores, not just the recent documents: a folder granted on this device - by a previous + * run, or by hand - would keep the empty state from ever being shown. + */ private fun clearLandingState() { context().deleteFile("recent_documents.json") + context().deleteFile("folder_trees.json") } private fun waitForDocumentFragment(activity: MainActivity): Boolean { diff --git a/app/src/main/java/app/opendocument/droid/background/DocumentTreeBrowser.kt b/app/src/main/java/app/opendocument/droid/background/DocumentTreeBrowser.kt new file mode 100644 index 000000000000..48b45d430cfc --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/background/DocumentTreeBrowser.kt @@ -0,0 +1,125 @@ +package app.opendocument.droid.background + +import android.content.Context +import android.net.Uri +import android.provider.DocumentsContract + +/** + * Lists what is inside a directory tree the user granted access to. + * + * Uses [DocumentsContract] rather than androidx's DocumentFile: DocumentFile.listFiles() issues a + * fresh query per child as soon as the name or type is asked for, which is one binder round trip + * per file. One cursor over the whole folder is what this needs instead. + */ +object DocumentTreeBrowser { + + class Child( + val documentId: String, + val displayName: String, + val mimeType: String, + val lastModified: Long, + ) { + val isDirectory: Boolean + get() = mimeType == DocumentsContract.Document.MIME_TYPE_DIR + } + + /** + * The supported documents and the sub directories of [parentDocumentId], directories first and + * then by name. + * + * Does binder work, so it must not run on the main thread. + */ + fun listChildren(context: Context, treeUri: Uri, parentDocumentId: String): List { + val childrenUri = + DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, parentDocumentId) + + val children = ArrayList() + + try { + context.contentResolver + .query( + childrenUri, + arrayOf( + DocumentsContract.Document.COLUMN_DOCUMENT_ID, + DocumentsContract.Document.COLUMN_DISPLAY_NAME, + DocumentsContract.Document.COLUMN_MIME_TYPE, + DocumentsContract.Document.COLUMN_LAST_MODIFIED, + ), + null, + null, + null, + ) + .use { cursor -> + if (cursor == null) { + return emptyList() + } + + while (cursor.moveToNext()) { + val documentId = cursor.getString(0) ?: continue + val displayName = cursor.getString(1) ?: continue + val mimeType = cursor.getString(2) ?: "" + + // dotfiles are not something anyone browses for a document + if (displayName.startsWith(".")) { + continue + } + + val isDirectory = mimeType == DocumentsContract.Document.MIME_TYPE_DIR + if ( + !isDirectory && + !SupportedDocumentTypes.isSupported(mimeType, displayName) + ) { + continue + } + + children.add(Child(documentId, displayName, mimeType, cursor.getLong(3))) + } + } + } catch (e: Exception) { + // a revoked grant, an unmounted sd card or a provider that died - nothing to list + return emptyList() + } + + children.sortWith( + compareBy { !it.isDirectory } + .thenBy(String.CASE_INSENSITIVE_ORDER) { + it.displayName + } + ) + + return children + } + + /** The document id of the folder the user picked, i.e. the root of the tree. */ + fun rootDocumentId(treeUri: Uri): String = DocumentsContract.getTreeDocumentId(treeUri) + + fun documentUri(treeUri: Uri, documentId: String): Uri = + DocumentsContract.buildDocumentUriUsingTree(treeUri, documentId) + + /** The name the provider gives the folder itself, falling back to its last path segment. */ + fun displayNameOf(context: Context, treeUri: Uri): String { + val documentUri = documentUri(treeUri, rootDocumentId(treeUri)) + + try { + context.contentResolver + .query( + documentUri, + arrayOf(DocumentsContract.Document.COLUMN_DISPLAY_NAME), + null, + null, + null, + ) + .use { cursor -> + if (cursor != null && cursor.moveToFirst()) { + cursor.getString(0)?.let { + return it + } + } + } + } catch (e: Exception) { + // fall through to the uri + } + + return treeUri.lastPathSegment ?: treeUri.toString() + } +} diff --git a/app/src/main/java/app/opendocument/droid/background/FolderTreesUtil.kt b/app/src/main/java/app/opendocument/droid/background/FolderTreesUtil.kt new file mode 100644 index 000000000000..2deecb2e745a --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/background/FolderTreesUtil.kt @@ -0,0 +1,125 @@ +package app.opendocument.droid.background + +import android.content.Context +import android.net.Uri +import java.io.BufferedReader +import java.io.InputStreamReader +import java.io.OutputStreamWriter +import java.nio.charset.StandardCharsets +import org.json.JSONArray +import org.json.JSONObject + +/** + * The folders the user granted access to, in the order they added them. + * + * A json file rather than shared preferences because the order matters and every entry carries a + * cached display name, which a StringSet could not express. Same shape as [RecentDocumentsUtil]. + */ +object FolderTreesUtil { + + /** + * How many folders to keep. Each holds a persisted uri permission, and those are capped per + * package - together with [RecentDocumentList.MAX_ENTRIES] this stays far below the limit. + */ + const val MAX_TREES: Int = 8 + + private const val FILENAME = "folder_trees.json" + + private const val KEY_URI = "uri" + private const val KEY_DISPLAY_NAME = "displayName" + private const val KEY_ADDED_AT = "addedAt" + + class FolderTree(val uri: Uri, val displayName: String, val addedAt: Long) + + @Synchronized fun getFolderTrees(context: Context): List = read(context) + + /** + * Adds [uri], or refreshes the name of a folder that is already there. + * + * @return the folders that fell out, so their grants can be released. + */ + @Synchronized + fun addFolderTree(context: Context, uri: Uri, displayName: String): List { + val value = uri.toString() + + val remaining = read(context).filter { it.uri.toString() != value } + val trees = ArrayList(remaining.size + 1) + trees.addAll(remaining) + trees.add(FolderTree(uri, displayName, System.currentTimeMillis())) + + if (trees.size <= MAX_TREES) { + write(context, trees) + + return emptyList() + } + + // oldest first, so the ones that drop off the front are the least recently added + val evicted = trees.subList(0, trees.size - MAX_TREES).toList() + write(context, trees.subList(trees.size - MAX_TREES, trees.size).toList()) + + return evicted + } + + @Synchronized + fun removeFolderTree(context: Context, uri: Uri) { + val value = uri.toString() + + val current = read(context) + val remaining = current.filter { it.uri.toString() != value } + + if (remaining.size != current.size) { + write(context, remaining) + } + } + + private fun read(context: Context): List { + val jsonArray = + try { + context.openFileInput(FILENAME).use { input -> + BufferedReader(InputStreamReader(input, StandardCharsets.UTF_8)).use { reader -> + JSONArray(reader.readText()) + } + } + } catch (e: Exception) { + return emptyList() + } + + val trees = ArrayList(jsonArray.length()) + for (i in 0 until jsonArray.length()) { + val tree = jsonArray.optJSONObject(i) ?: continue + + val uri = tree.optString(KEY_URI, "") + if (uri.isEmpty()) { + continue + } + + trees.add( + FolderTree( + Uri.parse(uri), + tree.optString(KEY_DISPLAY_NAME, uri), + tree.optLong(KEY_ADDED_AT, 0), + ) + ) + } + + return trees + } + + private fun write(context: Context, trees: List) { + val jsonArray = JSONArray() + for (tree in trees) { + val json = JSONObject() + json.put(KEY_URI, tree.uri.toString()) + json.put(KEY_DISPLAY_NAME, tree.displayName) + json.put(KEY_ADDED_AT, tree.addedAt) + + jsonArray.put(json) + } + + context.openFileOutput(FILENAME, Context.MODE_PRIVATE).use { output -> + OutputStreamWriter(output, StandardCharsets.UTF_8).use { writer -> + writer.write(jsonArray.toString()) + } + } + } +} diff --git a/app/src/main/java/app/opendocument/droid/background/PersistedUriPermissions.kt b/app/src/main/java/app/opendocument/droid/background/PersistedUriPermissions.kt index 13561deaffbc..c353908be7f1 100644 --- a/app/src/main/java/app/opendocument/droid/background/PersistedUriPermissions.kt +++ b/app/src/main/java/app/opendocument/droid/background/PersistedUriPermissions.kt @@ -2,6 +2,7 @@ package app.opendocument.droid.background import android.content.Context import android.content.Intent +import android.content.UriPermission import android.net.Uri /** @@ -17,16 +18,17 @@ object PersistedUriPermissions { private const val READ_FLAG = Intent.FLAG_GRANT_READ_URI_PERMISSION /** - * The grants taken during this process that the recent list does not name yet. + * The grants taken during this process that nothing on disk names yet. * * Taking a grant and writing down what needs it are not one step: a document only reaches the - * list once [MetadataLoader] has read it, which is long after [takeRead] ran - `loadUri` merely - * queues the load. A [prune] in that window would enumerate the fresh grant, find nothing - * referring to it and hand it straight back, leaving the entry that is about to be written - * unreadable on the next launch - the very failure releasing on close used to cause. + * recent list once [MetadataLoader] has read it, which is long after [takeRead] ran - `loadUri` + * merely queues the load - and a folder only reaches the tree list once the landing view + * model's executor gets to it. A [prune] in either window would enumerate the fresh grant, find + * nothing referring to it and hand it straight back, leaving the entry that is about to be + * written unreadable on the next launch - the very failure releasing on close used to cause. * - * Empty on a fresh process, so a grant whose document never made it onto the list is reclaimed - * on the next launch rather than leaking for good. + * Empty on a fresh process, so a grant whose document never made it onto a list is reclaimed on + * the next launch rather than leaking for good. */ private val pendingGrants = HashSet() @@ -83,9 +85,9 @@ object PersistedUriPermissions { * Does file and binder work, so it must not run on the main thread. */ fun prune(context: Context) { - // the three reads are in this order on purpose. a grant taken while this runs is either - // missing from the snapshot, or still pending when the pending set is read - which happens - // after the list that would have settled it, so it cannot fall through both + // the reads are in this order on purpose. a grant taken while this runs is either missing + // from the snapshot, or still pending when the pending set is read - which happens after + // the lists that would have settled it, so it cannot fall through both val held = context.contentResolver.persistedUriPermissions val keep = HashSet() @@ -93,32 +95,52 @@ object PersistedUriPermissions { keep.add(entry.uri) } + val trees = FolderTreesUtil.getFolderTrees(context).map { it.uri.toString() } + keep.addAll(trees) + keep.addAll(settlePending(keep)) for (permission in held) { - val uri = permission.uri.toString() - if (uri in keep) { - continue + if (!isReferenced(permission.uri.toString(), keep, trees)) { + release(context, permission) } + } + } - var flags = 0 - if (permission.isReadPermission) { - flags = flags or READ_FLAG - } - if (permission.isWritePermission) { - flags = flags or Intent.FLAG_GRANT_WRITE_URI_PERMISSION - } + /** + * Whether [uri] is still spoken for, either by name or by sitting below a granted tree. + * + * [prune]'s whole decision, and free of android imports, so a plain jvm test can cover it. + */ + internal fun isReferenced(uri: String, keep: Set, trees: List): Boolean { + if (uri in keep) { + return true + } - try { - // releasing with flags we do not hold throws, hence taking them off the grant - context.contentResolver.releasePersistableUriPermission(permission.uri, flags) - } catch (e: Exception) { - // nothing to do about it, and it will be retried on the next launch - } + // a document below a granted tree is reachable through that grant, and its uri spells + // the tree out: content://authority/tree//document/. so coverage + // can be recognised from the string, without recording where a document came from. + return trees.any { uri.startsWith("$it/") } + } + + private fun release(context: Context, permission: UriPermission) { + var flags = 0 + if (permission.isReadPermission) { + flags = flags or READ_FLAG + } + if (permission.isWritePermission) { + flags = flags or Intent.FLAG_GRANT_WRITE_URI_PERMISSION + } + + try { + // releasing with flags we do not hold throws, hence taking them off the grant + context.contentResolver.releasePersistableUriPermission(permission.uri, flags) + } catch (e: Exception) { + // nothing to do about it, and it will be retried on the next launch } } - /** Remembers a grant until the recent list names it. */ + /** Remembers a grant until one of the stored lists names it. */ @Synchronized internal fun markPending(uri: String) { pendingGrants.add(uri) @@ -126,7 +148,7 @@ object PersistedUriPermissions { /** * Drops the pending grants [keep] has caught up with - those are reclaimable the ordinary way - * from now on - and hands back the ones whose document is still on its way onto the list. + * from now on - and hands back the ones that are still in flight. */ @Synchronized internal fun settlePending(keep: Set): Set { diff --git a/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt b/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt new file mode 100644 index 000000000000..110ea6994f5d --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt @@ -0,0 +1,67 @@ +package app.opendocument.droid.background + +/** + * Whether a document is worth offering in the folder browser. + * + * Deliberately free of Android dependencies so it can be covered by plain JVM unit tests. + * + * This is a guess, not the real answer. [MetadataLoader] decides what is supported by running + * libmagic over the file once it has been copied into the cache, and neither [CoreLoader] nor + * [RawLoader] can answer before that - so a folder listing, which only has a name and whatever mime + * type the provider volunteered, has to go on appearances. + * + * Mirrors the STRICT_CATCH intent filter in AndroidManifest.xml. **Keep the two in step**: that + * filter is what the system offers us for, this is what we offer ourselves for. + */ +object SupportedDocumentTypes { + + /** Mime types and prefixes taken from the STRICT_CATCH filter. */ + private val MIME_PREFIXES = + listOf( + "application/vnd.oasis.opendocument", + "application/vnd.openxmlformats-officedocument", + "application/msword", + "application/pdf", + "application/zip", + "text/plain", + "text/csv", + "image/", + ) + + /** The extensions the STRICT_CATCH pathPatterns spell out. */ + private val EXTENSIONS = + setOf( + "odt", + "ods", + "odp", + "odg", + "ott", + "ots", + "otp", + "otg", + "doc", + "docx", + "xlsx", + "pdf", + "txt", + "csv", + "zip", + ) + + fun isSupported(mimeType: String?, filename: String?): Boolean { + // providers regularly hand out application/octet-stream for anything they do not know, + // so a recognised extension has to be able to win on its own + val extension = MimeTypeResolver.parseExtension(filename)?.lowercase() + if (extension != null && extension in EXTENSIONS) { + return true + } + + if (mimeType == null) { + return false + } + + val normalized = mimeType.lowercase() + + return MIME_PREFIXES.any { normalized.startsWith(it) } + } +} 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 7d8d884fe5a2..92c3cd7d1ec0 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 @@ -1,23 +1,31 @@ package app.opendocument.droid.ui.activity +import android.app.Activity +import android.content.Intent import android.net.Uri import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.activity.OnBackPressedCallback +import androidx.activity.result.contract.ActivityResultContracts import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProvider 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.nonfree.AnalyticsConstants +import app.opendocument.droid.ui.SnackbarHelper import app.opendocument.droid.ui.widget.LandingAdapter import app.opendocument.droid.ui.widget.LandingItem import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.google.android.material.floatingactionbutton.FloatingActionButton /** - * The screen the app opens on: the documents the user was last working on. + * The screen the app opens on: the documents the user was last working on, and the folders they + * granted access to. * * A fragment rather than more views inside MainActivity, following DocumentFragment - the list * state belongs in a ViewModel so it survives the recreation a theme or rotation change causes. @@ -35,6 +43,57 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { private var landingVisible = true + /** + * Back goes up a folder before it goes anywhere else. + * + * Registered on viewLifecycleOwner so it stacks above MainActivity's own callback - the + * dispatcher runs the most recently added enabled callback first. + */ + private val folderBackCallback = + object : OnBackPressedCallback(false) { + override fun handleOnBackPressed() { + if (!viewModel.leaveFolder()) { + isEnabled = false + } + } + } + + /** + * ACTION_OPEN_DOCUMENT_TREE. Read only - the app asks for no write access anywhere, which is + * what keeps it free of storage permissions entirely. + */ + private val openTreeLauncher = + registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> + if (result.resultCode != Activity.RESULT_OK) { + return@registerForActivityResult + } + + val treeUri = result.data?.data ?: return@registerForActivityResult + + // has to happen before the transient grant on the result intent expires, and through + // PersistedUriPermissions rather than the resolver directly: addFolderTree writes the + // tree down on the view model's executor, so a prune in between would find a grant + // nothing points at yet and hand it straight back + if (!PersistedUriPermissions.takeRead(requireContext(), treeUri)) { + // some providers refuse to hand out a lasting grant + mainActivity.crashManager.log("could not persist the grant for $treeUri") + + SnackbarHelper.show( + requireActivity(), + R.string.landing_add_folder_failed, + null, + false, + true, + ) + + return@registerForActivityResult + } + + mainActivity.analyticsManager.report("folder_added") + + viewModel.addFolderTree(treeUri) + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -65,6 +124,13 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { mainActivity.findDocument() } + view.findViewById(R.id.landing_empty_add_folder).setOnClickListener { + onActionClicked(LandingItem.ACTION_ADD_FOLDER) + } + + requireActivity() + .onBackPressedDispatcher + .addCallback(viewLifecycleOwner, folderBackCallback) viewModel.state.observe(viewLifecycleOwner) { state -> render(state) } } @@ -81,7 +147,8 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { * * The fragment is only hidden, never stopped - so nothing in the lifecycle fires when a * document is closed, and the list would keep showing what it held before the document was - * opened, missing the document that was just added to it. + * opened, missing the document that was just added to it. The back callback has to be turned + * off too, or a hidden landing screen would keep swallowing back inside a document. */ fun setLandingVisible(visible: Boolean) { landingVisible = visible @@ -92,14 +159,44 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { return } + folderBackCallback.isEnabled = visible && viewModel.isInsideFolder + if (visible) { viewModel.refresh() } } private fun render(state: LandingViewModel.State) { + folderBackCallback.isEnabled = landingVisible && state.location != null + val items = ArrayList() + val location = state.location + if (location != null) { + renderFolder(items, state, location) + } else { + renderRoot(items, state) + } + + val isEmpty = location == null && state.documents.isEmpty() && state.trees.isEmpty() + + empty.visibility = if (isEmpty) View.VISIBLE else View.GONE + list.visibility = if (isEmpty) View.GONE else View.VISIBLE + + // the empty state offers the same thing with a label on it, so the bare fab would just + // be a second unexplained button next to it + fab.visibility = if (isEmpty) View.GONE else View.VISIBLE + + // a refresh can land while a document is open - the fragment is only hidden, so it keeps + // getting lifecycle callbacks - and the folder name must not take over the title then + if (landingVisible) { + mainActivity.title = location?.displayName ?: "" + } + + adapter.submitList(items) + } + + private fun renderRoot(items: ArrayList, state: LandingViewModel.State) { if (state.documents.isNotEmpty()) { items.add(LandingItem.Header(R.string.landing_section_recent)) for (document in state.documents) { @@ -107,33 +204,77 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { } } + items.add(LandingItem.Header(R.string.landing_section_folders)) + for (tree in state.trees) { + items.add( + LandingItem.Folder( + tree.displayName, + tree.uri, + DocumentTreeBrowser.rootDocumentId(tree.uri), + ) + ) + } + items.add( + LandingItem.Action( + LandingItem.ACTION_ADD_FOLDER, + R.string.landing_add_folder, + R.drawable.ic_add, + ) + ) + items.add(LandingItem.Header(R.string.landing_section_settings)) items.add(LandingItem.CatchAll(state.catchAllEnabled)) + } - // with nothing to show but the setting, the list is just noise around one button - val isEmpty = state.documents.isEmpty() + private fun renderFolder( + items: ArrayList, + state: LandingViewModel.State, + location: LandingViewModel.FolderLocation, + ) { + items.add( + LandingItem.Action( + LandingItem.ACTION_UP, + R.string.landing_up_one_level, + R.drawable.ic_arrow_upward, + ) + ) - empty.visibility = if (isEmpty) View.VISIBLE else View.GONE - list.visibility = if (isEmpty) View.GONE else View.VISIBLE + if (state.children.isEmpty()) { + items.add(LandingItem.Message(R.string.landing_folder_empty)) - // the empty state offers the same thing with a label on it, so the bare fab would just - // be a second unexplained button next to it - fab.visibility = if (isEmpty) View.GONE else View.VISIBLE + return + } - adapter.submitList(items) + for (child in state.children) { + if (child.isDirectory) { + items.add(LandingItem.Folder(child.displayName, location.treeUri, child.documentId)) + } else { + items.add( + LandingItem.Document( + child.displayName, + DocumentTreeBrowser.documentUri(location.treeUri, child.documentId), + ) + ) + } + } } override fun onDocumentClicked(document: LandingItem.Document) { mainActivity.analyticsManager.report( AnalyticsConstants.EVENT_SELECT_CONTENT, AnalyticsConstants.PARAM_CONTENT_TYPE, - "recent", + if (viewModel.isInsideFolder) "folder" else "recent", ) mainActivity.loadUri(document.uri) } override fun onDocumentRemoveRequested(document: LandingItem.Document) { + // a document inside a folder is not ours to forget - it is simply there + if (viewModel.isInsideFolder) { + return + } + MaterialAlertDialogBuilder(requireContext()) .setTitle(document.filename) .setMessage(R.string.landing_remove_recent_message) @@ -144,6 +285,59 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { .show() } + override fun onFolderClicked(folder: LandingItem.Folder) { + viewModel.enterFolder(folder.treeUri, folder.documentId, folder.name) + } + + override fun onFolderRemoveRequested(folder: LandingItem.Folder) { + // only the folders the user added themselves can be given back; a sub directory is just + // part of the tree above it + if (viewModel.isInsideFolder) { + return + } + + MaterialAlertDialogBuilder(requireContext()) + .setTitle(folder.name) + .setMessage(R.string.landing_remove_folder_message) + .setNegativeButton(android.R.string.cancel, null) + .setPositiveButton(R.string.landing_remove_folder) { _, _ -> + mainActivity.analyticsManager.report("folder_removed") + + viewModel.removeFolderTree(folder.treeUri) + } + .show() + } + + override fun onActionClicked(action: Int) { + when (action) { + LandingItem.ACTION_ADD_FOLDER -> addFolder() + LandingItem.ACTION_UP -> viewModel.leaveFolder() + } + } + + private fun addFolder() { + val intent = + Intent(Intent.ACTION_OPEN_DOCUMENT_TREE) + .addFlags( + Intent.FLAG_GRANT_READ_URI_PERMISSION or + Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION + ) + + try { + openTreeLauncher.launch(intent) + } catch (e: Exception) { + mainActivity.crashManager.log(e) + + SnackbarHelper.show( + requireActivity(), + R.string.crouton_error_open_app, + null, + false, + true, + ) + } + } + override fun onCatchAllChanged(enabled: Boolean) { viewModel.setCatchAllEnabled(enabled) 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 ee42f45a46a6..b6043b320a0d 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 @@ -8,6 +8,8 @@ import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import app.opendocument.droid.background.CatchAllSetting +import app.opendocument.droid.background.DocumentTreeBrowser +import app.opendocument.droid.background.FolderTreesUtil import app.opendocument.droid.background.PersistedUriPermissions import app.opendocument.droid.background.RecentDocumentList import app.opendocument.droid.background.RecentDocumentsUtil @@ -17,22 +19,41 @@ import java.util.concurrent.Executors /** * What the landing screen shows. * - * Reading the recently opened documents touches a file and checking whether they still resolve - * talks to their content provider, so both happen on [executor] and are published through - * [LiveData] - which, unlike posting to a handler, drops the result when the fragment is gone. + * Reading the recently opened documents touches a file, listing a folder talks to its provider, and + * checking whether a document still resolves does too - so all of it happens on [executor] and is + * published through [LiveData], which, unlike posting to a handler, drops the result when the + * fragment is gone. * * There are no coroutines anywhere in this project, so this follows the executor plus handler shape * the loaders already use. */ class LandingViewModel(application: Application) : AndroidViewModel(application) { - class State(val documents: List, val catchAllEnabled: Boolean) + /** Where in a folder tree the browser currently is. */ + class FolderLocation(val treeUri: Uri, val documentId: String, val displayName: String) + + class State( + val documents: List, + val trees: List, + val catchAllEnabled: Boolean, + /** null while the roots are shown, otherwise the folder that is open. */ + val location: FolderLocation?, + val children: List, + ) private val executor: ExecutorService = Executors.newSingleThreadExecutor() private val mutableState = MutableLiveData() val state: LiveData = mutableState + // not persisted across process death on purpose: coming back into a folder whose grant was + // revoked while the app was away is a pile of edge cases for very little + private val backStack = ArrayList() + private var location: FolderLocation? = null + + val isInsideFolder: Boolean + get() = location != null + /** * Publishes what is on disk right away, then re-publishes once the documents that no longer * resolve have been dropped. Rendering does not wait on the provider round trip that way. @@ -42,9 +63,15 @@ class LandingViewModel(application: Application) : AndroidViewModel(application) val context = getApplication() val stored = RecentDocumentsUtil.getRecentDocuments(context) + val trees = FolderTreesUtil.getFolderTrees(context) val catchAll = CatchAllSetting.isEnabled(context) + val here = location - mutableState.postValue(State(stored, catchAll)) + val children = + if (here == null) emptyList() + else DocumentTreeBrowser.listChildren(context, here.treeUri, here.documentId) + + mutableState.postValue(State(stored, trees, catchAll, here, children)) val alive = stored.filter { isReadable(context, Uri.parse(it.uri)) } if (alive.size == stored.size) { @@ -56,10 +83,60 @@ class LandingViewModel(application: Application) : AndroidViewModel(application) } PersistedUriPermissions.prune(context) - mutableState.postValue(State(alive, catchAll)) + mutableState.postValue(State(alive, trees, catchAll, here, children)) + } + } + + fun addFolderTree(treeUri: Uri) { + executor.execute { + val context = getApplication() + + val displayName = DocumentTreeBrowser.displayNameOf(context, treeUri) + if (FolderTreesUtil.addFolderTree(context, treeUri, displayName).isNotEmpty()) { + PersistedUriPermissions.prune(context) + } + + refresh() } } + fun removeFolderTree(treeUri: Uri) { + executor.execute { + val context = getApplication() + + FolderTreesUtil.removeFolderTree(context, treeUri) + PersistedUriPermissions.prune(context) + + // the folder that was just dropped may be the one we are standing in + if (location?.treeUri == treeUri) { + backStack.clear() + location = null + } + + refresh() + } + } + + fun enterFolder(treeUri: Uri, documentId: String, displayName: String) { + location?.let { backStack.add(it) } + location = FolderLocation(treeUri, documentId, displayName) + + refresh() + } + + /** @return whether there was somewhere to go back to. */ + fun leaveFolder(): Boolean { + if (location == null) { + return false + } + + location = backStack.removeLastOrNull() + + refresh() + + return true + } + fun setCatchAllEnabled(enabled: Boolean) { CatchAllSetting.setEnabled(getApplication(), enabled) 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 157a9c649576..eb266f87ac8d 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 @@ -11,7 +11,10 @@ import androidx.recyclerview.widget.RecyclerView import app.opendocument.droid.R import com.google.android.material.materialswitch.MaterialSwitch -/** The rows of the landing screen: the recently opened documents and the catch-all setting. */ +/** + * The rows of the landing screen: the recently opened documents, the folders the user granted + * access to, and the settings. + */ class LandingAdapter(private val listener: Listener) : ListAdapter(DIFF) { @@ -21,6 +24,12 @@ class LandingAdapter(private val listener: Listener) : fun onDocumentRemoveRequested(document: LandingItem.Document) + fun onFolderClicked(folder: LandingItem.Folder) + + fun onFolderRemoveRequested(folder: LandingItem.Folder) + + fun onActionClicked(action: Int) + fun onCatchAllChanged(enabled: Boolean) } @@ -28,6 +37,8 @@ class LandingAdapter(private val listener: Listener) : 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.Message -> TYPE_MESSAGE } @@ -40,6 +51,7 @@ class LandingAdapter(private val listener: Listener) : TYPE_HEADER -> R.layout.item_landing_header TYPE_CATCH_ALL -> R.layout.item_landing_switch TYPE_MESSAGE -> R.layout.item_landing_message + // documents, folders and actions are all an icon plus a label else -> R.layout.item_landing_row } @@ -63,6 +75,24 @@ class LandingAdapter(private val listener: Listener) : } } + is LandingItem.Folder -> { + holder.icon.setImageResource(R.drawable.ic_folder) + holder.title.text = item.name + holder.itemView.setOnClickListener { listener.onFolderClicked(item) } + holder.itemView.setOnLongClickListener { + listener.onFolderRemoveRequested(item) + + true + } + } + + is LandingItem.Action -> { + holder.icon.setImageResource(item.icon) + holder.title.setText(item.label) + holder.itemView.setOnClickListener { listener.onActionClicked(item.action) } + holder.itemView.setOnLongClickListener(null) + } + is LandingItem.CatchAll -> { // set the state before the listener, so restoring it does not report a change holder.switch.setOnCheckedChangeListener(null) @@ -85,8 +115,10 @@ class LandingAdapter(private val listener: Listener) : private companion object { const val TYPE_HEADER = 0 const val TYPE_DOCUMENT = 1 - const val TYPE_CATCH_ALL = 2 - const val TYPE_MESSAGE = 3 + const val TYPE_FOLDER = 2 + const val TYPE_ACTION = 3 + const val TYPE_CATCH_ALL = 4 + const val TYPE_MESSAGE = 5 val DIFF = object : DiffUtil.ItemCallback() { @@ -102,10 +134,13 @@ class LandingAdapter(private val listener: Listener) : oldItem is LandingItem.Document && newItem is LandingItem.Document -> oldItem.filename == newItem.filename + oldItem is LandingItem.Folder && newItem is LandingItem.Folder -> + oldItem.name == newItem.name + oldItem is LandingItem.CatchAll && newItem is LandingItem.CatchAll -> oldItem.checked == newItem.checked - // headers and messages are fully described by their id + // headers, messages and actions are fully described by their id else -> true } } 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 d8658c7eec31..7beeee24c539 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 @@ -17,6 +17,15 @@ sealed class LandingItem { override val id: String = "document:$uri" } + class Folder(val name: String, val treeUri: Uri, val documentId: String) : LandingItem() { + override val id: String = "folder:$treeUri:$documentId" + } + + /** A tappable row that is not a document, such as "add a folder". */ + class Action(val action: Int, @param:StringRes val label: Int, val icon: Int) : LandingItem() { + override val id: String = "action:$action" + } + class CatchAll(val checked: Boolean) : LandingItem() { override val id: String = "catch_all" } @@ -25,4 +34,9 @@ sealed class LandingItem { class Message(@param:StringRes val text: Int) : LandingItem() { override val id: String = "message:$text" } + + companion object { + const val ACTION_ADD_FOLDER: Int = 1 + const val ACTION_UP: Int = 2 + } } diff --git a/app/src/main/res/drawable/ic_arrow_upward.xml b/app/src/main/res/drawable/ic_arrow_upward.xml new file mode 100644 index 000000000000..21a35e76a761 --- /dev/null +++ b/app/src/main/res/drawable/ic_arrow_upward.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/layout/fragment_landing.xml b/app/src/main/res/layout/fragment_landing.xml index 2cd3940dd852..141d1e2357e6 100644 --- a/app/src/main/res/layout/fragment_landing.xml +++ b/app/src/main/res/layout/fragment_landing.xml @@ -57,6 +57,14 @@ android:text="@string/landing_open_file" app:icon="@drawable/ic_folder_open" /> + Settings Open all file types 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. Nothing here yet diff --git a/app/src/test/java/app/opendocument/droid/background/PersistedUriPermissionsTest.kt b/app/src/test/java/app/opendocument/droid/background/PersistedUriPermissionsTest.kt index 0bf0f78f1b22..029d3041d2ff 100644 --- a/app/src/test/java/app/opendocument/droid/background/PersistedUriPermissionsTest.kt +++ b/app/src/test/java/app/opendocument/droid/background/PersistedUriPermissionsTest.kt @@ -4,8 +4,9 @@ import org.junit.Assert import org.junit.Test /** - * Covers the bookkeeping that keeps a grant alive between taking it and the recent list naming it. - * Taking and releasing the grants themselves needs a content resolver, so that part is device work. + * Covers the bookkeeping that keeps a grant alive between taking it and a stored list naming it, + * and the decision prune makes about each grant it holds. Taking and releasing the grants + * themselves needs a content resolver, so that part is device work. */ class PersistedUriPermissionsTest { @@ -31,4 +32,29 @@ class PersistedUriPermissionsTest { Assert.assertFalse(leaked in PersistedUriPermissions.settlePending(emptySet())) } + + @Test + fun keepsWhatSitsBelowAGrantedTree() { + val tree = "content://com.android.externalstorage.documents/tree/primary%3ADocuments" + val trees = listOf(tree) + + // the tree itself is named by the folder list + Assert.assertTrue(PersistedUriPermissions.isReferenced(tree, setOf(tree), trees)) + + // and a document below it is reachable through that grant, so its own grant can go even + // though nothing names it - the uri spells the tree out + val below = "$tree/document/primary%3ADocuments%2Freport.odt" + Assert.assertTrue(PersistedUriPermissions.isReferenced(below, setOf(tree), trees)) + + // a sibling tree that merely starts the same way is not covered by it + val sibling = "${tree}Archive" + Assert.assertFalse(PersistedUriPermissions.isReferenced(sibling, setOf(tree), trees)) + } + + @Test + fun releasesWhatNothingPointsAt() { + val orphan = "content://com.android.providers.downloads.documents/document/17" + + Assert.assertFalse(PersistedUriPermissions.isReferenced(orphan, emptySet(), emptyList())) + } } diff --git a/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt b/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt new file mode 100644 index 000000000000..610064cece4b --- /dev/null +++ b/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt @@ -0,0 +1,71 @@ +package app.opendocument.droid.background + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class SupportedDocumentTypesTest { + + @Test + fun opendocumentTypesAreSupported() { + assertTrue(supported("application/vnd.oasis.opendocument.text", "report.odt")) + assertTrue(supported("application/vnd.oasis.opendocument.spreadsheet", "budget.ods")) + assertTrue(supported("application/vnd.oasis.opendocument.presentation", "deck.odp")) + assertTrue(supported("application/vnd.oasis.opendocument.graphics", "drawing.odg")) + assertTrue(supported("application/vnd.oasis.opendocument.text-template", "letter.ott")) + } + + @Test + fun officeAndPdfTypesAreSupported() { + assertTrue( + supported( + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "notes.docx", + ) + ) + assertTrue(supported("application/msword", "old.doc")) + assertTrue(supported("application/pdf", "manual.pdf")) + assertTrue(supported("text/plain", "readme.txt")) + assertTrue(supported("text/csv", "rows.csv")) + assertTrue(supported("image/png", "scan.png")) + } + + @Test + fun aKnownExtensionWinsOverAnUnhelpfulMimeType() { + // this is the common case: providers fall back to octet-stream for anything they do not + // have a mapping for, which includes most opendocument files + assertTrue(supported("application/octet-stream", "report.odt")) + assertTrue(supported("application/octet-stream", "budget.ods")) + } + + @Test + fun aKnownMimeTypeWinsOverAMissingExtension() { + assertTrue(supported("application/pdf", "scan-without-extension")) + } + + @Test + fun unrelatedTypesAreNotSupported() { + // the reason catch-all defaults to off - see issue #477 + assertFalse(supported("text/vcard", "contact.vcf")) + assertFalse(supported("text/x-vcard", "contact.vcf")) + assertFalse(supported("audio/mpeg", "song.mp3")) + assertFalse(supported("video/mp4", "clip.mp4")) + assertFalse(supported("application/octet-stream", "firmware.bin")) + } + + @Test + fun nothingKnownAtAllIsNotSupported() { + assertFalse(supported(null, null)) + assertFalse(supported(null, "mystery")) + assertFalse(supported("application/octet-stream", null)) + } + + @Test + fun matchingIsCaseInsensitive() { + assertTrue(supported("APPLICATION/PDF", "MANUAL.PDF")) + assertTrue(supported(null, "Report.ODT")) + } + + private fun supported(mimeType: String?, filename: String?) = + SupportedDocumentTypes.isSupported(mimeType, filename) +} From f5bf3eb530016449f8622cc4ead691bfc246e38d Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:19:38 +0200 Subject: [PATCH 05/24] Offer the folder browser only for the ooxml types we open The list of mime prefixes took the whole openxmlformats-officedocument family, but the STRICT_CATCH filter names two of them - docx and xlsx - and CoreLoader opens no others. A provider that reports a pptx by its standard mime type therefore had it listed as openable, and tapping it led into the unsupported/upload path instead. The extension side of the guess already left pptx out, so the two halves disagreed. Both types are now spelled out, and stay prefixes rather than exact matches so that they keep testing the same strings the same way CoreLoader does. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SgJyGjRBpjAJ2724aMNnEp --- .../droid/background/SupportedDocumentTypes.kt | 6 +++++- .../droid/background/SupportedDocumentTypesTest.kt | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt b/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt index 110ea6994f5d..20ae54b4b992 100644 --- a/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt +++ b/app/src/main/java/app/opendocument/droid/background/SupportedDocumentTypes.kt @@ -19,7 +19,11 @@ object SupportedDocumentTypes { private val MIME_PREFIXES = listOf( "application/vnd.oasis.opendocument", - "application/vnd.openxmlformats-officedocument", + // the two ooxml types the filter spells out, rather than the family they belong to: + // pptx shares that prefix and CoreLoader does not open it yet, so a folder would + // offer a presentation and then drop it into the upload flow + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "application/msword", "application/pdf", "application/zip", diff --git a/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt b/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt index 610064cece4b..5d089641280e 100644 --- a/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt +++ b/app/src/test/java/app/opendocument/droid/background/SupportedDocumentTypesTest.kt @@ -30,6 +30,19 @@ class SupportedDocumentTypesTest { assertTrue(supported("image/png", "scan.png")) } + @Test + fun presentationsTheCoreCannotOpenAreNotSupported() { + // pptx sits in the same ooxml family as docx and xlsx, but neither the STRICT_CATCH filter + // nor CoreLoader takes it - listing one would promise an open that ends in the upload flow + assertFalse( + supported( + "application/vnd.openxmlformats-officedocument.presentationml.presentation", + "deck.pptx", + ) + ) + assertFalse(supported("application/vnd.ms-powerpoint", "deck.ppt")) + } + @Test fun aKnownExtensionWinsOverAnUnhelpfulMimeType() { // this is the common case: providers fall back to octet-stream for anything they do not From 7f092ea23ae962abcf0723ff6c58e895f5ecc176 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:19:47 +0200 Subject: [PATCH 06/24] Keep the settings on screen while there is nothing to list The empty state was shown instead of the list, which took the settings section with it: on a fresh install - and after the last recent document is removed - the catch-all switch was the one thing on the landing screen that could not be reached, and it is exactly what somebody whose file manager refuses to hand documents over needs to find. It is now the first row of the list rather than a screen of its own, so whatever sits below it stays where it is. It keeps both of its buttons, which is why the folders section is left out while it is up: the row already offers to add one, and an empty section under it would say the same thing twice. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SgJyGjRBpjAJ2724aMNnEp --- .../opendocument/droid/test/LandingTests.kt | 9 +++ .../droid/ui/activity/LandingFragment.kt | 75 ++++++++++--------- .../droid/ui/widget/LandingAdapter.kt | 17 ++++- .../droid/ui/widget/LandingItem.kt | 10 +++ app/src/main/res/layout/fragment_landing.xml | 55 -------------- .../main/res/layout/item_landing_empty.xml | 58 ++++++++++++++ 6 files changed, 132 insertions(+), 92 deletions(-) create mode 100644 app/src/main/res/layout/item_landing_empty.xml 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 64167ba9f6b7..51c88fdcaae4 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/LandingTests.kt @@ -156,6 +156,15 @@ class LandingTests { onView(withText(R.string.landing_catch_all_title)).check(matches(isDisplayed())) } + @Test + fun theCatchAllSettingIsOfferedBeforeAnythingWasOpened() { + // a fresh install is where the switch matters most, and the empty state used to be shown + // instead of the list it sits in + launch() + + onView(withText(R.string.landing_catch_all_title)).check(matches(isDisplayed())) + } + private fun launch(): MainActivity { val activity = activityRule.launchActivity(null) 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 92c3cd7d1ec0..cc8075007e0f 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 @@ -38,7 +38,6 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { private lateinit var adapter: LandingAdapter private lateinit var list: RecyclerView - private lateinit var empty: View private lateinit var fab: FloatingActionButton private var landingVisible = true @@ -111,22 +110,12 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { list.layoutManager = LinearLayoutManager(requireContext()) list.adapter = adapter - empty = view.findViewById(R.id.landing_empty) - fab = view.findViewById(R.id.landing_open_fab) fab.setOnClickListener { mainActivity.analyticsManager.report("fab_open") mainActivity.findDocument() } - view.findViewById(R.id.landing_empty_open).setOnClickListener { - mainActivity.analyticsManager.report("empty_open") - - mainActivity.findDocument() - } - view.findViewById(R.id.landing_empty_add_folder).setOnClickListener { - onActionClicked(LandingItem.ACTION_ADD_FOLDER) - } requireActivity() .onBackPressedDispatcher @@ -172,17 +161,14 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { val items = ArrayList() val location = state.location + val isEmpty = location == null && state.documents.isEmpty() && state.trees.isEmpty() + if (location != null) { renderFolder(items, state, location) } else { - renderRoot(items, state) + renderRoot(items, state, isEmpty) } - val isEmpty = location == null && state.documents.isEmpty() && state.trees.isEmpty() - - empty.visibility = if (isEmpty) View.VISIBLE else View.GONE - list.visibility = if (isEmpty) View.GONE else View.VISIBLE - // the empty state offers the same thing with a label on it, so the bare fab would just // be a second unexplained button next to it fab.visibility = if (isEmpty) View.GONE else View.VISIBLE @@ -196,31 +182,42 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { adapter.submitList(items) } - private fun renderRoot(items: ArrayList, state: LandingViewModel.State) { - 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))) + private fun renderRoot( + items: ArrayList, + state: LandingViewModel.State, + isEmpty: Boolean, + ) { + if (isEmpty) { + // a row of the list rather than a screen of its own: the settings below it are the + // only place the catch-all switch is offered, and a fresh install starts here. it + // carries both actions itself, so the empty folders section stays off the screen. + items.add(LandingItem.Empty()) + } else { + 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.Header(R.string.landing_section_folders)) - for (tree in state.trees) { + items.add(LandingItem.Header(R.string.landing_section_folders)) + for (tree in state.trees) { + items.add( + LandingItem.Folder( + tree.displayName, + tree.uri, + DocumentTreeBrowser.rootDocumentId(tree.uri), + ) + ) + } items.add( - LandingItem.Folder( - tree.displayName, - tree.uri, - DocumentTreeBrowser.rootDocumentId(tree.uri), + LandingItem.Action( + LandingItem.ACTION_ADD_FOLDER, + R.string.landing_add_folder, + R.drawable.ic_add, ) ) } - items.add( - LandingItem.Action( - LandingItem.ACTION_ADD_FOLDER, - R.string.landing_add_folder, - R.drawable.ic_add, - ) - ) items.add(LandingItem.Header(R.string.landing_section_settings)) items.add(LandingItem.CatchAll(state.catchAllEnabled)) @@ -338,6 +335,12 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { } } + override fun onOpenClicked() { + mainActivity.analyticsManager.report("empty_open") + + mainActivity.findDocument() + } + override fun onCatchAllChanged(enabled: Boolean) { viewModel.setCatchAllEnabled(enabled) 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 eb266f87ac8d..15c7fc254ee2 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 @@ -30,6 +30,8 @@ class LandingAdapter(private val listener: Listener) : fun onActionClicked(action: Int) + fun onOpenClicked() + fun onCatchAllChanged(enabled: Boolean) } @@ -41,6 +43,7 @@ class LandingAdapter(private val listener: Listener) : is LandingItem.Action -> TYPE_ACTION is LandingItem.CatchAll -> TYPE_CATCH_ALL is LandingItem.Message -> TYPE_MESSAGE + is LandingItem.Empty -> TYPE_EMPTY } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { @@ -51,6 +54,7 @@ class LandingAdapter(private val listener: Listener) : TYPE_HEADER -> R.layout.item_landing_header TYPE_CATCH_ALL -> 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 else -> R.layout.item_landing_row } @@ -64,6 +68,13 @@ class LandingAdapter(private val listener: Listener) : is LandingItem.Message -> holder.title.setText(item.text) + is LandingItem.Empty -> { + holder.open.setOnClickListener { listener.onOpenClicked() } + holder.addFolder.setOnClickListener { + listener.onActionClicked(LandingItem.ACTION_ADD_FOLDER) + } + } + is LandingItem.Document -> { holder.icon.setImageResource(R.drawable.ic_description) holder.title.text = item.filename @@ -110,6 +121,8 @@ class LandingAdapter(private val listener: Listener) : 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 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) } } private companion object { @@ -119,6 +132,7 @@ class LandingAdapter(private val listener: Listener) : const val TYPE_ACTION = 3 const val TYPE_CATCH_ALL = 4 const val TYPE_MESSAGE = 5 + const val TYPE_EMPTY = 6 val DIFF = object : DiffUtil.ItemCallback() { @@ -140,7 +154,8 @@ class LandingAdapter(private val listener: Listener) : oldItem is LandingItem.CatchAll && newItem is LandingItem.CatchAll -> oldItem.checked == newItem.checked - // headers, messages and actions are fully described by their id + // headers, messages, actions and the empty state are fully described + // by their id else -> true } } 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 7beeee24c539..7de80b17d34c 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 @@ -35,6 +35,16 @@ sealed class LandingItem { override val id: String = "message:$text" } + /** + * What the screen says while it has nothing to list, offering both ways to fill it. + * + * A row like any other, so that the settings underneath stay on screen - shown instead of the + * list, it would take the catch-all switch with it on a fresh install. + */ + class Empty : LandingItem() { + override val id: String = "empty" + } + companion object { const val ACTION_ADD_FOLDER: Int = 1 const val ACTION_UP: Int = 2 diff --git a/app/src/main/res/layout/fragment_landing.xml b/app/src/main/res/layout/fragment_landing.xml index 141d1e2357e6..86eec3767cda 100644 --- a/app/src/main/res/layout/fragment_landing.xml +++ b/app/src/main/res/layout/fragment_landing.xml @@ -1,7 +1,6 @@ @@ -13,60 +12,6 @@ android:paddingBottom="88dp" android:scrollbars="vertical" /> - - - - - - - - - - - - - - + + + + + + + + + + + + + + From 89fcd3199e7544e626b4142a97592e25a2ed2e5a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:46:49 +0200 Subject: [PATCH 07/24] Swipe a recent document away, and say when it was opened (#528) Two things the list was missing. Each row now carries the time it was last opened, which is the thing that tells two similarly named documents apart, and a recent document can be swiped off the list with an undo. Undo puts the entry back at the index it came from rather than at the front, so the list ends up looking exactly as it did - that is the point of an undo, and it is why RecentDocumentList grows an insert() next to add() instead of reusing it. Nothing releases the uri permission on the way out. An undo needs it back, and prune() reclaims it on the next launch anyway - which is what reconciling against the stored lists, rather than bookkeeping every removal, was for. Only the recently opened documents can be swiped. A document inside a granted folder is simply there, and the app has no business removing anything from disk; the adapter tells the two apart by whether the row has a timestamp on it. A "dark documents" switch was built here too and taken back out: with the html odrcore generates, algorithmic darkening changes nothing visible, so the setting would have looked broken. Making it work needs a dark stylesheet from the core, not a toggle here. Co-authored-by: Claude Opus 5 (1M context) --- .../opendocument/droid/test/LandingTests.kt | 26 ++-- .../droid/background/RecentDocumentList.kt | 15 +++ .../droid/background/RecentDocumentsUtil.kt | 6 + .../opendocument/droid/ui/SnackbarHelper.kt | 19 +++ .../droid/ui/activity/LandingFragment.kt | 125 +++++++++++++++++- .../droid/ui/activity/LandingViewModel.kt | 28 ++++ .../droid/ui/widget/LandingAdapter.kt | 45 +++++-- .../droid/ui/widget/LandingItem.kt | 16 ++- app/src/main/res/layout/item_landing_row.xml | 34 +++-- .../main/res/layout/item_landing_switch.xml | 4 +- app/src/main/res/values/strings.xml | 10 +- .../background/RecentDocumentListTest.kt | 47 +++++++ 12 files changed, 329 insertions(+), 46 deletions(-) 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) } From 8df9a17f87349607ced381838ec30f1529d99bf5 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:49:39 +0200 Subject: [PATCH 08/24] Drop the open action from the toolbar (#529) The landing screen already offers to open a document twice - the fab, and the labelled button in the empty state - and both go straight to the system picker since the chooser dialog went away. The toolbar icon was a third copy of the same intent, sitting in the one place where it is least explained: an unlabelled folder glyph next to the title. Inside a document it was the odd one out too. Every other toolbar action acts on the document that is open; this one threw it away and started over, which is what back already does, and back lands on the list where the next document is picked anyway. The tests clicked the toolbar icon to reach the picker, so they now click whichever landing entry point is on screen. Which of the two it is depends on whether an earlier test already left something in the recent list, so they match either. Co-authored-by: Claude Opus 5 (1M context) --- .../app/opendocument/droid/test/MainActivityTests.kt | 11 +++++++---- .../opendocument/droid/ui/activity/MainActivity.kt | 11 ----------- app/src/main/res/menu/menu_main.xml | 7 ------- app/src/main/res/values/strings.xml | 1 - 4 files changed, 7 insertions(+), 23 deletions(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt index 8f200797694f..ec18c4b0b3d1 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt @@ -10,6 +10,7 @@ import android.content.Intent import android.net.Uri import android.os.SystemClock import android.util.Log +import android.view.View import androidx.core.content.FileProvider import androidx.test.espresso.Espresso.onView import androidx.test.espresso.IdlingRegistry @@ -48,6 +49,7 @@ import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicReference import org.hamcrest.Matchers.allOf +import org.hamcrest.Matchers.anyOf import org.hamcrest.Matchers.equalTo import org.junit.After import org.junit.AfterClass @@ -253,13 +255,14 @@ class MainActivityTests { ) } - // the toolbar action now launches ACTION_OPEN_DOCUMENT directly, which the test stubs out. - // it used to raise a chooser of our own that a file manager had to be picked from first. + // the landing screen is the only thing offering to open a document now that the toolbar + // action is gone. exactly one of its two entry points is on screen - the fab is hidden + // while the empty state is up - and which one depends on whether an earlier test already + // left a document in the recent list, so match either. private fun openDocumentThroughPicker() { onView( allOf( - withId(R.id.menu_open), - withContentDescription("Open document"), + anyOf(withId(R.id.landing_open_fab), withId(R.id.landing_empty_open)), isDisplayed(), ) ) diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt index 7656b7466ac1..a5688407183c 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt @@ -452,17 +452,6 @@ class MainActivity : AppCompatActivity(), MenuProvider { analyticsManager.report(AnalyticsConstants.EVENT_SEARCH) } - R.id.menu_open -> { - findDocument() - - analyticsManager.report("menu_open") - analyticsManager.report( - AnalyticsConstants.EVENT_SELECT_CONTENT, - AnalyticsConstants.PARAM_CONTENT_TYPE, - "choose", - ) - } - R.id.menu_open_with -> { documentFragment?.openWith(this) diff --git a/app/src/main/res/menu/menu_main.xml b/app/src/main/res/menu/menu_main.xml index 9c0df3c4e1b0..7e8de18e11dc 100644 --- a/app/src/main/res/menu/menu_main.xml +++ b/app/src/main/res/menu/menu_main.xml @@ -3,13 +3,6 @@ xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools"> - - Uploaded files are private and automatically deleted after 24 hours. We aren\'t able to open this document, because we don\'t support its format. Do you want to upload it to our server temporarily, so we can display it for you anyway? Uploaded files are private and automatically deleted after 24 hours. Search in document - Open document Open with... Share document Edit document From 29960644bef7885c35c9f7766bbb7a2a46f3be88 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:54:37 +0200 Subject: [PATCH 09/24] Let the row say it is a recent, instead of guessing from the subtitle The subtitle carries the last opened time, and an entry written before that time was recorded has none - so reading "is this a recent" off it made exactly those legacy entries unswipeable, on the one install where it matters: an upgrade with a recent_documents.json already in place. They would have stayed that way until each was opened again. LandingItem.Document now carries the provenance itself, set where the row is built and defaulted off for the folder listing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SgJyGjRBpjAJ2724aMNnEp --- .../droid/ui/activity/LandingFragment.kt | 1 + .../droid/ui/widget/LandingAdapter.kt | 7 ++++--- .../opendocument/droid/ui/widget/LandingItem.kt | 17 ++++++++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) 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 260f75db4dea..03d4a9d7949b 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 @@ -281,6 +281,7 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { document.filename, Uri.parse(document.uri), lastOpenedLabel(document.lastOpenedAt), + recent = true, ) ) } 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 1354c7753ab2..0618c9630da2 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 @@ -43,9 +43,10 @@ class LandingAdapter(private val listener: Listener) : 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 + // a document inside a folder is not ours to forget - it is simply there. the row says + // which of the two it is; the subtitle cannot, because an entry stored before the last + // opened time was recorded has none. + return item is LandingItem.Document && item.recent } fun documentAt(position: Int): LandingItem.Document? = 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 c2ee2ac007db..e6d74cca2fb5 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,9 +13,20 @@ sealed class LandingItem { override val id: String = "header:$title" } - /** [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() { + /** + * [subtitle] is the second line, when there is something worth saying - a last opened time. + * + * [recent] says the row came from the recently opened list, which is the only thing the app may + * forget; a document inside a granted folder is simply there. It is carried explicitly rather + * than read off [subtitle], which an entry stored before the last opened time was recorded does + * not have. + */ + class Document( + val filename: String, + val uri: Uri, + val subtitle: String? = null, + val recent: Boolean = false, + ) : LandingItem() { override val id: String = "document:$uri" } From 63c7068dc2e3e260f89e2a60de928c707093ce0c Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:54:46 +0200 Subject: [PATCH 10/24] Cap the recents list when an undo puts a document back insert() skipped the eviction add() does, so an undo could take the list past MAX_ENTRIES: swipe one away with a full list, open another document while the snackbar is still up, then undo, and the list comes back one entry longer - with the extra entry holding a persisted uri permission of its own, against a per package cap the class exists to respect. Both paths share a cap() now, and insert() returns the same Update as add() so the entry that fell off the end can have its grant handed back. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SgJyGjRBpjAJ2724aMNnEp --- .../droid/background/RecentDocumentList.kt | 26 +++++++---- .../droid/background/RecentDocumentsUtil.kt | 20 ++++++-- .../droid/ui/activity/LandingViewModel.kt | 8 +++- .../background/RecentDocumentListTest.kt | 46 ++++++++++++++++--- 4 files changed, 80 insertions(+), 20 deletions(-) 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 a430cd990a35..95913ce48590 100644 --- a/app/src/main/java/app/opendocument/droid/background/RecentDocumentList.kt +++ b/app/src/main/java/app/opendocument/droid/background/RecentDocumentList.kt @@ -29,8 +29,8 @@ object RecentDocumentList { } /** - * The list after an [add], plus whatever fell off the end of it. The evicted entries are handed - * back so their persisted uri permissions can be released again. + * The list after an [add] or an [insert], plus whatever fell off the end of it. The evicted + * entries are handed back so their persisted uri permissions can be released again. */ class Update internal constructor(val entries: List, val evicted: List) @@ -43,11 +43,7 @@ object RecentDocumentList { entries.add(entry) current.filterTo(entries) { it.uri != entry.uri } - if (entries.size <= max) { - return Update(entries, emptyList()) - } - - return Update(entries.subList(0, max).toList(), entries.subList(max, entries.size).toList()) + return cap(entries, max) } /** Drops every entry for [uri]. Returns [current] unchanged if there is none. */ @@ -58,13 +54,25 @@ object RecentDocumentList { * * 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. + * + * [max] still applies: the list can have filled up again while the undo was on offer - another + * document opened over the top of the snackbar - and an undo is no reason to grow past the cap. */ - fun insert(current: List, entry: Entry, index: Int): List { + fun insert(current: List, entry: Entry, index: Int, max: Int = MAX_ENTRIES): Update { val entries = ArrayList(current.size + 1) current.filterTo(entries) { it.uri != entry.uri } entries.add(index.coerceIn(0, entries.size), entry) - return entries + return cap(entries, max) + } + + /** Keeps the first [max] entries, handing the tail back to be released. */ + private fun cap(entries: List, max: Int): Update { + if (entries.size <= max) { + return Update(entries, emptyList()) + } + + return Update(entries.subList(0, max).toList(), entries.subList(max, entries.size).toList()) } } 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 0f04cd4de649..f8264ba151cc 100644 --- a/app/src/main/java/app/opendocument/droid/background/RecentDocumentsUtil.kt +++ b/app/src/main/java/app/opendocument/droid/background/RecentDocumentsUtil.kt @@ -65,10 +65,24 @@ object RecentDocumentsUtil { return update.evicted } - /** Puts a removed document back where it was, for undoing a swipe. */ + /** + * Puts a removed document back where it was, for undoing a swipe. + * + * @return the entries that fell out of the list, so [PersistedUriPermissions] can release the + * uri permissions they were holding - the list can have filled up again while the undo was + * still on offer. + */ @Synchronized - fun restoreRecentDocument(context: Context, entry: RecentDocumentList.Entry, index: Int) { - write(context, RecentDocumentList.insert(read(context), entry, index)) + fun restoreRecentDocument( + context: Context, + entry: RecentDocumentList.Entry, + index: Int, + ): List { + val update = RecentDocumentList.insert(read(context), entry, index) + + write(context, update.entries) + + return update.evicted } /** Drops [uri] from the list. Does nothing if it is not in it. */ 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 3399d2f7d4df..5abc0ba410ce 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 @@ -160,7 +160,13 @@ class LandingViewModel(application: Application) : AndroidViewModel(application) */ fun restoreRecentDocument(entry: RecentDocumentList.Entry, index: Int) { executor.execute { - RecentDocumentsUtil.restoreRecentDocument(getApplication(), entry, index) + val context = getApplication() + + // putting one back can push another off the end, if a document was opened while the + // undo was still on offer - that one is holding a grant nothing points at any more + if (RecentDocumentsUtil.restoreRecentDocument(context, entry, index).isNotEmpty()) { + PersistedUriPermissions.prune(context) + } refresh() } 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 bb306c674c0d..b7b8f6937a24 100644 --- a/app/src/test/java/app/opendocument/droid/background/RecentDocumentListTest.kt +++ b/app/src/test/java/app/opendocument/droid/background/RecentDocumentListTest.kt @@ -100,9 +100,10 @@ class RecentDocumentListTest { fun insertPutsADocumentBackWhereItWas() { val current = listOf(entry("a"), entry("c")) - val restored = RecentDocumentList.insert(current, entry("b"), 1) + val update = RecentDocumentList.insert(current, entry("b"), 1) - assertEquals(listOf("a", "b", "c"), restored.map { it.uri }) + assertEquals(listOf("a", "b", "c"), update.entries.map { it.uri }) + assertTrue(update.evicted.isEmpty()) } @Test @@ -111,11 +112,11 @@ class RecentDocumentListTest { assertEquals( listOf("x", "a", "b"), - RecentDocumentList.insert(current, entry("x"), 0).map { it.uri }, + RecentDocumentList.insert(current, entry("x"), 0).entries.map { it.uri }, ) assertEquals( listOf("a", "b", "x"), - RecentDocumentList.insert(current, entry("x"), 2).map { it.uri }, + RecentDocumentList.insert(current, entry("x"), 2).entries.map { it.uri }, ) } @@ -125,11 +126,11 @@ class RecentDocumentListTest { assertEquals( listOf("a", "x"), - RecentDocumentList.insert(current, entry("x"), 99).map { it.uri }, + RecentDocumentList.insert(current, entry("x"), 99).entries.map { it.uri }, ) assertEquals( listOf("x", "a"), - RecentDocumentList.insert(current, entry("x"), -5).map { it.uri }, + RecentDocumentList.insert(current, entry("x"), -5).entries.map { it.uri }, ) } @@ -139,9 +140,40 @@ class RecentDocumentListTest { assertEquals( listOf("b", "a"), - RecentDocumentList.insert(current, entry("a"), 1).map { it.uri }, + RecentDocumentList.insert(current, entry("a"), 1).entries.map { it.uri }, ) } + @Test + fun insertIntoAListThatFilledUpAgainStillEvictsTheOldest() { + // the swipe left room, then another document was opened over the top of the snackbar + val current = (1..3).map { entry("uri$it") } + + val update = RecentDocumentList.insert(current, entry("restored"), 1, max = 3) + + assertEquals(listOf("uri1", "restored", "uri2"), update.entries.map { it.uri }) + assertEquals(listOf("uri3"), update.evicted.map { it.uri }) + } + + @Test + fun insertingPastAFullListEvictsTheEntryItself() { + val current = (1..3).map { entry("uri$it") } + + val update = RecentDocumentList.insert(current, entry("restored"), 3, max = 3) + + assertEquals(listOf("uri1", "uri2", "uri3"), update.entries.map { it.uri }) + assertEquals(listOf("restored"), update.evicted.map { it.uri }) + } + + @Test + fun insertNeverGrowsPastTheDefaultCap() { + val current = (1..RecentDocumentList.MAX_ENTRIES).map { entry("uri$it") } + + val update = RecentDocumentList.insert(current, entry("restored"), 0) + + assertEquals(RecentDocumentList.MAX_ENTRIES, update.entries.size) + assertEquals(1, update.evicted.size) + } + private fun entry(uri: String) = RecentDocumentList.Entry("name of $uri", uri, 0) } From 0b720cba94472f47d9b7d6f98e83287929b0ad10 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 20:56:44 +0200 Subject: [PATCH 11/24] Offer the ad removal from the settings section (#530) It was the last item in the toolbar menu on the landing screen, and with the open action gone it was the only one - a screen whose entire content is a list kept an overflow menu alive for a single row that belongs in that list. It now sits under the settings header, next to the catch-all switch, and the landing screen has no menu at all. The row is only built when there is something to sell: never in pro, where the purchase is implied, and not once it has been bought. LandingFragment asks the activity at render time rather than reading it from the state, because billing is set up by initializeProprietaryLibraries - which runs again after the play services dialog - and is not something the ViewModel could read off disk itself. That takes it out of the document view too, where the overflow menu carried it. What is left there is the snackbar that offers the purchase on entering fullscreen, which is untouched. Co-authored-by: Claude Opus 5 (1M context) --- .../droid/ui/activity/LandingFragment.kt | 18 +++++++++++++++ .../droid/ui/activity/MainActivity.kt | 23 ++++++++++--------- .../droid/ui/widget/LandingItem.kt | 1 + app/src/main/res/drawable/ic_block.xml | 10 ++++++++ app/src/main/res/menu/menu_main.xml | 4 ---- 5 files changed, 41 insertions(+), 15 deletions(-) create mode 100644 app/src/main/res/drawable/ic_block.xml 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 03d4a9d7949b..146e3c91b54a 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 @@ -315,6 +315,19 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { state.catchAllEnabled, ) ) + + // asked here rather than carried in the state: what billing knows lives on the activity, + // not in anything the ViewModel reads. it is only ever false in pro or after a purchase, + // both of which are settled long before the first refresh comes back + if (mainActivity.offersAdRemoval()) { + items.add( + LandingItem.Action( + LandingItem.ACTION_REMOVE_ADS, + R.string.menu_remove_ads, + R.drawable.ic_block, + ) + ) + } } /** "2 hours ago", or nothing at all for entries written before this was recorded. */ @@ -417,6 +430,11 @@ class LandingFragment : Fragment(), LandingAdapter.Listener { when (action) { LandingItem.ACTION_ADD_FOLDER -> addFolder() LandingItem.ACTION_UP -> viewModel.leaveFolder() + LandingItem.ACTION_REMOVE_ADS -> { + mainActivity.analyticsManager.report("settings_remove_ads") + + mainActivity.buyAdRemoval() + } } } diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt index a5688407183c..30779b51bdfb 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt @@ -365,10 +365,6 @@ class MainActivity : AppCompatActivity(), MenuProvider { override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { menuInflater.inflate(R.menu.menu_main, menu) - - if (billingManager.hasPurchased()) { - menu.findItem(R.id.menu_remove_ads).isVisible = false - } } // The play services availability dialog calls startActivityForResult() itself with a @@ -470,12 +466,6 @@ class MainActivity : AppCompatActivity(), MenuProvider { analyticsManager.report("menu_share") } - R.id.menu_remove_ads -> { - analyticsManager.report("menu_remove_ads") - - buyAdRemoval() - } - R.id.menu_fullscreen -> { if (fullscreen) { analyticsManager.report("menu_fullscreen_leave") @@ -572,7 +562,18 @@ class MainActivity : AppCompatActivity(), MenuProvider { ttsActionMode = null } - private fun buyAdRemoval() { + /** + * Whether the ad removal is still worth offering: never in pro, where the purchase is implied, + * and not once it has been bought. + * + * The landing screen asks rather than being told, because billing is set up by + * [initializeProprietaryLibraries] - which can run a second time, after the play services + * dialog - and it is not something the ViewModel could read off disk itself. + */ + fun offersAdRemoval(): Boolean = + ::billingManager.isInitialized && !billingManager.hasPurchased() + + fun buyAdRemoval() { analyticsManager.report(AnalyticsConstants.EVENT_ADD_TO_CART) // the play listing id is the applicationId, which stays at.tomtasche.reader.pro 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 e6d74cca2fb5..f94f2cc28e16 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 @@ -67,6 +67,7 @@ sealed class LandingItem { companion object { const val ACTION_ADD_FOLDER: Int = 1 const val ACTION_UP: Int = 2 + const val ACTION_REMOVE_ADS: Int = 3 const val SETTING_CATCH_ALL: Int = 1 } diff --git a/app/src/main/res/drawable/ic_block.xml b/app/src/main/res/drawable/ic_block.xml new file mode 100644 index 000000000000..53979f05ff4c --- /dev/null +++ b/app/src/main/res/drawable/ic_block.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/menu/menu_main.xml b/app/src/main/res/menu/menu_main.xml index 7e8de18e11dc..5e0e7c730bfe 100644 --- a/app/src/main/res/menu/menu_main.xml +++ b/app/src/main/res/menu/menu_main.xml @@ -56,8 +56,4 @@ android:visible="false" app:showAsAction="never" /> - From a0a6e1fdc9b972de6b1d5b7dcf1e1277c4c67d49 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 21:06:05 +0200 Subject: [PATCH 12/24] Put the document actions where the thumb is (#531) The toolbar menu is gone. What can be done with the open document is now two buttons over its bottom right corner: search, which is what a reader reaches for most, and one that unfolds the other seven with their names next to them. The top right corner of a modern phone is the one place a thumb holding it cannot reach, and it is where every one of these lived - four of them behind "More options", which says nothing about what is inside. Naming them costs a tap that the overflow charged anyway. DocumentActions is a view rather than a menu, so the actions are set whenever a load finishes instead of whenever something happens to invalidate the menu. That is the reason for the odd rule the old code lived by: prepareMenu ran from onCreateMenu only, and a document that finished loading does not invalidate anything, so which items were visible depended on what else had asked the toolbar to rebuild. There is no menu provider left in either class, and menu_main.xml is deleted. The rows scroll: seven of them do not fit above the buttons on a phone held sideways. They hide while an action mode is up - find, tts and edit each bring their own controls - and while fullscreen has the screen, which back still leaves exactly as before. Back folds them up first. Co-authored-by: Claude Opus 5 (1M context) --- .../droid/test/MainActivityTests.kt | 39 ++-- .../droid/ui/activity/DocumentFragment.kt | 168 ++++++++++----- .../droid/ui/activity/MainActivity.kt | 95 ++++++--- .../droid/ui/widget/DocumentActions.kt | 196 ++++++++++++++++++ .../res/drawable/bg_document_action_label.xml | 9 + app/src/main/res/drawable/ic_close.xml | 10 + app/src/main/res/drawable/ic_more_vert.xml | 10 + app/src/main/res/drawable/ic_open_in_new.xml | 10 + app/src/main/res/drawable/ic_print.xml | 10 + app/src/main/res/drawable/ic_share.xml | 10 + app/src/main/res/drawable/ic_volume_up.xml | 10 + app/src/main/res/layout/fragment_document.xml | 36 +++- .../main/res/layout/item_document_action.xml | 29 +++ .../main/res/layout/view_document_actions.xml | 64 ++++++ app/src/main/res/menu/menu_main.xml | 59 ------ app/src/main/res/values/colors.xml | 6 + app/src/main/res/values/strings.xml | 2 + 17 files changed, 586 insertions(+), 177 deletions(-) create mode 100644 app/src/main/java/app/opendocument/droid/ui/widget/DocumentActions.kt create mode 100644 app/src/main/res/drawable/bg_document_action_label.xml create mode 100644 app/src/main/res/drawable/ic_close.xml create mode 100644 app/src/main/res/drawable/ic_more_vert.xml create mode 100644 app/src/main/res/drawable/ic_open_in_new.xml create mode 100644 app/src/main/res/drawable/ic_print.xml create mode 100644 app/src/main/res/drawable/ic_share.xml create mode 100644 app/src/main/res/drawable/ic_volume_up.xml create mode 100644 app/src/main/res/layout/item_document_action.xml create mode 100644 app/src/main/res/layout/view_document_actions.xml delete mode 100644 app/src/main/res/menu/menu_main.xml diff --git a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt index ec18c4b0b3d1..7dc86df7dcf5 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt @@ -22,9 +22,7 @@ import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.intent.Intents import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction import androidx.test.espresso.matcher.ViewMatchers.isDisplayed -import androidx.test.espresso.matcher.ViewMatchers.isEnabled import androidx.test.espresso.matcher.ViewMatchers.withClassName -import androidx.test.espresso.matcher.ViewMatchers.withContentDescription import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -121,7 +119,11 @@ class MainActivityTests { // next onView will be blocked until the idling resource is idle, which now covers // the load itself and not just the picker round trip. - clickEditWithOverflowFallback() + waitForDocumentActions() + + unfoldDocumentActions() + + onView(withText(R.string.menu_edit)).check(matches(isDisplayed())) } @Test @@ -132,7 +134,7 @@ class MainActivityTests { // next onView will be blocked until the idling resource is idle, which now covers // the load itself and not just the picker round trip. - clickEditWithOverflowFallback() + waitForDocumentActions() // there used to be a 10s sleep here that asserted nothing, and it is why "testPDF // crashed" was an api 34 bucket of its own: the app is killed whenever play services @@ -185,8 +187,8 @@ class MainActivityTests { onView(withId(android.R.id.button1)).perform(click()) - // Check if edit button becomes available (indicating successful load) - clickEditWithOverflowFallback() + // Check if the document buttons become available (indicating successful load) + waitForDocumentActions() } @Test @@ -269,22 +271,15 @@ class MainActivityTests { .perform(click()) } - private fun clickEditWithOverflowFallback() { - onView(allOf(withId(R.id.menu_edit), withContentDescription("Edit document"), isEnabled())) - .withFailureHandler { _, _ -> - // fails on small screens, try again with overflow menu - onView(allOf(withContentDescription("More options"), isDisplayed())) - .perform(click()) - - onView( - allOf( - withId(R.id.menu_edit), - withContentDescription("Edit document"), - isDisplayed(), - ) - ) - .perform(click()) - } + // the buttons of the open document are up once it has loaded, so this blocks on the idling + // resource and then says whether anything came of the load. only the button that unfolds the + // rest is checked: what is inside it differs per format, a pdf cannot be edited. + private fun waitForDocumentActions() { + onView(withId(R.id.document_actions_more)).check(matches(isDisplayed())) + } + + private fun unfoldDocumentActions() { + onView(withId(R.id.document_actions_more)).perform(click()) } private fun recreate(activity: MainActivity): MainActivity? { diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt index 5fe7aa31c5bc..ed774bbe0b88 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt @@ -8,16 +8,13 @@ import android.os.Handler import android.os.Looper import android.text.InputType import android.view.LayoutInflater -import android.view.Menu -import android.view.MenuInflater -import android.view.MenuItem import android.view.View import android.view.ViewGroup import android.widget.EditText import android.widget.Toast +import androidx.activity.OnBackPressedCallback import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog -import androidx.core.view.MenuProvider import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider @@ -32,6 +29,7 @@ import app.opendocument.droid.nonfree.AnalyticsManager import app.opendocument.droid.nonfree.CrashManager import app.opendocument.droid.ui.OpenFileIdling import app.opendocument.droid.ui.SnackbarHelper +import app.opendocument.droid.ui.widget.DocumentActions import app.opendocument.droid.ui.widget.PageView import app.opendocument.droid.ui.widget.ProgressDialogFragment import com.google.android.material.tabs.TabLayout @@ -40,7 +38,7 @@ import java.io.File import java.io.FileNotFoundException import java.io.IOException -class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider { +class DocumentFragment : Fragment(), LoaderService.LoaderListener { private lateinit var mainHandler: Handler @@ -56,7 +54,15 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider var pageView: PageView? = null private set - private var menu: Menu? = null + private lateinit var actions: DocumentActions + + /** Folding the actions back up is what back does first, while they are unfolded. */ + private val actionsBackCallback = + object : OnBackPressedCallback(false) { + override fun handleOnBackPressed() { + actions.collapse() + } + } private var resultOnStart: FileLoader.Result? = null private var errorOnStart: Throwable? = null @@ -135,11 +141,7 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?, - ): View? { - requireActivity().addMenuProvider(this, requireActivity()) - - return inflater.inflate(R.layout.fragment_document, container, false) - } + ): View? = inflater.inflate(R.layout.fragment_document, container, false) private fun initializePageView() { pageView?.let { @@ -186,6 +188,16 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider analyticsManager = mainActivity.analyticsManager crashManager = mainActivity.crashManager + actions = view.findViewById(R.id.document_actions) + actions.listener = DocumentActions.Listener { action -> + mainActivity.onDocumentAction(action) + } + actions.expandedListener = { expanded -> actionsBackCallback.isEnabled = expanded } + + // on viewLifecycleOwner, so it stacks above the activity's own callback - the dispatcher + // runs the most recently added enabled callback first + mainActivity.onBackPressedDispatcher.addCallback(viewLifecycleOwner, actionsBackCallback) + serviceQueue = mainActivity.loaderServiceQueue serviceQueue.addToQueue { service -> service.setListener(this) } @@ -213,6 +225,7 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider prepareLoad(lastResult.loaderType, false) restoreTabs(lastResult) + prepareActions(lastResult) } pageView?.restoreState(savedInstanceState) @@ -244,24 +257,6 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider tabLayout.addOnTabSelectedListener(tabSelectedListener) } - override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { - this.menu = menu - - menu.findItem(R.id.menu_fullscreen).isVisible = true - menu.findItem(R.id.menu_open_with).isVisible = true - menu.findItem(R.id.menu_share).isVisible = true - menu.findItem(R.id.menu_save).isVisible = true - menu.findItem(R.id.menu_print).isVisible = true - - // the other menu items are dynamically enabled based on the loaded document - state.lastResult?.let { prepareMenu(it.loaderType, it.options.fileType) } - } - - override fun onMenuItemSelected(menuItem: MenuItem): Boolean { - // TODO: handle menu item clicks here. currently done in Activity for historical reasons - return false - } - override fun onSaveInstanceState(outState: Bundle) { super.onSaveInstanceState(outState) @@ -360,7 +355,10 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider } private fun unload() { - toggleDocumentMenu(false) + // guarded like resetTabs below: a load can fail before there is a view to put right + if (::actions.isInitialized) { + actions.setActions(null, emptyList()) + } resetTabs() } @@ -375,32 +373,17 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider state.lastSelectedTab = -1 } - private fun toggleDocumentMenu(enabled: Boolean) { - toggleDocumentMenu(enabled, enabled) - } - - private fun toggleDocumentMenu(enabled: Boolean, editEnabled: Boolean) { - val menu = this.menu - if (menu == null) { - val activity = activity - val pageView = this.pageView - if (activity == null || activity.isFinishing || pageView == null) { - return - } - - // menu is not set when loadUri is called via onStart, retry later - pageView.post { toggleDocumentMenu(enabled, editEnabled) } - - return - } - - menu.findItem(R.id.menu_edit).isVisible = editEnabled - - menu.findItem(R.id.menu_search).isVisible = enabled - menu.findItem(R.id.menu_tts).isVisible = enabled - } + /** + * Puts the buttons of the loaded document up, in the order they are worth reaching for. + * + * Called for every result rather than from a menu callback, which is what the toolbar version + * relied on: the menu was only rebuilt when something happened to invalidate it, and a document + * that finished loading is not one of those things. + */ + private fun prepareActions(result: FileLoader.Result) { + val loaderType = result.loaderType + val fileType = result.options.fileType - private fun prepareMenu(loaderType: FileLoader.LoaderType, fileType: String?) { var isEditEnabled = false var isDarkModeSupported = true @@ -423,10 +406,81 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider } } - toggleDocumentMenu(true, isEditEnabled) + val unfolding = ArrayList() + if (isEditEnabled) { + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_EDIT, + R.string.menu_edit, + R.drawable.ic_edit, + ) + ) + } + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_TTS, + R.string.menu_tts, + R.drawable.ic_volume_up, + ) + ) + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_SHARE, + R.string.menu_share, + R.drawable.ic_share, + ) + ) + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_PRINT, + R.string.menu_cloud_print, + R.drawable.ic_print, + ) + ) + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_OPEN_WITH, + R.string.menu_open_with, + R.drawable.ic_open_in_new, + ) + ) + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_SAVE, + R.string.action_edit_save, + R.drawable.ic_save, + ) + ) + unfolding.add( + DocumentActions.Action( + DocumentActions.ACTION_FULLSCREEN, + R.string.menu_fullscreen, + R.drawable.ic_fullscreen, + ) + ) + + actions.setActions( + DocumentActions.Action( + DocumentActions.ACTION_SEARCH, + R.string.menu_search, + R.drawable.ic_search, + ), + unfolding, + ) + pageView?.toggleDarkMode(isDarkModeSupported) } + /** Takes the buttons away while the document has the screen to itself. */ + fun setActionsVisible(visible: Boolean) { + if (!::actions.isInitialized) { + return + } + + actions.collapse() + actions.visibility = if (visible) View.VISIBLE else View.GONE + } + private fun requestInAppRating(activity: Activity) { analyticsManager.report("in_app_review_eligible") @@ -494,6 +548,8 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider loadData(result.partUris[0].toString()) } + prepareActions(result) + if ( result.loaderType == FileLoader.LoaderType.RAW || result.loaderType == FileLoader.LoaderType.ONLINE diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt index 30779b51bdfb..0545b4029675 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt @@ -12,15 +12,11 @@ import android.os.Handler import android.os.IBinder import android.os.Looper import android.view.ActionMode -import android.view.Menu -import android.view.MenuInflater -import android.view.MenuItem import android.view.View import android.widget.LinearLayout import androidx.activity.OnBackPressedCallback import androidx.activity.result.contract.ActivityResultContracts import androidx.appcompat.app.AppCompatActivity -import androidx.core.view.MenuProvider import androidx.core.view.ViewCompat import androidx.core.view.WindowCompat import androidx.core.view.WindowInsetsCompat @@ -41,10 +37,11 @@ import app.opendocument.droid.ui.FindActionModeCallback import app.opendocument.droid.ui.OpenFileIdling import app.opendocument.droid.ui.SnackbarHelper import app.opendocument.droid.ui.TtsActionModeCallback +import app.opendocument.droid.ui.widget.DocumentActions import com.google.android.gms.common.ConnectionResult import com.google.android.gms.common.GoogleApiAvailability -class MainActivity : AppCompatActivity(), MenuProvider { +class MainActivity : AppCompatActivity() { private lateinit var handler: Handler @@ -255,8 +252,6 @@ class MainActivity : AppCompatActivity(), MenuProvider { analyticsManager.setCurrentScreen(this, "screen_main") } - - addMenuProvider(this, this) } override fun onStart() { @@ -363,10 +358,6 @@ class MainActivity : AppCompatActivity(), MenuProvider { ) } - override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { - menuInflater.inflate(R.menu.menu_main, menu) - } - // The play services availability dialog calls startActivityForResult() itself with a // request code we hand it, so its result cannot be routed through an // ActivityResultLauncher. Everything the app launches on its own goes through the @@ -435,11 +426,16 @@ class MainActivity : AppCompatActivity(), MenuProvider { documentFragment.loadUri(uri, isPersistentUri) } - override fun onMenuItemSelected(item: MenuItem): Boolean { + /** + * A button of the open document was tapped. These used to be the toolbar menu, and the ids are + * now [DocumentActions]' own - the handling stays here, where the action modes and the printing + * manager already live. + */ + fun onDocumentAction(action: Int) { val documentFragment = this.documentFragment - when (item.itemId) { - R.id.menu_search -> { + when (action) { + DocumentActions.ACTION_SEARCH -> { val findActionModeCallback = FindActionModeCallback(this) documentFragment?.pageView?.let { findActionModeCallback.setWebView(it) } startSupportActionMode(findActionModeCallback) @@ -448,25 +444,25 @@ class MainActivity : AppCompatActivity(), MenuProvider { analyticsManager.report(AnalyticsConstants.EVENT_SEARCH) } - R.id.menu_open_with -> { + DocumentActions.ACTION_OPEN_WITH -> { documentFragment?.openWith(this) analyticsManager.report("menu_open_with") } - R.id.menu_save -> { + DocumentActions.ACTION_SAVE -> { documentFragment?.prepareSave({ requestSave() }, true) analyticsManager.report("menu_save") } - R.id.menu_share -> { + DocumentActions.ACTION_SHARE -> { documentFragment?.share(this) analyticsManager.report("menu_share") } - R.id.menu_fullscreen -> { + DocumentActions.ACTION_FULLSCREEN -> { if (fullscreen) { analyticsManager.report("menu_fullscreen_leave") @@ -496,9 +492,11 @@ class MainActivity : AppCompatActivity(), MenuProvider { } fullscreen = !fullscreen + + updateDocumentActionsVisible() } - R.id.menu_print -> { + DocumentActions.ACTION_PRINT -> { analyticsManager.report("menu_print") documentFragment?.pageView?.let { pageView -> @@ -508,7 +506,7 @@ class MainActivity : AppCompatActivity(), MenuProvider { } } - R.id.menu_tts -> { + DocumentActions.ACTION_TTS -> { analyticsManager.report("menu_tts") documentFragment?.pageView?.let { pageView -> @@ -519,7 +517,7 @@ class MainActivity : AppCompatActivity(), MenuProvider { } } - R.id.menu_edit -> { + DocumentActions.ACTION_EDIT -> { analyticsManager.report("menu_edit") documentFragment?.let { fragment -> @@ -529,11 +527,7 @@ class MainActivity : AppCompatActivity(), MenuProvider { startSupportActionMode(editActionMode) } } - - else -> return super.onOptionsItemSelected(item) } - - return true } private fun offerPurchase() { @@ -555,13 +549,56 @@ class MainActivity : AppCompatActivity(), MenuProvider { ) } - override fun onActionModeFinished(mode: ActionMode?) { - super.onActionModeFinished(mode) + /** + * The buttons of the document are only up while nothing else is using the screen: an action + * mode has taken the toolbar over and brought its own controls, and fullscreen is for reading. + * + * Counted rather than a flag, because the two kinds of action mode overlap - selecting text in + * the page starts a framework one on top of the appcompat one that edit mode is. + */ + private var actionModes = 0 + + private fun updateDocumentActionsVisible() { + documentFragment?.setActionsVisible(actionModes == 0 && !fullscreen) + } + + // the appcompat ones, which is what startSupportActionMode() raises: find, tts and edit + override fun onSupportActionModeStarted(mode: androidx.appcompat.view.ActionMode) { + super.onSupportActionModeStarted(mode) + + actionModes++ + + updateDocumentActionsVisible() + } + + override fun onSupportActionModeFinished(mode: androidx.appcompat.view.ActionMode) { + super.onSupportActionModeFinished(mode) + + actionModes-- + + updateDocumentActionsVisible() editActionMode = null ttsActionMode = null } + // and the framework ones, which is what selecting text in the page raises + override fun onActionModeStarted(mode: ActionMode?) { + super.onActionModeStarted(mode) + + actionModes++ + + updateDocumentActionsVisible() + } + + override fun onActionModeFinished(mode: ActionMode?) { + super.onActionModeFinished(mode) + + actionModes-- + + updateDocumentActionsVisible() + } + /** * Whether the ad removal is still worth offering: never in pro, where the purchase is implied, * and not once it has been bought. @@ -598,13 +635,13 @@ class MainActivity : AppCompatActivity(), MenuProvider { fullscreen = false + updateDocumentActionsVisible() + analyticsManager.report("fullscreen_end") } private fun closeDocument() { documentFragment?.let { fragment -> - removeMenuProvider(fragment) - supportFragmentManager.beginTransaction().remove(fragment).commitNow() documentFragment = null diff --git a/app/src/main/java/app/opendocument/droid/ui/widget/DocumentActions.kt b/app/src/main/java/app/opendocument/droid/ui/widget/DocumentActions.kt new file mode 100644 index 000000000000..7d3572183a31 --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/ui/widget/DocumentActions.kt @@ -0,0 +1,196 @@ +package app.opendocument.droid.ui.widget + +import android.content.Context +import android.util.AttributeSet +import android.view.LayoutInflater +import android.view.View +import android.widget.FrameLayout +import android.widget.LinearLayout +import android.widget.ScrollView +import android.widget.TextView +import androidx.annotation.DrawableRes +import androidx.annotation.StringRes +import app.opendocument.droid.R +import com.google.android.material.floatingactionbutton.FloatingActionButton + +/** + * What can be done with the open document, as buttons over the bottom right corner of it: one for + * the action worth its own button, and one that unfolds the rest. + * + * This is what the toolbar menu used to be. A document is read with the phone in one hand, and the + * top right corner of a modern screen is the one place a thumb cannot reach - so the actions sit + * where the thumb already is, and the ones that were hidden behind "More options" now say what they + * are. + * + * Material ships no speed dial component (the one it had was never brought over to Material 3), so + * the rows are built here from [R.layout.item_document_action]. + */ +class DocumentActions(context: Context, attributeSet: AttributeSet?) : + FrameLayout(context, attributeSet) { + + /** One button, identified by one of the ACTION_ ids the caller gets back when it is tapped. */ + class Action(val id: Int, @param:StringRes val label: Int, @param:DrawableRes val icon: Int) + + fun interface Listener { + fun onDocumentActionClicked(id: Int) + } + + var listener: Listener? = null + + /** Told whenever the actions unfold or fold back up, so back can close them. */ + var expandedListener: ((Boolean) -> Unit)? = null + + private val scrim: View + private val rowsScroll: ScrollView + private val rows: LinearLayout + private val primaryButton: FloatingActionButton + private val moreButton: FloatingActionButton + + var isExpanded: Boolean = false + private set + + init { + LayoutInflater.from(context).inflate(R.layout.view_document_actions, this, true) + + scrim = findViewById(R.id.document_actions_scrim) + rowsScroll = findViewById(R.id.document_actions_rows_scroll) + rows = findViewById(R.id.document_actions_rows) + primaryButton = findViewById(R.id.document_actions_primary) + moreButton = findViewById(R.id.document_actions_more) + + scrim.setOnClickListener { collapse() } + moreButton.setOnClickListener { if (isExpanded) collapse() else expand() } + + setActions(null, emptyList()) + } + + /** + * What the document can do right now. [primary] gets a button of its own, the rest unfold out + * of the second one, the first of them closest to it - so the order is most wanted first. + * + * Nothing and an empty list take the buttons away entirely, which is what a document that + * failed to load leaves behind. + */ + fun setActions(primary: Action?, unfolding: List) { + collapse() + + if (primary == null) { + primaryButton.visibility = View.GONE + } else { + primaryButton.visibility = View.VISIBLE + primaryButton.setImageResource(primary.icon) + primaryButton.contentDescription = context.getString(primary.label) + primaryButton.setOnClickListener { listener?.onDocumentActionClicked(primary.id) } + } + + rows.removeAllViews() + + // added back to front, because the row added last is the one that ends up at the bottom + // of the column, right above the button they unfold from + for (action in unfolding.asReversed()) { + rows.addView(newRow(action)) + } + + moreButton.visibility = if (unfolding.isEmpty()) View.GONE else View.VISIBLE + } + + private fun newRow(action: Action): View { + val row = LayoutInflater.from(context).inflate(R.layout.item_document_action, rows, false) + + val label: TextView = row.findViewById(R.id.document_action_label) + label.setText(action.label) + + val button: FloatingActionButton = row.findViewById(R.id.document_action_button) + button.setImageResource(action.icon) + button.contentDescription = context.getString(action.label) + + // the button is only the visible half of the row: the label is as much of a target + val click = OnClickListener { + collapse() + + listener?.onDocumentActionClicked(action.id) + } + row.setOnClickListener(click) + button.setOnClickListener(click) + + return row + } + + private fun expand() { + if (isExpanded || rows.childCount == 0) { + return + } + + isExpanded = true + + // a fold that was still fading out would otherwise hide all of this again when it ends + scrim.animate().cancel() + rowsScroll.animate().cancel() + + scrim.alpha = 0f + scrim.visibility = View.VISIBLE + scrim.animate().alpha(1f).setDuration(ANIMATION_MILLIS) + + rowsScroll.alpha = 1f + rowsScroll.visibility = View.VISIBLE + + // the rows hang off the bottom of the scroller, and that is the end the buttons are at + rowsScroll.post { rowsScroll.fullScroll(View.FOCUS_DOWN) } + + val rise = RISE_DP * resources.displayMetrics.density + for (index in 0 until rows.childCount) { + val row = rows.getChildAt(index) + + row.alpha = 0f + row.translationY = rise + row.animate().alpha(1f).translationY(0f).setDuration(ANIMATION_MILLIS) + } + + moreButton.setImageResource(R.drawable.ic_close) + moreButton.contentDescription = context.getString(R.string.document_actions_close) + + expandedListener?.invoke(true) + } + + fun collapse() { + if (!isExpanded) { + // also the state setActions() starts from, so the views have to be put right anyway + scrim.visibility = View.GONE + rowsScroll.visibility = View.GONE + + return + } + + isExpanded = false + + scrim.animate().alpha(0f).setDuration(ANIMATION_MILLIS).withEndAction { + scrim.visibility = View.GONE + } + + rowsScroll.animate().alpha(0f).setDuration(ANIMATION_MILLIS).withEndAction { + rowsScroll.visibility = View.GONE + rowsScroll.alpha = 1f + } + + moreButton.setImageResource(R.drawable.ic_more_vert) + moreButton.contentDescription = context.getString(R.string.document_actions_more) + + expandedListener?.invoke(false) + } + + companion object { + const val ACTION_SEARCH: Int = 1 + const val ACTION_EDIT: Int = 2 + const val ACTION_TTS: Int = 3 + const val ACTION_SHARE: Int = 4 + const val ACTION_PRINT: Int = 5 + const val ACTION_OPEN_WITH: Int = 6 + const val ACTION_SAVE: Int = 7 + const val ACTION_FULLSCREEN: Int = 8 + + private const val ANIMATION_MILLIS = 150L + + /** How far below its place a row starts out, so the column rises as it appears. */ + private const val RISE_DP = 16f + } +} diff --git a/app/src/main/res/drawable/bg_document_action_label.xml b/app/src/main/res/drawable/bg_document_action_label.xml new file mode 100644 index 000000000000..4eaa6a035da3 --- /dev/null +++ b/app/src/main/res/drawable/bg_document_action_label.xml @@ -0,0 +1,9 @@ + + + + + + + + diff --git a/app/src/main/res/drawable/ic_close.xml b/app/src/main/res/drawable/ic_close.xml new file mode 100644 index 000000000000..7a52b3b744f4 --- /dev/null +++ b/app/src/main/res/drawable/ic_close.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/drawable/ic_more_vert.xml b/app/src/main/res/drawable/ic_more_vert.xml new file mode 100644 index 000000000000..2d31aa6ebe86 --- /dev/null +++ b/app/src/main/res/drawable/ic_more_vert.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/drawable/ic_open_in_new.xml b/app/src/main/res/drawable/ic_open_in_new.xml new file mode 100644 index 000000000000..1bd20d427424 --- /dev/null +++ b/app/src/main/res/drawable/ic_open_in_new.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/drawable/ic_print.xml b/app/src/main/res/drawable/ic_print.xml new file mode 100644 index 000000000000..5175220eb535 --- /dev/null +++ b/app/src/main/res/drawable/ic_print.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/drawable/ic_share.xml b/app/src/main/res/drawable/ic_share.xml new file mode 100644 index 000000000000..b9996940831c --- /dev/null +++ b/app/src/main/res/drawable/ic_share.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/drawable/ic_volume_up.xml b/app/src/main/res/drawable/ic_volume_up.xml new file mode 100644 index 000000000000..62f0a8e4cc05 --- /dev/null +++ b/app/src/main/res/drawable/ic_volume_up.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/layout/fragment_document.xml b/app/src/main/res/layout/fragment_document.xml index ff854beddbc6..774974211d26 100644 --- a/app/src/main/res/layout/fragment_document.xml +++ b/app/src/main/res/layout/fragment_document.xml @@ -1,20 +1,34 @@ - - - + android:layout_height="match_parent" + android:orientation="vertical"> - + + + - + diff --git a/app/src/main/res/layout/item_document_action.xml b/app/src/main/res/layout/item_document_action.xml new file mode 100644 index 000000000000..2cdcae9f12a5 --- /dev/null +++ b/app/src/main/res/layout/item_document_action.xml @@ -0,0 +1,29 @@ + + + + + + + + diff --git a/app/src/main/res/layout/view_document_actions.xml b/app/src/main/res/layout/view_document_actions.xml new file mode 100644 index 000000000000..0823f936e4f7 --- /dev/null +++ b/app/src/main/res/layout/view_document_actions.xml @@ -0,0 +1,64 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/app/src/main/res/menu/menu_main.xml b/app/src/main/res/menu/menu_main.xml deleted file mode 100644 index 5e0e7c730bfe..000000000000 --- a/app/src/main/res/menu/menu_main.xml +++ /dev/null @@ -1,59 +0,0 @@ - - - - - - - - - - - - - - - - - - - - diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index 082052a015f3..012ea56e1d19 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -48,4 +48,10 @@ --> #FFFFFF + + #52000000 + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c36f2b8818a1..d7e2b05ec1d8 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -53,6 +53,8 @@ Up one level Nothing here yet Open a document to get started. It will show up here next time. + More actions + Close actions Reading… Initializing TTS… Ready! From fc10c7249715e1f4360919648feed7951d5c1bdb Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 28 Jul 2026 21:07:36 +0200 Subject: [PATCH 13/24] Take the empty toolbar off the screen (#542) Nothing has been left in the options menu: the landing screen carries its own headers, the open action went away with the direct picker, the ad removal moved into the settings section and the document's actions moved down to the buttons by the thumb. What stayed behind is an empty strip of colour at the top of every screen. The bar cannot simply be dropped though - it is where the action modes appear. find, tts and edit are all raised with startSupportActionMode(), and appcompat puts them in the ActionBar's own container, in place of the toolbar. So rather than moving to a .NoActionBar theme, MainActivity hides the bar on start and lets appcompat bring the container back for as long as a mode is up: checkShowingFlags() shows it whenever an action mode is showing, whatever the app asked for, and takes it away again when the mode finishes. That also leaves the window insets alone. ActionBarOverlayLayout keeps the system bar insets off the content and reports the toolbar's height on to the app as an inset of its own, which is what MainActivity's listener pads main_root with - so the find bar pushes the page down by exactly its own height, and nothing pads anything while no mode is up. A NoActionBar theme would swap that decor for a FitWindowsLinearLayout, which pads itself by the system bars on top of the padding MainActivity already applies. The fullscreen hide()/show() pair goes with it: with the bar hidden for good there is nothing left for it to toggle, and show() would have put the empty bar back on leaving fullscreen. Co-authored-by: Claude Opus 5 (1M context) --- .../opendocument/droid/ui/activity/MainActivity.kt | 12 +++++++----- app/src/main/res/values/themes.xml | 9 +++++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt index 0545b4029675..d30765784db4 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/MainActivity.kt @@ -174,7 +174,13 @@ class MainActivity : AppCompatActivity() { WindowInsetsCompat.CONSUMED } - title = "" + // nothing lives in the toolbar any more - the landing screen has its own header and the + // document its buttons - so the bar would just be an empty strip of colour. Hiding it + // rather than moving to a .NoActionBar theme keeps it as the host the action modes are + // raised in: appcompat shows the container again for as long as one is up, and takes it + // back down afterwards, so find, tts and edit still get their bar without the app + // having to put one on screen itself. + supportActionBar?.hide() onBackPressedDispatcher.addCallback(this, backCallback) @@ -476,8 +482,6 @@ class MainActivity : AppCompatActivity() { WindowInsetsControllerCompat.BEHAVIOR_SHOW_TRANSIENT_BARS_BY_SWIPE insetsController.hide(WindowInsetsCompat.Type.statusBars()) - supportActionBar?.hide() - // delay offer to wait for fullscreen animation to finish handler.postDelayed( { @@ -628,8 +632,6 @@ class MainActivity : AppCompatActivity() { return } - supportActionBar?.show() - WindowCompat.getInsetsController(window, window.decorView) .show(WindowInsetsCompat.Type.statusBars()) diff --git a/app/src/main/res/values/themes.xml b/app/src/main/res/values/themes.xml index 25eb44bca7b9..0dec01cdbb63 100644 --- a/app/src/main/res/values/themes.xml +++ b/app/src/main/res/values/themes.xml @@ -6,8 +6,13 @@ have to add what is specific to them. Copying the whole theme per qualifier is what let the colours drift apart before. - Not a .NoActionBar variant: there is no Toolbar view anywhere, the app relies on the - default ActionBar and drives it through supportActionBar / startSupportActionMode. + Not a .NoActionBar variant, even though no screen puts anything in the bar any more: + the ActionBar's container is what startSupportActionMode() raises find, tts and edit + in, and appcompat only shows it for as long as one of them is up. MainActivity hides + the bar itself on start, so the container is off screen the rest of the time and the + window inset math stays the one ActionBarOverlayLayout does - which a NoActionBar + theme would swap for a decor that pads itself, on top of the padding MainActivity + applies. -->