Skip to content
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.

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

[GSoC'24] Add Right-Click Support to Long Click Listeners #16712

Merged
merged 1 commit into from
Jul 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 7 additions & 5 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import android.view.KeyEvent
import android.view.Menu
import android.view.MenuItem
import android.view.View
import android.view.View.OnLongClickListener
import android.view.ViewPropertyAnimator
import android.widget.Filterable
import android.widget.ImageButton
Expand Down Expand Up @@ -438,9 +437,13 @@ open class DeckPicker :
}
}

private val deckLongClickListener = OnLongClickListener { v ->
private val deckContextAndLongClickListener = OnContextAndLongClickListener { v ->
val deckId = v.tag as DeckId
Timber.i("DeckPicker:: Long tapped on deck with id %d", deckId)
showDeckPickerContextMenu(deckId)
true
}

private fun showDeckPickerContextMenu(deckId: DeckId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with having a function that is just a function. But in this case, I think you should not have the value deckContextAndLongClickListener and directly move its content at the only place where it's used

launchCatchingTask {
val (deckName, isDynamic, hasBuriedInDeck) = withCol {
decks.select(deckId)
Expand All @@ -460,7 +463,6 @@ open class DeckPicker :
)
)
}
true
}

private val notificationPermissionLauncher = registerForActivityResult(ActivityResultContracts.RequestPermission()) {
Expand Down Expand Up @@ -554,7 +556,7 @@ open class DeckPicker :
setDeckClickListener(deckClickListener)
setCountsClickListener(countsClickListener)
setDeckExpanderClickListener(deckExpanderClickListener)
setDeckLongClickListener(deckLongClickListener)
setDeckContextAndLongClickListener(deckContextAndLongClickListener)
enablePartialTransparencyForBackground(hasDeckPickerBackground)
}
recyclerView.adapter = deckListAdapter
Expand Down
3 changes: 2 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/NoteEditor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import com.google.android.material.snackbar.Snackbar
import com.ichi2.anim.ActivityTransitionAnimation
import com.ichi2.anki.CollectionManager.TR
import com.ichi2.anki.CollectionManager.withCol
import com.ichi2.anki.OnContextAndLongClickListener.Companion.setOnContextAndLongClickListener
import com.ichi2.anki.bottomsheet.ImageOcclusionBottomSheetFragment
import com.ichi2.anki.dialogs.ConfirmationDialog
import com.ichi2.anki.dialogs.DeckSelectionDialog.DeckSelectionListener
Expand Down Expand Up @@ -2062,7 +2063,7 @@ class NoteEditor : AnkiActivity(), DeckSelectionListener, SubtitleListener, Tags

// Allow Ctrl + 1...Ctrl + 0 for item 10.
v.tag = (visualIndex % 10).toString()
v.setOnLongClickListener {
v.setOnContextAndLongClickListener {
displayEditToolbarDialog(b)
true
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/***************************************************************************************
* *
* Copyright (c) 2024 Sanjay Sargam <sargamsanjaykumar@gmail.com> *
* *
* This program is free software; you can redistribute it and/or modify it under *
* the terms of the GNU General Public License as published by the Free Software *
* Foundation; either version 3 of the License, or (at your option) any later *
* version. *
* *
* This program is distributed in the hope that it will be useful, but WITHOUT ANY *
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A *
* PARTICULAR PURPOSE. See the GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License along with *
* this program. If not, see <http://www.gnu.org/licenses/>. *
****************************************************************************************/

package com.ichi2.anki

import android.view.View
import timber.log.Timber

/**
* A listener that has the same action for both "context click" (i.e., mostly right-click) and "long click" (i.e., holding the finger on the view).
david-allison marked this conversation as resolved.
Show resolved Hide resolved
david-allison marked this conversation as resolved.
Show resolved Hide resolved
*
* * Note: In some contexts, a long press (long click) is expected to be informational, whereas a right-click (context click) is expected to be functional.
* * Ensure that using the same action for both is appropriate for your use case.
*/
fun interface OnContextAndLongClickListener : View.OnContextClickListener, View.OnLongClickListener {
/**
* The action to do for both contextClick and long click
* @returns whether the operation was successful
*/
fun onAction(v: View): Boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As usual, I'd like documentation. It can be very short, such as

The action to do for both contextClick and long click
@returns whether the operation was succesful


override fun onContextClick(v: View): Boolean {
Timber.i("${this.javaClass}: user context clicked")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, when I state "something such as", or similar requirement, you don't have to copy-paste it as-is. I don't guarantee that it's working properly.
In particular, if you tested the Timber, you'd see it gives

class com.ichi2.anki.DeckPicker$$ExternalSyntheticLambda6: user long clicked

Which to be honest I only realized because I could not guess what would be the class of this with your implementation.

I admit that I have no great idea about how to solve that. I would have loved to have an extra parameter which is a string value, but that would make the code far less readable, since we could not use only the declaration as a single function.
Worse, the TagsArrayAdapter is not easy to correct

As I've no better idea, I guess we can leave it as is. Unless someone wiser find a good solution that don't require too much work.
I doubt it'll really impede debugging, given the small number of place it's used in each file.

return onAction(v)
}

override fun onLongClick(v: View): Boolean {
Timber.i("${this.javaClass}: user long clicked")
return onAction(v)
}

companion object {
/**
* Ensures [this] gets both a long click and a context click listener.
* @see View.setOnLongClickListener
* @see View.setOnContextClickListener
*/
fun View.setOnContextAndLongClickListener(listener: OnContextAndLongClickListener?) {
setOnLongClickListener(listener)
setOnContextClickListener(listener)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import androidx.recyclerview.widget.RecyclerView
import anki.decks.DeckTreeNode
import com.ichi2.anki.CollectionManager.withCol
import com.ichi2.anki.DeckSpinnerSelection
import com.ichi2.anki.OnContextAndLongClickListener.Companion.setOnContextAndLongClickListener
import com.ichi2.anki.R
import com.ichi2.anki.analytics.AnalyticsDialogFragment
import com.ichi2.anki.dialogs.DeckSelectionDialog.DecksArrayAdapter.DecksFilter
Expand Down Expand Up @@ -179,7 +180,20 @@ open class DeckSelectionDialog : AnalyticsDialogFragment() {
}
}

private fun showSubDeckDialog(parentDeckPath: String) {
/**
* Displays a dialog to create a subdeck under the specified parent deck.
*
* If the `deckID` is equal to `DeckSpinnerSelection.ALL_DECKS_ID`, a toast message is shown
* indicating that a subdeck cannot be created for "All Decks," and the dialog is not displayed.
*
* @param parentDeckPath The path of the parent deck under which the subdeck will be created.
* @param deckID The ID of the deck where the subdeck should be created.
*/
private fun showSubDeckDialog(parentDeckPath: String, deckID: DeckId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some javadoc.
It used to be very clear what this method was doing. Now that it may not open the subdeck dialog, it's important to note this special case

if (deckID == DeckSpinnerSelection.ALL_DECKS_ID) {
context?.let { showThemedToast(it, R.string.cannot_create_subdeck_for_all_decks, true) }
return
}
launchCatchingTask {
val parentId = withCol { decks.id(parentDeckPath) }
val createDeckDialog = CreateDeckDialog(requireActivity(), R.string.create_subdeck, CreateDeckDialog.DeckDialogType.SUB_DECK, parentId)
Expand Down Expand Up @@ -265,12 +279,9 @@ open class DeckSelectionDialog : AnalyticsDialogFragment() {
expander.setOnClickListener {
toggleExpansion(deckID)
}
deckHolder.setOnLongClickListener { // creating sub deck with parent deck path
if (deckID == DeckSpinnerSelection.ALL_DECKS_ID) {
context?.let { showThemedToast(it, R.string.cannot_create_subdeck_for_all_decks, true) }
} else {
showSubDeckDialog(deckName)
}
deckHolder.setOnContextAndLongClickListener {
// creating sub deck with parent deck path
showSubDeckDialog(deckName, deckID)
true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import android.widget.LinearLayout
import android.widget.TextView
import androidx.annotation.VisibleForTesting
import androidx.recyclerview.widget.RecyclerView
import com.ichi2.anki.OnContextAndLongClickListener
import com.ichi2.anki.OnContextAndLongClickListener.Companion.setOnContextAndLongClickListener
import com.ichi2.anki.R
import com.ichi2.annotations.NeedsTest
import com.ichi2.ui.CheckBoxTriStates
Expand Down Expand Up @@ -224,10 +226,10 @@ class TagsArrayAdapter(private val tags: TagsList, private val resources: Resour
private val tagToIsExpanded: HashMap<String, Boolean>

/**
* Long click listener for each tag item. Used to add a subtag for the clicked tag.
* Context and Long click listener for each tag item. Used to add a subtag for the clicked tag.
* The full tag is passed through View.tag
*/
var tagLongClickListener: View.OnLongClickListener? = null
var tagContextAndLongClickListener: OnContextAndLongClickListener? = null

fun sortData() {
tags.sort()
Expand Down Expand Up @@ -266,8 +268,8 @@ class TagsArrayAdapter(private val tags: TagsList, private val resources: Resour
vh.checkBoxView.refreshDrawableState()
}
}
// long clicking a tag opens the add tag dialog with the current tag as the prefix
vh.itemView.setOnLongClickListener(tagLongClickListener)
// context and long clicking a tag opens the add tag dialog with the current tag as the prefix
vh.itemView.setOnContextAndLongClickListener(tagContextAndLongClickListener)
return vh
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import androidx.core.os.BundleCompat
import androidx.core.os.bundleOf
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import com.ichi2.anki.OnContextAndLongClickListener
import com.ichi2.anki.R
import com.ichi2.anki.analytics.AnalyticsDialogFragment
import com.ichi2.anki.model.CardStateFilter
Expand Down Expand Up @@ -189,14 +190,15 @@ class TagsDialog : AnalyticsDialogFragment {
dialogTitle = resources.getString(R.string.card_details_tags)
optionsGroup.visibility = View.GONE
positiveText = getString(R.string.dialog_ok)
tagsArrayAdapter!!.tagLongClickListener = View.OnLongClickListener { v ->
createAddTagDialog(v.tag as String)
true
}
tagsArrayAdapter!!.tagContextAndLongClickListener =
OnContextAndLongClickListener { v ->
createAddTagDialog(v.tag as String)
true
}
} else {
dialogTitle = resources.getString(R.string.studyoptions_limit_select_tags)
positiveText = getString(R.string.select)
tagsArrayAdapter!!.tagLongClickListener = View.OnLongClickListener { false }
tagsArrayAdapter!!.tagContextAndLongClickListener = OnContextAndLongClickListener { false }
}
adjustToolbar(tagsDialogView)
dialog = AlertDialog.Builder(requireActivity())
Expand Down
11 changes: 6 additions & 5 deletions AnkiDroid/src/main/java/com/ichi2/anki/widgets/DeckAdapter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import android.content.Context
import android.graphics.drawable.Drawable
import android.view.LayoutInflater
import android.view.View
import android.view.View.OnLongClickListener
import android.view.ViewGroup
import android.widget.Filter
import android.widget.Filterable
Expand All @@ -32,6 +31,8 @@ import androidx.annotation.CheckResult
import androidx.annotation.VisibleForTesting
import androidx.recyclerview.widget.RecyclerView
import com.ichi2.anki.CollectionManager.withCol
import com.ichi2.anki.OnContextAndLongClickListener
import com.ichi2.anki.OnContextAndLongClickListener.Companion.setOnContextAndLongClickListener
import com.ichi2.anki.R
import com.ichi2.libanki.DeckId
import com.ichi2.libanki.sched.DeckNode
Expand Down Expand Up @@ -64,7 +65,7 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)
// Listeners
private var deckClickListener: View.OnClickListener? = null
private var deckExpanderClickListener: View.OnClickListener? = null
private var deckLongClickListener: OnLongClickListener? = null
private var deckContextAndLongClickListener: OnContextAndLongClickListener? = null
private var countsClickListener: View.OnClickListener? = null

// Totals accumulated as each deck is processed
Expand Down Expand Up @@ -114,8 +115,8 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)
deckExpanderClickListener = listener
}

fun setDeckLongClickListener(listener: OnLongClickListener?) {
deckLongClickListener = listener
fun setDeckContextAndLongClickListener(listener: OnContextAndLongClickListener?) {
deckContextAndLongClickListener = listener
}

/** Sets whether the control should have partial transparency to allow a background to be seen */
Expand Down Expand Up @@ -219,7 +220,7 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)

// Set click listeners
holder.deckLayout.setOnClickListener(deckClickListener)
holder.deckLayout.setOnLongClickListener(deckLongClickListener)
holder.deckLayout.setOnContextAndLongClickListener(deckContextAndLongClickListener)
holder.countsLayout.setOnClickListener(countsClickListener)
}

Expand Down