Skip to content

Put the document actions where the thumb is #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andiwand merged 1 commit into from
Jul 28, 2026
Merged

Put the document actions where the thumb is #531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 17 additions & 22 deletions app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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? {
Expand Down
168 changes: 112 additions & 56 deletions app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) }

Expand Down Expand Up @@ -213,6 +225,7 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider

prepareLoad(lastResult.loaderType, false)
restoreTabs(lastResult)
prepareActions(lastResult)
}

pageView?.restoreState(savedInstanceState)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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()
}
Expand All @@ -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

Expand All @@ -423,10 +406,81 @@ class DocumentFragment : Fragment(), LoaderService.LoaderListener, MenuProvider
}
}

toggleDocumentMenu(true, isEditEnabled)
val unfolding = ArrayList<DocumentActions.Action>()
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")

Expand Down Expand Up @@ -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
Expand Down
Loading
Loading