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

mainmenu: Remove shortcut popup delay #1771

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

Conversation

palinek
Copy link
Contributor

@palinek palinek commented Apr 8, 2022

This was introduced as workaround for "menu not in focus after shortcut
activation". But the root cause of the problem was probably in
lxqt-globalkeys, where the signal was triggered on KeyPress instead of
KeyRelease <- this was fixed recently. Now the delay is pointless.

Closes #1770

This was introduced as workaround for "menu not in focus after shortcut
activation". But the root cause of the problem was probably in
lxqt-globalkeys, where the signal was triggered on KeyPress instead of
KeyRelease <- this was fixed recently. Now the delay is pointless.
This is some remnant from previous workaround to
lxqt/lxqt#549.

Note: the referenced bug is not seen in current version.
@stefonarch
Copy link
Member

Works fine.

Copy link
Member

@tsujan tsujan left a comment

Choose a reason for hiding this comment

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

The removal of the delay causes a serious issue here: not only the search entry main menu doesn't get focused but the main menu it can't be closed by any means other than pressing the shortcut again or pressing the menu button on Panel. A menu that can't be closed easily is dangerous.

@tsujan
Copy link
Member

tsujan commented Apr 14, 2022

The above-mentioned problem happens randomly but, IMO, since it has never happened here with the delay, it shows that the delay is needed.

@stefonarch
Copy link
Member

I didn't notice any misbehavior until now here.

@tsujan
Copy link
Member

tsujan commented Apr 14, 2022

It may depend on WM and/or its animations. I can easily reproduce the problem with the patch + KWin.

I don't think lxqt-globalkeys caused the focus problem in the first place. It may be related to Qt and how it shows menus. In the case of lxqt-panel, there's no focus when the menu is going to be shown, and that may contribute to the problem.

EDIT: I can reproduce the issue with all LXQt themes and all widget styles.

@stefonarch
Copy link
Member

stefonarch commented Apr 14, 2022

Kwin too here. Did you test a shorter delay?
But to ship it in 1.1.0 it has to be 100% ok for everyone.

@tsujan
Copy link
Member

tsujan commented Apr 14, 2022

Did you test a shorter delay?

The 200-ms delay in the master is the best. With 100 ms, it happens rarely. Without a delay, it happens most of the time here.

IMO, until we find the real cause and eradicate the problem — or at least until we find an efficient workaround without a delay — the current delay is the safest workaround.

@AdelKS
Copy link

AdelKS commented Apr 16, 2022

IMO, until we find the real cause and eradicate the problem — or at least until we find an efficient workaround without a delay — the current delay is the safest workaround.

I think the changes I did in PR #1698 maybe uncover the root cause of all this, I will dig deeper and see if we can use QKeySequence everywhere instead of QString of keyboard shortcuts.

@AdelKS
Copy link

AdelKS commented Apr 17, 2022

I will dig deeper and see if we can use QKeySequence everywhere instead of QString of keyboard shortcuts.

Unfortunately not, it is more constraining and it does not solve the fact that we cannot use the Super key in key combinations as it is considered to be a normal key, it took me a while to understand that QKeySequence cannot encode e.g. Ctrl+Super+A+B.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delay after opening Application Menu (mainmenu)
4 participants