-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Change behavior when tray icon is clicked #19940
base: master
Are you sure you want to change the base?
Conversation
The question is shouldn't we change behavior only for icon click and preserve the current one for context menu? |
qBittorrent will be hidden only when it is in focus.
Done. |
if (isActiveWindow()) | ||
{ | ||
hide(); | ||
} | ||
else | ||
{ | ||
if (m_uiLocked && !unlockUI()) // Ask for UI lock password | ||
return; | ||
|
||
// Make sure the window is not minimized | ||
setWindowState((windowState() & ~Qt::WindowMinimized) | Qt::WindowActive); | ||
|
||
// Then show it | ||
show(); | ||
raise(); | ||
activateWindow(); | ||
} |
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.
I believe you shouldn't do setWindowState((windowState() & ~Qt::WindowMinimized) | Qt::WindowActive);
manually. The next 3 methods will care about it. Or did your tests show the another result?
And if I'm right, then it can even be rewritten as the following:
if (isActiveWindow()) | |
{ | |
hide(); | |
} | |
else | |
{ | |
if (m_uiLocked && !unlockUI()) // Ask for UI lock password | |
return; | |
// Make sure the window is not minimized | |
setWindowState((windowState() & ~Qt::WindowMinimized) | Qt::WindowActive); | |
// Then show it | |
show(); | |
raise(); | |
activateWindow(); | |
} | |
if (isActiveWindow()) | |
hide(); | |
else | |
activate(); |
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.
I didn't notice any problems at all.
I believe you shouldn't do setWindowState((windowState() & ~Qt::WindowMinimized) | Qt::WindowActive); manually.
Removing only that line or with your suggestion, when qBt is minimized (not to tray) clicking the tray icon doesn't restore the window.
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.
Removing only that line or with your suggestion, when qBt is minimized (not to tray) clicking the tray icon doesn't restore the window.
OK.
Seems setWindowState(windowState() & ~Qt::WindowMinimized)
is really required but setting Qt::WindowActive
flag does exactly the same as activateWindow()
so one of them can be omitted.
Unfortunately, this code does not work correctly (at least on Windows, where I managed to test it). It does not hide the main window when clicking on the tray icon. For some reason, isActiveWindow()
always returns false
.
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.
but setting Qt::WindowActive flag does exactly the same as activateWindow() so one of them can be omitted.
Ok i see no problems with removing Qt::WindowActive
but i do with removing activateWindow()
:
- qBt out of focus
- Click tray icon and it brings qBt to the front
- Subsequent tray icon clicks do nothing (qBt is not in focus)
Unfortunately, this code does not work correctly (at least on Windows, where I managed to test it). It does not hide the main window when clicking on the tray icon. For some reason, isActiveWindow() always returns false.
Damn... i can't test in Windows, i mean i could with CI builds but it would be a chore.
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.
Ok i see no problems with removing
Qt::WindowActive
but i do with removingactivateWindow()
In fact, the order of operations matters. show()
has to go before activateWindow()
, otherwise it doesn't work.
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.
For some reason,
isActiveWindow()
always returnsfalse
.
It looks like main window becomes inactive at the moment when the user interacts with the tray icon.
In fact, I was always annoyed by the behavior of the qBittorrent tray icon, but it was a small thing for me, so I never even tried to delve into what exactly it does.
Note that none of the icons (except qBittorrent) hides the opened window when clicked. This can only be done either through the context menu of the icon, or by closing the window itself in the usual way. |
Isn't that what we want?
Mumble behaves like qbittorrent exactly (except no "Hide" from context menu), i have no opinion for this behavior. |
Yes. |
qBittorrent will be hidden only when it is in focus.
Closes #11424