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

Noop on clicking both mouse buttons #2244

Merged
merged 1 commit into from Oct 26, 2021
Merged

Conversation

brisvag
Copy link
Collaborator

@brisvag brisvag commented Oct 25, 2021

Small change to 3D cameras mouse event callback to prevent firing a million warnings when accidentally pressing both right and left mouse buttons: napari/napari#3518.

I'm not sure if we prefer a noop (as I did in this PR) or just ignoring the second click, though the second option requires a bit more refactoring I think.

@djhoese
Copy link
Member

djhoese commented Oct 25, 2021

Where does the warning come from?

@brisvag
Copy link
Collaborator Author

brisvag commented Oct 25, 2021

Something about how self._event_value is used and updated gets thrown off by multiple clicks, so that self._event_value[0] is None when running _update_rotation, rather than a float.

  File "/home/brisvag/git/vispy/vispy/scene/cameras/perspective.py", line 225, in viewbox_mouse_event
    self._update_rotation(event)
  File "/home/brisvag/git/vispy/vispy/scene/cameras/turntable.py", line 136, in _update_rotation
    self.elevation = self._event_value[1] + (p2 - p1)[1] * 0.5
TypeError: unsupported operand type(s) for +: 'NoneType' and 'float'

@djhoese
Copy link
Member

djhoese commented Oct 25, 2021

Do we have any real tests for the cameras where this could be added?

@brisvag
Copy link
Collaborator Author

brisvag commented Oct 26, 2021

Not sure how to add fake clicks or at least mouse events to tests.

@djhoese djhoese merged commit 6897495 into vispy:main Oct 26, 2021
@djhoese
Copy link
Member

djhoese commented Oct 26, 2021

Eh don't worry about it. Thanks.

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.

None yet

3 participants