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

Qt: Fix restoration of minimized to tray window #14222

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@hebasto
Contributor

hebasto commented Sep 15, 2018

Fix UX bug on Windows. Steps to reproduce (tested on Windows 10):

  1. In Options->Window enable "Minimize to the tray instead of the taskbar".
  2. Minimize the main window.
  3. Select Show/Hide on the tray icon menu (or just click on the tray icon).

The normal size window is expected.

Actually the main window is restored minimized to the taskbar.

Rationale: minimizing the window with fMinimizeToTray==true actually do both showMinimized() and hide(). During restoration isMinimized() should be checked first. Otherwise, the restored window will be minimized.

The logic is broken both on Windows and Linux, but on my Linux (Mint 19 + Cinnamon 3.8.9) the window manager renders the main window as if all is correct. Nevertheless, other Linux systems may be affected by this bug.

Ref #11859

Fix restoration of minimized to tray window
Minimizing the window with fMinimizeToTray=true actually do both
showMinimized() and hide(). During restoration isMinimized() should be
checked first. Otherwise, the restored window will be minimized.

@fanquake fanquake added the GUI label Sep 15, 2018

@fanquake

This comment has been minimized.

Show comment
Hide comment
@fanquake

fanquake Sep 15, 2018

Member

@hebasto Can you have a look at #14123, as it might fix the issue you're seeing.

Member

fanquake commented Sep 15, 2018

@hebasto Can you have a look at #14123, as it might fix the issue you're seeing.

@DrahtBot

This comment has been minimized.

Show comment
Hide comment
@DrahtBot

DrahtBot Sep 15, 2018

Contributor
No more conflicts as of last run.
Contributor

DrahtBot commented Sep 15, 2018

No more conflicts as of last run.
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Sep 27, 2018

Member

Tested ACK f03c5c3

This successfully solves the "two step" show with the tray icon. Tested on all Ubuntu/macOS/Win.

Member

jonasschnelli commented Sep 27, 2018

Tested ACK f03c5c3

This successfully solves the "two step" show with the tray icon. Tested on all Ubuntu/macOS/Win.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Sep 27, 2018

Member

Is this for backport?

Member

MarcoFalke commented Sep 27, 2018

Is this for backport?

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Sep 27, 2018

Member

Not sure this is worth back-porting... All UI-glitch-fixes have the risk to act differently in back ported versions. I would recommend to not back port it (if its not a clear bug that has been fixed).

Member

jonasschnelli commented Sep 27, 2018

Not sure this is worth back-porting... All UI-glitch-fixes have the risk to act differently in back ported versions. I would recommend to not back port it (if its not a clear bug that has been fixed).

@DrahtBot

This comment has been minimized.

Show comment
Hide comment
@DrahtBot

DrahtBot Sep 28, 2018

Contributor
Coverage Change (pull 14222) Reference (master)
Lines +0.0044 % 87.0361 %
Functions +0.0154 % 84.1130 %
Branches -0.0190 % 51.5451 %
Contributor

DrahtBot commented Sep 28, 2018

Coverage Change (pull 14222) Reference (master)
Lines +0.0044 % 87.0361 %
Functions +0.0154 % 84.1130 %
Branches -0.0190 % 51.5451 %
@hebasto

This comment has been minimized.

Show comment
Hide comment
@hebasto

hebasto Oct 12, 2018

Contributor

@Sjors Do you mind reviewing this PR?

Contributor

hebasto commented Oct 12, 2018

@Sjors Do you mind reviewing this PR?

@Sjors

This comment has been minimized.

Show comment
Hide comment
@Sjors

Sjors Oct 13, 2018

Member

I can't find the "Minimize to the tray instead of the taskbar" option on macOS 10.14 with QT 5.11.2.

Member

Sjors commented Oct 13, 2018

I can't find the "Minimize to the tray instead of the taskbar" option on macOS 10.14 with QT 5.11.2.

@hebasto

This comment has been minimized.

Show comment
Hide comment
@hebasto

hebasto Oct 13, 2018

Contributor

@Sjors

/* Window elements init */
#ifdef Q_OS_MAC
/* remove Window tab on Mac */
ui->tabWidget->removeTab(ui->tabWidget->indexOf(ui->tabWindow));
#endif

Contributor

hebasto commented Oct 13, 2018

@Sjors

/* Window elements init */
#ifdef Q_OS_MAC
/* remove Window tab on Mac */
ui->tabWidget->removeTab(ui->tabWidget->indexOf(ui->tabWindow));
#endif

@Sjors

This comment has been minimized.

Show comment
Hide comment
@Sjors

Sjors Oct 14, 2018

Member

Ok, this seems to have no effect on macOS, so works for me.

@promag does this get in the way of what you're trying to achieve in #14123? That PR fixes #13829, this PR doesn't.

Member

Sjors commented Oct 14, 2018

Ok, this seems to have no effect on macOS, so works for me.

@promag does this get in the way of what you're trying to achieve in #14123? That PR fixes #13829, this PR doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment