processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.66k stars 3.32k forks source link

Add an easier way to disable right-click context menus on the canvas #7098

Open bojidar-bg opened 4 months ago

bojidar-bg commented 4 months ago

Increasing access

Would allow developers of low technical skill to freely use the right mouse button in their sketches without having to copy/paste cryptic lines from StackOverflow.

Most appropriate sub-area of p5.js?

Feature request details

When making a sketch that uses mouseButton === RIGHT, students/users are currently surprised that this results in a context menu popping up.
Unfortunately, the simplest way to disable that context menu right now is to add code akin to (c.ref. StackOverflow):

function setup() {
  // createCanvas() ...
  document.querySelector('canvas').addEventListener('contextmenu', e => e.preventDefault())
}

However, that code is a significant step up from a simple if (mouseButton === RIGHT).

Having a simpler API to disable the context menu would be much appreciated. For example, something like:

createCanvas(400, 400, {contextmenu: false})
createCanvas(400, 400, WEBGL, {contextmenu: false})
// or:
createCanvas(400, 400).disableContextMenu()
// or:
createCanvas(400, 400)
disableContextMenu()
// or:
function setup() { ... }
function contextMenu(e) {
  e.preventDefault() // still "ugly", but at least reasonably okay
}
// or:
function setup() { ... }
function contextMenu() {
  return true // somewhat magic
}
welcome[bot] commented 4 months ago

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

davepagurek commented 4 months ago

This could make sense on all p5.Element objects (including the return value of createCanvas). Maybe to be consistent with some of the other methods we have, it could be called .contextMenu(), where passing a boolean to it sets whether or not it should be enabled, and passing no arguments returns whether or not it is currently enabled?

RanitMukherjee commented 4 months ago

That sounds great, I can get it done, Should I proceed & submit a PR against the same?

limzykenneth commented 4 months ago

Going with the API suggested by @davepagurek make sense to me with attaching to p5.Element in general and having .contextMenu() as the method. One thing to keep an eye out for would be that technically iOS does not have the contextmenu event supported (which it probably didn't need anyway) but the implementation need to be careful in case trying to use it results in a blocking error.

@bojidar-bg Are you thinking of trying to implement this or if not I can assign @RanitMukherjee to work on this?

bojidar-bg commented 4 months ago

@limzykenneth Thinking, maybe; I was hoping to see discussion slowly organize itself into consensus before throwing a weekend at it, but given the influx of volunteers, I'll say, assign away! :tada: :sparkles:

limzykenneth commented 4 months ago

@bojidar-bg I'm fine with leaving a few days for us to think about the implementation since this will be adding a new API which we want to avoid getting it wrong. :smiley:

bojidar-bg commented 4 months ago

@limzykenneth I'm fine either way, but do note I cannot guarantee having a weekend to throw at the issue, as it is :smiling_face_with_tear:

Anyway, re: the proposed .contextMenu API: Note that contextmenu is an event, so it would make some sense to have it accept a handler function instead of being a property—that is, I believe it should be like .mousePressed() and not like .style()

Some ideas:

let canvas = createCanvas(400, 400)
canvas.contextMenu(function customHandler(e) { ... }) // go wild, do whatever
// 1:
canvas.contextMenu(false) // default/current behavior -- consistent with .mousePressed
canvas.contextMenu(true) // disable menu -- magic, counter-intuitive
// 2:
canvas.contextMenu(true) // default/current behavior -- inconsistent with .mousePressed
canvas.contextMenu(false) // disable menu -- very intuitive ("and here we say, no context menu")
// 3:
canvas.contextMenu(false) // default/current behavior -- consistent with .mousePressed
canvas.contextMenu(DISABLE) // disable menu -- intuitive, can later say "DISABLE is like `e => e.preventDefault()`"
davepagurek commented 4 months ago

Good point about consistency with mousePressed. While in isolation option 2 was what I was originally thinking, it does introduce some inconsistency. your third option seems like a good compromise to me!

RanitMukherjee commented 2 months ago

In my opinion, behavioral consistency is necessary, thus I think idea 1 or 3 are good enough & we should consider to proceed with either of those (apologies for not being around for a while, had gotten kinda busy irl since my sems just started)

davepagurek commented 2 months ago

I'd support option 3 then I think. @limzykenneth does that sound OK to you to move forward with?

limzykenneth commented 2 months ago

Option 3 sounds good for now to have consistency with other event handling functions like mousePressed() so let's go for it. We can revisit and unify event handling with 2.0 as necessary too.

RanitMukherjee commented 2 months ago

Alright then, I am working toward implementing idea/option 3 & will submit a PR for the same as early as possible.