[Bug]: contextmenu in V4 is not default-preventable about joint HOT 11 OPEN

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.

from joint.

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,

from joint.

It has nothing to do with jQuery removal. This was introduced in v3.6.0 in #1727.

Can you use paper.options.preventContextMenu = true?

from joint.

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!

from joint.

Is it happening in this simple example? Note that preventContextMenu is true by default.

https://jsfiddle.net/kumilingus/pv087q1w/

from joint.

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!

from joint.

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:

• adding a delay for the popup to show with a setTimeout.
• making the popup window itself right-click-proof.

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!

from joint.

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.

from joint.

from joint.

It was to unify the behaviour between Windows and macOS: #1590

Windows - contextmenu fired on mouseup
Mac - contextmenu fired on mousedown

Also, it seems that people have experienced the same issues in past: #1891

from joint.

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.

That's a good point. Normalizing the contextmenu event seems to be a bad decision.

An ideal solution could be to:

• use native contextmenu event for cell:contextmenu and blank:contextmenu events.
• trigger cell:pointerdown, cell:pointermove and cell:pointerup no matter what button was triggered (also valid for blank:* events). We currently do not trigger the events.

This would allow users:

• to always have access to the native contextmenu event
• be able to drag & drop paper with right button

  paper.on('blank:pointerdown', (evt) => { evt.button === 2 && scroller.startPanning(evt); });

This would greatly simplify the code.
The users could use guard paper callback to prevent cell:pointer** events from triggering when the right button is clicked.

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 contextmenu behavior:

new dia.Paper({ normalizeContextmenuEvent: false });

from joint.