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 Cmd-Shift-M to bring up 3dConnexion Spacemouse Dialog (EXCEPT on "Home" sub-screen) #5598

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

SuperTango
Copy link
Contributor

Description

this is for issues:

The problem is that hitting "Cmd-Shift-M" on mac always minimizes the app, even though it should only minimize on "Cmd-M", and not on "Cmd-Shift-M".

The code that minimizes (using the WXWidgets "Iconize()" call) happens in MainFrame.cpp keyboard event loop. The code that's checking, looks for "Cmd-M" but does not check for any other keyboard modifiers, so I added a check to ignore the event if Shift is pressed along with "Cmd-M".

There's a secondary issue that isn't really relevant to this bug in that the app will still minimize when pressing "Cmd-Shift-M", but ONLY on the "Home" sub-screen. (all other sub-screens work as they should).

I'm not sure why, but when the "Home" sub-screen is selected, the keyboard event loop (MainFrame.cpp, line 609), is called TWICE when "Cmd-Shift-" is pressed:

  • Once where the event's wxKeyModifier (retrieved via evt.GetModifiers() is set to wxMOD_CONTROL AND wxMOD_SHIFT. (this is correct)
  • Once where the event's wxKeyModifier is ONLY set to wxMOD_CONTROL (this is wrong).

I hunted around the code for a long while trying to figure this out, but I don't know much about wxWidgets (and the docs/googling didn't reveal anything), so I wasn't able to fix this root cause.

Again, this double-event (with the wrong modifiers) only happens when the user is on the "Home" sub-screen. For the context of this bug the 3DConnexion preferences dialog isn't needed on the "Home" sub-screen so this secondary bug doesn't really matter. But it does make the UX odd where Cmd-Shift-M will minimize the app when the user is viewing the "Home" sub-screen, but not minimize the app when the user is viewing any other sub-screen.

Screenshots/Recordings/Graphs

Tests

manual testing was done.

  • On Mac, verified that the "Cmd-Shift-M" brings up the 3dConnexion dialog box and does not minimize the app (on all sub-screens except for the "Home" sub-screen as described above in the "secondary issue" above).
  • On Mac, verified that bringing up the "Keyboard Shortcuts" dialog shows "Cmd-Shift-M" instead of "Cmd-M" on a mac. I do not have a windows or linux machine to test on, but it's a one-line #ifdef change, so it should be good.

The problem is that hitting "Cmd-Shift-M" on mac always minimizes the app, even though it should only minimize on "Cmd-M", and not on "Cmd-Shift-M".

The code that minimizes (using the WXWidgets "Iconize()" call) happens in MainFrame.cpp keyboard event loop.  The code that's checking, looks for "Cmd-M" but does not check for any other keyboard modifiers, so I added a check to ignore the event if Shift is pressed along with "Cmd-M".

There's a secondary issue that isn't really relevant to this bug in that the app will still minimize when pressing "Cmd-Shift-M", but ONLY on the "Home" sub-screen. (all other sub-screens work as they should).

I'm not sure why, but when the "Home" sub-screen is selected, the keyboard event loop (MainFrame.cpp, line 609), is called TWICE when "Cmd-Shift-<any key>" is pressed:

* Once where the event's wxKeyModifier (retrieved via `evt.GetModifiers()` is set to `wxMOD_CONTROL`  AND `wxMOD_SHIFT`.  (this is correct)
* Once where the event's wxKeyModifier  is **ONLY** set to `wxMOD_CONTROL` (this is wrong).

Again, this double-event (with the wrong modifiers) only happens when the user is on the "Home" sub-screen.  For the context of this bug the 3DConnexion preferences dialog isn't needed on the "Home" sub-screen so this secondary bug doesn't matter.  But it does make the UX odd where Cmd-Shift-M will minimize the app when the user is viewing the "Home" sub-screen, but not minimize the app when the user is viewing any other sub-screen.
@foreachthing
Copy link

Why not bring this dialog into this image menu? Or, allow the user to define the shortcuts.

@SuperTango
Copy link
Contributor Author

Why not bring this dialog into this image menu? Or, allow the user to define the shortcuts.

I was merely providing a fix to the bug that prevented the 3dConnexion dialog from appearing without making any structural changes to the codebase. TBH, i'm not sure what menu you're referencing (I don't see that icon on mac), and as for letting users choose their own shortcuts, I think that's a great feature request, but that's a pretty hefty change to the codebase, which is out of scope of this bugfix.

@foreachthing
Copy link

i'm not sure what menu you're referencing

My mistake! I'm on Windows and assumed it looked the same on all OSs.

In OrcaSlicer 2.0:
image

In OrcaSlicer 2.1:
image

Copy link
Owner

@SoftFever SoftFever left a comment

Choose a reason for hiding this comment

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

Looks good.
Thank you for digging into the code and fix it!

@SoftFever SoftFever merged commit 06ef58a into SoftFever:main Jun 7, 2024
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.

3 participants