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

Allow sending "mouse move" with left/right down with EnableTestAssistant #715

Merged
merged 8 commits into from
Mar 15, 2021

Conversation

aaronayres35
Copy link
Contributor

fixes #460

This PR adds left_down and right_down kwargs to mouse_move so that the test assistant can simulate moving the mouse with either the left or right button down. As mentioned on the issue, there are cases where we expect mouse_move events to have {left/right}_down set to True. This PR also adds a simple test to check that the traits are set if we call mousee_move with the new kwargs. Additionally, the press_move_release method is updated so that left_down is True for the mouse_move generated event itself.

@aaronayres35
Copy link
Contributor Author

It seems in some cases mouse move events are not being handled. Need to investigate further.

@aaronayres35 aaronayres35 changed the title Allow sending "mouse move" with left/right down with EnableTestAssistant [WIP] Allow sending "mouse move" with left/right down with EnableTestAssistant Mar 12, 2021
@jwiggins
Copy link
Member

You just need to add left_down=True to the mouse_move calls in the failing tests.

That said, it would be nice to know why this broke those tests to begin with.

@jwiggins
Copy link
Member

We should maybe add support for middle_down while we're here, no?

@aaronayres35
Copy link
Contributor Author

aaronayres35 commented Mar 15, 2021

Previously, if {left/right}_down was not set, they defaulted to Undefined. Thus, in this code:

button_down = getattr(event, self.drag_button + "_down")
if state == "nondrag":
if (button_down

button_down would evaluate to True, event if {left/right}_down was never set. Now, this PR explicitly sets them to False, hence the new test failures.

Unfortunately that suggests this PR may break existing code out in the wild that unknowingly depended on this buggy behavior

Perhaps a less disruptive fix would be to have the left_down/ right_down kwargs default to Undefined like they used to, instead of explicitly making them False? The dependence on Undefined being True surely seems like a bug, but we may not want to break code. @jwiggins what do you think?

@jwiggins
Copy link
Member

@jwiggins what do you think?

Hard question. I don't like the bug exposed here but we might want to keep the default values as Undefined so that we don't break downstream CI for dependent projects.

@aaronayres35
Copy link
Contributor Author

@jwiggins what do you think?

Hard question. I don't like the bug exposed here but we might want to keep the default values as Undefined so that we don't break downstream CI for dependent projects.

That makes sense, I will make that change and then open a separate issue for the use of Undefined

@aaronayres35 aaronayres35 changed the title [WIP] Allow sending "mouse move" with left/right down with EnableTestAssistant Allow sending "mouse move" with left/right down with EnableTestAssistant Mar 15, 2021
Copy link
Member

@jwiggins jwiggins left a comment

Choose a reason for hiding this comment

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

LGTM

@aaronayres35 aaronayres35 merged commit 8b7f812 into master Mar 15, 2021
@jwiggins jwiggins deleted the leftright-down-mouse-move branch March 15, 2021 17:51
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.

Cannot send "mouse move" with left/right down with EnableTestAssistant
2 participants