OpenBoard-org / OpenBoard

OpenBoard is a cross-platform interactive whiteboard application intended for use in a classroom setting.
https://openboard.ch/
GNU General Public License v3.0
2.4k stars 429 forks source link

refactor: avoid QKeyCombination warnings on Qt6 #1079

Open letsfindaway opened 2 months ago

letsfindaway commented 2 months ago

This PR is an attempt to fix #1077. See there for a discussion of the problem.

Vekhir commented 2 months ago

This looks like a good solution for the warnings. I find the code more readable now too.

Vekhir commented 2 months ago
        if (action->property("builtIn").toBool())
        {
            for (const QKeySequence& shortcut : action->shortcuts())
            {
                shortcuts << shortcut;
            }
        }

https://github.com/OpenBoard-org/OpenBoard/blob/dev/src/core/UBShortcutManager.cpp#L599-L605

Not directly related, but why aren't there ctrlShortcuts added? Are builtIns guaranteed to never have a CTRL? Though it's never checked anywhere else. I'd expect the QKeySequence(shortcut ^ Qt::CTRL) that's in the other lines, even if it might not actually do anything useful.

letsfindaway commented 2 months ago

Not directly related, but why aren't there ctrlShortcuts added? Are builtIns guaranteed to never have a CTRL? Though it's never checked anywhere else. I'd expect the QKeySequence(shortcut ^ Qt::CTRL) that's in the other lines, even if it might not actually do anything useful.

The buildins are those which are not defined as action shortcuts but are interpreted from key press events. Here the ignore Ctrl feature is not and cannot be applied. Therefore they are not added to the ctrlShortcuts because they are not affected by the ignoreCtrl flag.