Skip to content
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

Changed mouse button to object #7404

Merged
merged 4 commits into from
Dec 17, 2024
Merged

Conversation

diyaayay
Copy link
Contributor

@diyaayay diyaayay commented Dec 3, 2024

Resolves #6847

Changes:

  • Changed mouseButton from a string to object.
  • mouseButton now intends to track the current state of mouse buttons, showing which buttons are pressed at any given moment.

This work is done over the PR #7378

PR Checklist

@diyaayay
Copy link
Contributor Author

diyaayay commented Dec 3, 2024

It looks like if you have two fingers down, and then raise one, this would cause mouseButton.left to become false because the pointer that was lifted would now have no left button. It might be prefereable to, when this happens, keep mouseButton.left true if any remaining touch still has a left button down.

So maybe that would mean adding a buttons property on each active touch, and when an event happens, we loop through activeTouches and set left = activeTouches.some(touch => touch.buttons.left)? (I think this would also mean using activeTouches for all pointer types, not just touch, to still handle the single-pointer desktop case correctly.

Does that behaviour sound like what we want?

I think it makes more sense for mouseButton.left to remain true as long as any touch action is present on the screen. Setting it to false in onpointerup, using something like this, might work well:

if (this._activeTouches.size === 0) {
  Object.keys(this.mouseButton).forEach((key) => (this.mouseButton[key] = false));
}

This approach means we treat all active touches on the screen as left touches, correct? In that case, right and center clicks would only be supported by a mouse if I'm not wrong. @davepagurek

@davepagurek
Copy link
Contributor

This approach means we treat all active touches on the screen as left touches, correct?

I think this is fine if it ends up being required. Although it looks like on MDN, PointerEvent extends MouseEvent, so would that mean that when we receive a pointer event, we can check e.buttons? If so, it feels like maybe we don't need to do a special case for touches at all.

If we keep track of the buttons for each active touch in a similar way to how you currently do the global mouseButton, then after any pointer event, we could update just the active pointer from the current event, and then update the global mouseButton to match:

const pointer = this.activePointers[e.pointerId]
pointer.mouseButton.left = (e.buttons & 1) !== 0;   
pointer.mouseButton.center = (e.buttons & 4) !== 0; 
pointer.mouseButton.right = (e.buttons & 2) !== 0;

for (const key in this.mouseButton) {
  this.mouseButton[key] = Object.values(this.activePointers).some((p) => p.mouseButton[key])
}

@diyaayay
Copy link
Contributor Author

diyaayay commented Dec 6, 2024

@davepagurek This now uses _activePointers to keep track of mouseButton by using the buttons property of each active pointer on the screen i.e. in the Mouse-Button sketch here, the mouseButton.left stays true for as long as there is a finger/touch on the screen. I've tested all the other sketches from the Event-handling PR to ensure that nothing breaks there from these changes.

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, just one minor comment about the reference!

src/events/pointer.js Outdated Show resolved Hide resolved
@limzykenneth
Copy link
Member

@diyaayay @davepagurek Can we merge this now?

@davepagurek
Copy link
Contributor

I think we're good to go!

@davepagurek davepagurek merged commit 2ada04a into processing:dev-2.0 Dec 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants