Skip to content

fix/prevent_button_spam_on_scroll_to_replied_message - and VisibleMessageViews in general #983

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

Merged

Conversation

AL-Session
Copy link
Contributor

Prevents button spam via a generic per-VisibleMessageView spam prevention mechanism for onPress(event: MotionEvent).

Also added a spam-filtering onClickListener (currently unused but potentially useful in the future) based on code provided by @ThomasSession .

The reason for the generic-VisibleMessageView spam prevention rather than an onClickListener is that ConversationActivityV2.handlePress warns us against directly adding onClickListeners to views as it will interfere with gestures such as drag-to-reply:

private fun handlePress(message: MessageRecord, position: Int, view: VisibleMessageView, event: MotionEvent) {
    val actionMode = this.actionMode
    if (actionMode != null) {
        onDeselect(message, position, actionMode)
    } else {
        // NOTE: We have to use onContentClick (rather than a click listener directly on
        // the view) so as to not interfere with all the other gestures. Do not add
        // onClickListeners directly to message content views!
        view.onContentClick(event)
    }
}

@AL-Session AL-Session changed the title WIP fix/prevent_button_spam_on_scroll_to_replied_message - and VisibleMessageViews in general Feb 26, 2025
@AL-Session AL-Session marked this pull request as ready for review February 26, 2025 05:20
// Convenience accessor to avoid having to `!isSenderSelf` all the time.
// e.g, `message.senderIsNotUs` is simpler than mentally parsing `!message.isSenderSelf`.
val senderIsNotUs: Boolean
get() = !isSenderSelf
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed as it isn't apparently used anywhere.
I don't think we should change all the other files to swap the flag.

@@ -120,3 +120,13 @@ fun EditText.addTextChangedListener(listener: (String) -> Unit) {
}
})
}

// Listener class that only accepts clicks at given interval to prevent button spam - can be used instead
Copy link
Collaborator

Choose a reason for hiding this comment

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

So what do we do with these two new addition (setSafeClickListener and SafeClickListener), since you didn't end up using them in this branch?
Are we keeping it just in case for other QA tickets?

@ThomasSession ThomasSession merged commit 635cee1 into dev Mar 17, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants