-
Notifications
You must be signed in to change notification settings - Fork 204
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
Right-click tap handler for replies #696
Conversation
The hoveredLink does not work, because the MessageDelegate::enabled is set to false, so any input event isn't forwarded to it including the mouse hover. (That's also why clicking links, text selection and the normal context menu doesn't work). |
Thank you! I set it to true. I'm still noticing the context menu appearing at a really weird position but it's also happening for normal messages (#698), so I suppose it's not related to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welp, I never clicked the submit button on the review...
resources/qml/delegates/Reply.qml
Outdated
@@ -41,6 +42,40 @@ Item { | |||
gesturePolicy: TapHandler.ReleaseWithinBounds | |||
} | |||
|
|||
Platform.Menu { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this somewhere global, like into the MessageView where the other menu is? Menus have been a performance issue in the past, although I don't know, if the Platform.Menu would still trigger that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how to do that (no idea about how QT/QML works). Feel free to push code wherever you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just put that menu into a different file (the MessageView.qml, where the other Menu should be). That is all. It should just work, since that is a parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to MessageView.qml
resources/qml/delegates/Reply.qml
Outdated
@@ -99,11 +134,10 @@ Item { | |||
callType: r.callType | |||
relatedEventCacheBuster: r.relatedEventCacheBuster | |||
encryptionError: r.encryptionError | |||
enabled: false | |||
enabled: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was done, so that tapping the reply would scroll to the correct position instead of selecting some text. Does that not happen anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not working as you described. I reverted it back to enabled: false
, added a comment and moved the TapHandler
to column. I think now it achieves the interaction you mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait, setting enabled: false
breaks links within the message :-|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a way of making it work even with enabled: false
by using linkAt
in the text message and subtracting the username's height from the point's Y (since TapHandler
is installed in the column which has both the username and the text).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a horrible hack, but might be the best option for now :D
resources/qml/delegates/Reply.qml
Outdated
} | ||
|
||
TapHandler { | ||
acceptedButtons: Qt.RightButton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also make this trigger on long press?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Seems to work correctly and the Got to Reply is a nice addition to the menu! Thanks! |
This is an attempt at fixing #695, but it doesn't actually work fully. I was unable to make more progress than this because I don't know anything about QML.
The intention for this PR is to support only "copy" and "copy link" actions, but there are problems with this code:
reply.child.hoveredLink
does not actually work, apparently;reply.child.copyText
doesFeel free to disregard any part of the code which looks nonsensical, as aforementioned I don't know anything about QML. Also feel free to take over this PR and push the remaining code to this branch.