[Bug]: contextmenu in V4 is not default-preventable #2658
Comments
|
It has nothing to do with jQuery removal. This was introduced in Can you use |
|
Ah, ok. I remember that jquery had some sophisticated event mappings at some point, so i guessed (wrongly) it might be related to that. We moved from V3.4 straight to v4, so missed this. I tried your suggestion but it didn't work. Am i doing something wrong? this._graph = new joint.dia.Graph();
this._paper = new joint.dia.Paper({
el: this._canvasElement.childNodes[0]
,width: "5000px"
,height: "5000px"
,model: this._graph
,gridSize: 10
});
this._paper.options.preventContextMenu = true;
this._paper.on('element:contextmenu', function(cellView, evt, x, y) {
evt.stopPropagation();
evt.preventDefault();
var str = '<LI>'+(cellView.model.get('showKeys' ) != true ? 'Show Keys' : 'Hide Keys')+'</LI>'
+'<LI>'+(cellView.model.get('showColumns') != true ? 'Show Columns' : 'Hide Columns')+'</LI>'
+'<LI>Remove from Canvas</LI>'
;
showContextMenu(str, evt.pageX, evt.pageY);
});I also tried for good measure this._paper = new joint.dia.Paper({
el: this._canvasElement.childNodes[0]
,width: "5000px"
,height: "5000px"
,model: this._graph
,gridSize: 10
,options: { preventContextMenu: true }
});and this._paper = new joint.dia.Paper({
el: this._canvasElement.childNodes[0]
,width: "5000px"
,height: "5000px"
,model: this._graph
,gridSize: 10
,preventContextMenu: true
});The default browser context menu still shows. I am testing on Windows with Chrome, Firefox and Edge.. I did not try this on Mac or other environments. Thank you! |
|
Is it happening in this simple example? Note that |
|
Hello Roman. This example is working as expected... I have to check this in more details as i am not seeing a difference with our code. This is very strange. Will investigate further and report back. Thank you! |
|
Hello Roman. I was able to reproduce my issue, but i am dumbfounded as to why that is happening. I have made simple updates to your fiddle here: https://jsfiddle.net/Lqjhy1v8/6/ What i found is that if you uncomment the two lines // e.style.left=evt.pageX+"px";
// e.style.top=evt.pageY+"px";then you can reproduce my issue. With those 2 lines commented out, all is good. Looking at the browser, it looks like the new div is shown and positioned under the cursor, and the mouseup event then fires on that div, short-circuiting JointJS's internal mechanics for the events? I am not sure if you'd classify this as a jointJS issue or not (i think it is), but thinking of work-arounds for me:
I'll try those later. Is there a best practice in jointJS for context menus? I can't be the only person doing context menus with this issue? Thank you! |
|
We show a context menu e.g. in our Kitchen Sink application. The difference is that we append the context menu as a paper's child (so it scrolls with the paper). That's why the paper prevents the default for the subsequent event I assume. I would like to first understand why it works in Another workaround is to position the context menu e.style.left = `${evt.pageX+1}px`;
e.style.top = `${evt.pageY+1}px`;Here's the same JSFiddle using JointJS |
|
Ok, I will check those examples :) thank you so much.
As to why the behavior changed, my analysis is so ple I think. Previously, the context menu event in JointJS was activated at the end of the sequence, i.e., mouseup. That's the event the browser responds to for its context menu. So preventing default on the mouseup did its job. Since v3.6, the context menu sequence mousedown+mouseup was broken down and contextmenu is activated for mousedown, leaving mouseup unhandled and inaccessible from the handler: the prevent default won't block mouseup which is coming next.
I wonder why the native event contextmenu wasn't handled directly instead of the mousedown+mouseup combo? Is it because of the complexities involved regarding Firefox and potentially being activated by shortcuts vs mouse behavior?
https://developer.mozilla.org/en-US/docs/Web/API/Element/contextmenu_event
Thank you,
Laurent Hasson
…________________________________
From: Roman Bruckner ***@***.***>
Sent: Wednesday, May 15, 2024 04:31
To: clientIO/joint
Cc: ***@***.***; Author
Subject: Re: [clientIO/joint] [Bug]: contextmenu in V4 is not default-preventable (Issue #2658)
We show a context menu e.g. in our Kitchen Sink application. The difference is that we append the context menu as a paper's child (so it scrolls with the paper). That's why the paper prevents the default for the subsequent event I assume.
I would like to first understand why it works in v.3.5.5 (and not in v3.6.0).
Another workaround is to position the context menu 1px to the left and 1px to the bottom from the pointer:
e.style.left = `${evt.pageX+1}px`;
e.style.top = `${evt.pageY+1}px`;
Here's the same JSFiddle using JointJS v3.5 https://jsfiddle.net/kumilingus/rku5wsme/ (preventing contextmenu works fine)
It can be reproduced on MacOS too.
—
Reply to this email directly, view it on GitHub<#2658 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AADEHB4UZJAQSLPRMGPL4VDZCMMNVAVCNFSM6AAAAABHQWQSLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJRHA4TGOBXGA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
OK Roman, maybe i can help. Can you direct me to the piece of code where you handle these events so i can have a look? Many developers use the mouse events and check if the right button has been clicked. There is a good discussion here: https://stackoverflow.com/questions/76047602/determine-if-a-mousedown-will-trigger-contextmenu-in-js. The most important part is that there is a native browser event "contextmenu" because technically, the context menu may be invoked via key shortcuts (no mouse events, and possibly no target) for example. https://developer.mozilla.org/en-US/docs/Web/API/Element/contextmenu_event I don't know how your code works, but i can have a look and maybe i can offer an option or two? Thank you. |
That's a good point. Normalizing the An ideal solution could be to:
This would allow users:
This would greatly simplify the code. It’s a breaking change though. So we would need to bump the version. A non-breaking change would be to add an option to switch between the current and the new new dia.Paper({ normalizeContextmenuEvent: false }); |
|
I agree 100%. This is more in line with the browser architecture and less surprising to a web developer. As for how to implement, it's a tough one. One solution breaks, while the other one adds complexity and messier code (as you have to keep both options available). As the library owner, i think this is for you to decide :) My preference is to break in general to keep the long-term cleanliness of the code in check. Also, 4.0 is brand new so it feels like a breaking change now will have a lesser impact to the community for a 4.1, which i expect would see the growth of adoption really picking up? Thank you, |
Current versus expected behaviour
In v3.4, we had a context menu handler on an element:
When inspecting the event, it was the proper "contextmenu" event, and so prevending default and stopping propagation would work in preventing the default context menu from the browser.
In V4 (we are upgrading!!!) this behavior no longer works. The issue i believe is that the event is not the "mousedown" event which triggers the contextmenu logic in joinjs. However, the default browser contextmenu responds to the "mouseup" event. As such, the stopPropagation and preventDefault calls have no effect because we are on the "wrong" event?
I couldn't not find documentation about this change of behavior and wonder if this is an error due to the removal of jQuery which was handling such things?
Steps to reproduce
Adding a contextmenu on an element cannot prevent default browser behavior
Version
4.0.2
What browsers are you seeing the problem on?
Firefox, Chrome, Microsoft Edge
What operating system are you seeing the problem on?
Windows
The text was updated successfully, but these errors were encountered: