-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
(fix/UX) Track menu: Left key in Search related & Crates menus closes them again #13602
base: 2.5
Are you sure you want to change the base?
Conversation
bba8f41
to
693c17e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The issue fixed by this is still present in main
, so maybe merge it to main if it should already be too late for 2.5.
I tried this and it crashed with:
|
@JoergAtGithub I can't reproduce the crasher. It looks like a dangling pointer access. Unfortunately I cannot reproduce the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
friendly ping @JoergAtGithub - are you able to provide reproduction steps for this bug?
src/widget/wtrackmenu.cpp
Outdated
if (!m_pSearchRelatedMenu->isEmpty()) { | ||
// We're interested in keypress Qt::Key_Left, so use our | ||
// event filter like we do for the crate checkboxes. | ||
for (const auto ch : m_pSearchRelatedMenu->children()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang tidy here
for (const auto ch : m_pSearchRelatedMenu->children()) { | |
for (auto *const ch : m_pSearchRelatedMenu->children()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks! overlooked that.
Though, why is it suggesting
auto *const ch
and not
const auto* ch
?
I extended that to ignore Right key as suggested by @JoergAtGithub |
If a QWidgetAction in either Search related & Crates menu is focused, Left/Right keys are translated to Up/Down which prevents closing the submenus with Left key like in other submenus.
(in the Crates menu left works as soon as the New Crate button is reached)
Fix:
Translate Left to Esc to close this submenu.Call hide() of the respective sumenu.
For fixing the crates menu this should actually go to 2.4 ("Search related Tracks" has QWidgetActions only in 2.5), but since we're going to release that shortly before 2.5 I think it's okay to merge the double fix to 2.5-beta