Fixes #17701 - Integrated Terminal Context Menu is triggered via contextmenu event instead of mousedown#18980
Conversation
|
Hi @chirag64, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
|
Thanks @chirag64, I'll look at this in a week or so since we're in the process of finishing up/verifying the next release. |
There was a problem hiding this comment.
Should the right click copy/paste stuff still be in the mousedown handler and cancel the event if it goes off? I think cmd copies/pastes on mousedown rather than mouseup.
There was a problem hiding this comment.
I can confirm that cmd in Windows copy-pastes on 'mousedown' rather than 'mouseup'. Do you want me to keep the code inside the if condition within 'mousedown' and just move the else condition code in 'contextmenu' event?
There was a problem hiding this comment.
Yeah I think that's ideal 👍 and make sure the event is canceled such that a rightClickCopyPaste mousedown event does not trigger the contextmenu event.
There was a problem hiding this comment.
I was thinking I can use the if condition of getRightClickCopyPaste in the mousedown event and revert the if condition of getRightClickCopyPaste in the contextmenu event. Not sure how I can cancel the triggering of the event any other way, maybe I'm missing something obvious here.
There was a problem hiding this comment.
Try returning false inside the mousedown handler when the rightClickCopyPaste is done, I would expect that to prevent the contextmenu event.
There was a problem hiding this comment.
Do event.preventDefault or event.stopPropagation() work?
There was a problem hiding this comment.
Tried both, but didn't work. I was sure one of these would work.
There was a problem hiding this comment.
One last try: stopImmediatePropagation() 😛
There was a problem hiding this comment.
Tried all 4 together, no luck. ☹
There was a problem hiding this comment.
Well we don't want to regress with right click copy paste so we'll probably want to keep a boolean member variable that we'll use to "cancel" the contextmenu event.
… via contextmenu event instead of mousedown
|
@Tyriar Can you please review my recent commit 184e728? I've added a boolean variable that we can use to 'cancel' the contextmenu event. |
|
Perfect! Thanks for the contribution 😃 |
3 participants
Before this change, the Context Menu used to show up on mousedown event in the integrated terminal, but on mouseup (contextmenu event) inside the editor. This change is clearly visible on Windows rather than OS X or Linux when you hold the right-click button.
Now the behavior of the context menu of integrated terminal should be consistent with that of the editor