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

Click and drag firing when modal context menu is open #272

Open
tznind opened this issue Dec 29, 2023 · 2 comments
Open

Click and drag firing when modal context menu is open #272

tznind opened this issue Dec 29, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@tznind
Copy link
Collaborator

tznind commented Dec 29, 2023

There is code that is supposed to lock operations and prevent move events firing. This may be a regression or a new bug or something that came from upstream. Needs investigation

Maybe we just need to expand the return conditions to also include context menus e.g.

// if another window is showing don't respond to mouse
if (!this.IsCurrentTop /* TODO: or context menu showing*/)
{
    return;
}

mouse-down-in-context-menu-bad

@tznind tznind added the bug Something isn't working label Dec 29, 2023
@dodexahedron
Copy link
Contributor

The MouseManager has some problems that I've notated in the updated MouseManagerTests.cs file.

They're basic issues with how conditions are identified, so might be a wise place to begin before directly addressing this. Building on a foundation of stone rather than sand and all that.

This branch has the refactored MouseManager tests, with some specific additions to one of the tests to aid in debugging the root cause of the problem.

Some of those tests should be failing, and would be if I had left the "correct" assertions in (I committed at least one commented-out example of what I think should be tested in at least one of the cases.. I think the comments I left in the MouseManagerTests.cs file should be pretty self-explanatory, but definitely let me know if you have any questions/concerns/thoughts/letters to the editor about any of that.

@dodexahedron
Copy link
Contributor

Specifically about locking:

I know there's at least one instance of booleans being used for locking, which isn't safe and has lots of scary all-caps and red-x bearing notes in the dotnet docs.

One of them that I am already aware of and which is one of the parts of what requires the unit tests to run mostly serially and which is directly relevant to this issue posting is SelectionManager.LockSelection, which isn't thread-safe for parallelism or reentrancy.

It's another class of issues, though, that I think are best served by waiting til the test project is finished being updated and enhanced to provide more complete and more accurate information for us to work off of.

I should be able to start looking at some of that kind of thing pretty soon. I've been spending less time improving the tests, for the past couple of test fixtures, opting to focus more on the ultra-basics of converting them to the constraint model and that sort of thing. I intend to keep taking that approach (aside from when I get ADHDistracted by something 😅), so I should probably be able to at least convert the rest of the remaining test fixtures pretty soon. Maybe within the next week or two, even. 🤞

I went ahead and started a discussion here for thread safety, so we can chat about that when we get around to it or whenever inspiration strikes. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants