agateau / nanonote

A minimalist note taking application
Other
59 stars 11 forks source link

The shortcuts are not visible in context menu on macOS #25

Closed agateau-g closed 1 year ago

agateau-g commented 3 years ago

See screenshot:

Screenshot 2021-01-08 at 14 22 56
dlaidig commented 1 year ago

Seems like I should have been more careful with mentioning the issue number in my quick tests. Sorry for the noise. 😅

I thought this should be an easy one-line merge request after I discovered that there is a Qt::AA_DontShowShortcutsInContextMenus flag. This also has the desired effect with Qt6 (with a few quick-and-dirty changes to get everything to compile). In more recent versions of Qt5, this flag exists but has no effect (tested with 5.15.2). After a while I found out that QGuiApplication::styleHints()->setShowShortcutsInContextMenus(true) works (but requires Qt 5.13).

So, the fix to get this working is quite trivial:

#include <QStyleHints>

// ...

#ifdef Q_OS_MACOS
    app.setAttribute(Qt::AA_DontShowShortcutsInContextMenus, false);
    QGuiApplication::styleHints()->setShowShortcutsInContextMenus(true);
#endif

The tricky thing for me was the Qt version. The Linux build via the GitHub actions uses Qt 5.9, which does not matter with the #ifdef. However, QT_VERSION is set to 5.12.8 in ci/install-dependencies which seems to be used on MacOS and Windows and does not have setShowShortcutsInContextMenus().

When I set the version to 5.15.2, I got a working build for MacOS, but broke the Windows build. I thought upgrading the aqtinstall version might be worth a shot. However, this still failed and just added some deprecation warnings because of changes in aqtinstall.

At this point I decided to exit the rabbit hole and just write down what I found here. If you are interested in changing the Qt version in the proper way, this issue should be easy to fix.

agateau commented 1 year ago

Hey, sorry for the late reply.

Thanks for giving it a try and reporting what you found deep in the rabbit hole :)

It's been quite some time since the minimum Qt version has been defined, I think we can bump it to 5.15. I am going to look at the Windows issue. I don't have access to a macOS machine anymore these days though, so if you can apply your fix once the minimum version has been raised, it would be awesome.

dlaidig commented 1 year ago

Sure, once the Qt version is changed, I'll test again and provide a pull request.

On the off chance that you still remember (and maybe this was even the trigger to open this issue...):

Did the keyboard shortcuts to move lines up/down work for you on MacOS? For me, the default key combination of Option+Shift+Up/Down is not recognized, but if I remove the shift modifier (i.e., change it to Option+Up/Down) it works. In any case, I'll investigate further. Maybe I messed too much with my system and it is not a general issue on MacOS.

agateau commented 1 year ago

Sure, once the Qt version is changed, I'll test again and provide a pull request.

After much wrestling with the CI code and aqtinstall, I managed to update Qt to 5.15.2 for macOS and Windows. I kept it at 5.12.8 on Linux for now so that binaries continue to work on Ubuntu 20.04. This should not be an issue for your fix, since it's going to be in a #ifdef Q_OS_MACOS block.

That CI code I came up with is a convoluted mess, I need to simplify it :)

On the off chance that you still remember (and maybe this was even the trigger to open this issue...):

Did the keyboard shortcuts to move lines up/down work for you on MacOS? For me, the default key combination of Option+Shift+Up/Down is not recognized, but if I remove the shift modifier (i.e., change it to Option+Up/Down) it works. In any case, I'll investigate further. Maybe I messed too much with my system and it is not a general issue on MacOS.

I think they did work, but I could be wrong.

dlaidig commented 1 year ago

Thanks! I'm glad that I stopped fighting with the CI stuff at some point... ;)

I'll test my patch on macOS and then create a pull request.