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

Keyboard State Indicator: do not toggle on right click #1932

Closed
wants to merge 3 commits into from

Conversation

isf63
Copy link
Contributor

@isf63 isf63 commented Sep 7, 2023

This suppresses toggling states when trying to access the context menu

This suppresses toggling states when accessing the context menu
@isf63 isf63 changed the title Keyboard State Indicator: do not toggle on left click Keyboard State Indicator: do not toggle on right click Sep 7, 2023
@stefonarch
Copy link
Member

stefonarch commented Sep 7, 2023

It never toggled here with right click and I can't reproduce this. When does that happen?

@tsujan
Copy link
Member

tsujan commented Sep 7, 2023

It never toggled here with right click

Here neither, with or without context_menu_on_mouse_release (a new feature that isn't merged yet).

@isf63
Copy link
Contributor Author

isf63 commented Sep 7, 2023

It never toggled here with right click and I can't reproduce this. When does that happen?

Oh, I see I forgot to disable my patch when testing lxqt-qtplugin's ContextMenuOnMouseRelease.

With case ContextMenuOnMouseRelease: return QVariant(true); the state indicators toggle on right click. With this PR that is suppressed, although it also prevents middle click toggle but that can be easily fixed.

Very odd how it's not reproducible.

@tsujan
Copy link
Member

tsujan commented Sep 7, 2023

With case ContextMenuOnMouseRelease: return QVariant(true); the state indicators toggle on right click.

It doesn't — at least not with lxqt/lxqt-qtplugin#79

@tsujan
Copy link
Member

tsujan commented Sep 9, 2023

Oh, sorry! This is for locks, not the indicator itself (I don't add locks; my keyboard shows them). The word "state" in the title misled me. Yes, it's a good change.

However, please note that the PR about ContextMenuOnMouseRelease might never be merged, because other devs might not show any interest in it. For this reason, I won't merge my lxqt-archiver PR either. If ContextMenuOnMouseRelease PR is approved by other members and merged, these two PRs should be merged too — and I'll make two more to cover all needed changes.

Keep middle click behavior the same
@isf63
Copy link
Contributor Author

isf63 commented Feb 7, 2024

Close this? We can revisit these edge cases once Qt consistently supports ContextMenuOnMouseRelease on Linux.

@tsujan
Copy link
Member

tsujan commented Feb 7, 2024

Close this?

Yes. I closed my PRs but forgot this.

once Qt consistently supports ContextMenuOnMouseRelease on Linux.

If it happens at all, I don't think it'll be soon. But yes, until Qt becomes self-consistent in this regard, we could do nothing.

Anyway, thanks for starting a discussion at lxqt/lxqt#2460!

@tsujan tsujan closed this Feb 7, 2024
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.

3 participants