Differential D15371

Don't assign twice the same key to the action New Tab
ClosedPublic
Actions

Authored by jtamate on Sep 9 2018, 11:25 AM.

Details

Reviewers
elvisangelaccio
ngraham
Group Reviewers
Dolphin
Commits
R318:9991eb0f5eaf: Don't assign twice the same key to the action New Tab
Summary

QKeySequence::AddTab = Qt::CTRL + Qt::Key_T.
Leaving only the first avoids the warning window "There are two actions (New Tab, New Tab) that want to use the same shortcut (Ctrl+T)"

BUG: 398324

Test Plan

su - test
unset KDE_FULL_SESSION
dolphin
After: No more the warning window.

Diff Detail

Repository
R318 Dolphin
Lint
Lint Skipped
Unit
Unit Tests Skipped
jtamate created this revision.Sep 9 2018, 11:25 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptSep 9 2018, 11:25 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
jtamate requested review of this revision.Sep 9 2018, 11:25 AM
elvisangelaccio accepted this revision.Sep 9 2018, 1:56 PM
elvisangelaccio added a subscriber: elvisangelaccio.

Stable branch, please. Thanks!

This revision is now accepted and ready to land.Sep 9 2018, 1:56 PM
Closed by commit R318:9991eb0f5eaf: Don't assign twice the same key to the action New Tab (authored by jtamate). · Explain WhySep 10 2018, 2:47 PM
This revision was automatically updated to reflect the committed changes.
ngraham reopened this revision.EditedOct 5 2018, 1:20 PM
ngraham added a subscriber: ngraham.

This broke the Ctrl+T keyboard shortcut. Can we please fix it?

This revision is now accepted and ready to land.Oct 5 2018, 1:20 PM
jtamate updated this revision to Diff 42924.Oct 5 2018, 2:40 PM

As it turns out, QKeySequence::AddTab is Ctrl+Shift+N under plasma.
Filled Qt bug: https://bugreports.qt.io/browse/QTBUG-70963
Change to Qt::CTRL + Qt::Key_T.

jtamate updated this revision to Diff 42928.Oct 5 2018, 3:17 PM
jtamate edited the test plan for this revision. (Show Details)

Keep both key sequences in case someone used ctrl+shift+n.

ngraham accepted this revision.Oct 5 2018, 10:58 PM

Much better, thanks. Please commit to the stable branch and merge to master, since the original commit went into the stable branch and we don't want it to stay broken there.

elvisangelaccio added a comment.Oct 6 2018, 8:18 AM

I don't get it. Ctrl+T works with dolphin from my arch distro package. It doesn't work with my self-compiled dolphin though. What's going on?

Revision Contents
Changeset List

PathPackages
Msrc/dolphinmainwindow.cpp (2 lines)

Diff 42928

View Options

src/dolphinmainwindow.cpp

Loading...