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

Using Command + Backspace as the shortcut to delete torrents on macOS #20668

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DouiKo
Copy link
Contributor

@DouiKo DouiKo commented Apr 7, 2024

macOS usually uses the Command key plus another key as shortcuts, similar to qBittorrent's other shortcuts.

Closes #20187.

@@ -857,6 +857,7 @@ void MainWindow::createKeyboardShortcuts()
m_ui->actionExit->setShortcut(Qt::CTRL | Qt::Key_Q);
#ifdef Q_OS_MACOS
m_ui->actionCloseWindow->setShortcut(QKeySequence::Close);
m_ui->actionDelete->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_Backspace));
Copy link
Member

Choose a reason for hiding this comment

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

Remove redundant cast:

Suggested change
m_ui->actionDelete->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_Backspace));
m_ui->actionDelete->setShortcut(Qt::CTRL | Qt::Key_Backspace);

Also shouldn't you use QAction::setShortcuts? otherwise you'll override line 854.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could this code style be approved? Or should it be called separately?

m_ui->actionDelete->setShortcut(
    #ifdef Q_OS_MACOS
        Qt::CTRL | Qt::Key_Backspace
    #else
        QKeySequence::Delete
    #endif
);

Copy link
Member

Choose a reason for hiding this comment

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

Could this code style be approved?

I wouldn't use it if there are other ways. Besides that code is still overriding QKeySequence::Delete. Qt::CTRL | Qt::Key_Backspace should be additional to QKeySequence::Delete. They are not mutually exclusive.

Would the following work? Are you able to confirm it?

Suggested change
m_ui->actionDelete->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_Backspace));
m_ui->actionDelete->setShortcuts(Qt::CTRL | Qt::Key_Backspace);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setShortcuts(Qt::CTRL | Qt::Key_Backspace) works. Following your suggestion, I pushed a new commit.

@glassez glassez changed the title Using Command + Backspace as the shortcut to delete torrents on macOS. Using Command + Backspace as the shortcut to delete torrents on macOS Apr 7, 2024
@glassez glassez added OS: macOS Issues specific to macOS GUI GUI-related issues/changes labels Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI GUI-related issues/changes OS: macOS Issues specific to macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete shortcut
3 participants