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

fix: simultaneous opening of Pitch Pie Menu and Grey Menu on Right Click #4007

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haroon10725
Copy link
Contributor

I found that this.container has both an on-click and a press-up event listener:
The on-click listener checks if the Ctrl key is pressed. If it is, it triggers the piemenuBlockContext function, which displays a grey menu. After this, the function returns, preventing any further code from executing.
The press-up listener calls the _mouseoutCallback function, but only if that.blocks.getLongPressStatus is false. This ultimately triggers the _changeLabels function without checking whether the Ctrl key is pressed or not.

@walterbender Can you please review it ?

@walterbender
Copy link
Member

Can you please link to the issue you are fixing? Does it describe how to reproduce the issue?
I don't recall the ctrl key being mentioned.

@haroon10725
Copy link
Contributor Author

#3986

@walterbender
Copy link
Member

Thanks. I don't have a Mac, so I cannot test it. I presume you did?

@haroon10725
Copy link
Contributor Author

@walterbender Yes, I did. I think you can also reproduce the same error by doing ctrl + click on a block.

@walterbender
Copy link
Member

walterbender commented Sep 17, 2024

If you control-click on the background, you don't get the large menu. Is that OK? Long press works on a block, but not on the background. Is that OK?

I think a better solution would be to determine where the click is occurring.

@haroon10725
Copy link
Contributor Author

No, I get large menu.

@walterbender
Copy link
Member

Hmm. The only way I can get the large menu on my machine (Fedora) is a right-click.

@haroon10725
Copy link
Contributor Author

haroon10725 commented Sep 17, 2024

@walterbender Were you able to reproduce it ( ctrl + click on a block and both menu's appear ) ?

@walterbender
Copy link
Member

I cannot reproduce the bug as stated in the ticket using ctrl, but I can do this:
Screenshot from 2024-09-17 11-27-49

I'll try this with your branch. But I suspect that we need to think this through more.

@haroon10725
Copy link
Contributor Author

haroon10725 commented Sep 17, 2024

I am pressing ctrl + click ( using touchpad ). Maybe try this. I have tested on Windows and Mac, works fine.

@haroon10725
Copy link
Contributor Author

I cannot reproduce the bug as stated in the ticket using ctrl, but I can do this: Screenshot from 2024-09-17 11-27-49

I'll try this with your branch. But I suspect that we need to think this through more.

How were you able to do it ?

@walterbender
Copy link
Member

There are 4 different mechanisms at play across 5 conditions:

Mechanisms:
click
right click
ctrl click
long press

Conditions:
background
normal block
collapsible block (on collapse button)
block that opens a pie menu
number or text block

We want the following priority:

  1. pie menu for block value
  2. input form for block value
  3. collapse/expand block
  4. block context menu (small gray pie menu)
  5. large gray menu

1 and 2 can co-exist.
Otherwise, the higher priority should be used.

In the current implementation, sometimes multiple conditions are triggered.

@haroon10725
Copy link
Contributor Author

I couldn't fully understand what you say.

@walterbender
Copy link
Member

Any progress on this?

@haroon10725
Copy link
Contributor Author

@walterbender I am identifying in the code where each of the events are being triggered.

@haroon10725 haroon10725 marked this pull request as draft October 14, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants