PR: Add mappings for QMouseEvent methods#408
Conversation
CAM-Gerlach
left a comment
There was a problem hiding this comment.
Seems good so far, thanks, but would it be possible to actually test that the functions can be called correctly and consistently? That seems to be the user's concern on that issue. Thanks!
See spyder-ide#408 (comment) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
|
It looks like the test crashes the process on Ubuntu with For the issue with Ubuntu, the most related thread I found on SO was https://stackoverflow.com/questions/56281631/qapplication-instance-qtbot-fixture-causes-travis-ci-to-abort-and-core-dump. Maybe, it'll help.
Oh, wow, I get the same crash in GitHub tests when I create an instance of I had to skip the test for the config for now. And I can't actually get the coordinates I move the mouse to from the event methods. I can't predict the window border dimensions. Possibly, I missed something. Anyway, the mapping I use should prevent any default values (like |
CAM-Gerlach
left a comment
There was a problem hiding this comment.
Looks generally good to me, thanks, aside from another minor nit; for more substantive expert feedback I would defer to @dalthviz
See spyder-ide#408 (comment) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
`QMainWindow.show()` finishes when the window appears. So, no extra waiting needed.
This reverts commit 966be08.
…qt` crashes" This reverts commit e90bb1a.
|
(Sidenote: @dalthviz , if/when this is merged, it should be presumably done via the Squash strategy (or with rebase cleanup first) to avoid all the repeated fixup commits for a relatively small change. I enabled a setting that will still preserve a list of the commit messages in the commit description, and the full commit history of this branch can still be referred to on the PR in perpetuity, while the history on the main branch is kept clean.) |
dalthviz
left a comment
There was a problem hiding this comment.
Thanks @StSav012 for your work here! Besides that seems like there is a change from a PR #407 also here, this LGTM 👍
Do you know what could have happened there @CAM-Gerlach? Also, feel free to squash merge this when you think is ready (seems like there a unresolved conversation)
See spyder-ide#408 (comment) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Added
for PyQt6 and PySide6.
And I had to add
to PyQt5 and PySide2 to make the event test universal and (consequently, but this was not the required enhancement) bidirectional. This way, I check that the coordinates returned by the back-ported functions are equal to what the Qt6 functions return (except for the type, but I use integer coordinates for that to work).