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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+17 −1
Open

Change behavior when tray icon is clicked #19940

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/gui/mainwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,23 @@ MainWindow::MainWindow(IGUIApplication *app, WindowState initialState)
if (!isVisible())
activate();
#else
toggleVisibility();
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();
}
Comment on lines +402 to +418
Copy link
Member

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:

Suggested change
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();

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

@thalieht thalieht Nov 14, 2023 •
edited
Loading

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():

  1. qBt out of focus
  2. Click tray icon and it brings qBt to the front
  3. 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.

Copy link
Member

@glassez glassez Nov 14, 2023 •
edited
Loading

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 removing activateWindow()

In fact, the order of operations matters. show() has to go before activateWindow(), otherwise it doesn't work.

Copy link
Member

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 returns false.

It looks like main window becomes inactive at the moment when the user interacts with the tray icon.

#endif
});

Expand Down
Loading