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

Workaround to fix QTBUG-67275: respect Enter key for triggering context menu items #81

Conversation

PoisonousJohn
Copy link
Contributor

Right now it's not possible to trigger MenuItems in context menu using Enter key, which is a default behavior for all platforms

@PoisonousJohn PoisonousJohn self-assigned this Mar 23, 2018
@PoisonousJohn PoisonousJohn added the bug Something isn't working label Mar 23, 2018
@plfiorini
Copy link
Member

This feels so wrong, Enter is used to trigger menu items everywhere in addition to Space but with Qt 5.10.1 only Space works... According to https://stackoverflow.com/questions/49436480/qml-menuitem-trigger-by-enter-and-keyboard-events-forwarding-dont-work this look like a behavior change compared to Controls 1 so maybe the Qt developers forgot about Enter with Controls 2?

You should file a bug report.

@plfiorini
Copy link
Member

Please if you merge this commit change the commit message to reflect that this is a workaround for a Qt behavior that doesn't feel right. Also link to the Qt bug report.

@PoisonousJohn
Copy link
Contributor Author

@plfiorini thx, I'll do

@plfiorini
Copy link
Member

I'm trying to understand how triggered is emitted.
The clicked signal is connected to triggered here.

Key press and release events of the abstract button only handle Space, see here and here.

If my analysis is right the same problem should happen with push buttons.

The fix seems simple: just check for Enter too and keypad Enter, in addition to Space.

@PoisonousJohn
Copy link
Contributor Author

@plfiorini yes, came up to the same thoughts

@plfiorini
Copy link
Member

Maybe the patch can be sent to the 5.9 or 5.11 branch and might be even part of Qt 5.11.0 in May.

@PoisonousJohn
Copy link
Contributor Author

@plfiorini I don't have experience sending patches, what is a right process for that? I'll need to test the patch somehow, and I'm afraid it's a big deal, since you should recompile Qt

@plfiorini
Copy link
Member

You need a gerrit account at https://codereview.qt-project.org, sign the contribution agreement and follow the guide: http://wiki.qt.io/Qt_Contribution_Guidelines
And yes you would need to build the Qt modules from qt5.git you are interested in (in this case at least qtbase, qtdeclarative, qtxmlpatterns and qtquickcontrols2).

Considering that I built Qt 5.11 from sources two days ago I can make the patch if you submit a bug report :)

@PoisonousJohn
Copy link
Contributor Author

@plfiorini it would be great. Here's a bug https://bugreports.qt.io/browse/QTBUG-67275

@PoisonousJohn PoisonousJohn changed the title fix to respect Enter key for triggering context menu items Workaround to fix QTBUG-67275: respect Enter key for triggering context menu items Mar 23, 2018
@PoisonousJohn PoisonousJohn force-pushed the feature/fix-enter-key-context-menu branch from 688db5a to 749d42f Compare March 23, 2018 10:32
@PoisonousJohn
Copy link
Contributor Author

PoisonousJohn commented Mar 23, 2018

@plfiorini fixed commit description as well

@timsueberkrueb
Copy link
Collaborator

Thank you @PoisonousJohn and @plfiorini!
Nitpick: the commit message is pretty long, if you change the commit message to use two lines like this:

Respect enter key for triggering context menu items
This is a workaround for QTBUG-67275.

It will look better in GitHub and in the console.

It's a workaround to fix QTBUG-67275
@PoisonousJohn PoisonousJohn force-pushed the feature/fix-enter-key-context-menu branch 2 times, most recently from 33ab0dd to 2a2faa9 Compare March 23, 2018 17:52
@timsueberkrueb timsueberkrueb merged commit dc46757 into lirios:develop Mar 23, 2018
@PoisonousJohn PoisonousJohn deleted the feature/fix-enter-key-context-menu branch March 23, 2018 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants